[Merged by Bors] - Update node health endpoint #4310

eserilev · 2023-05-21T02:58:44Z

Issue Addressed

#4292

Proposed Changes

Updated the node health endpoint

will return a 200 status code if !syncing && !el_offline && !optimistic

wil return a 206 if (syncing || optimistic) && !el_offline

will return a 503 if el_offline

Additional Info

eserilev · 2023-05-21T03:03:40Z

beacon_node/http_api/src/lib.rs

+                    let unhealthy = is_syncing || Some(is_optimistic) || Some(el_offline);
+
+                    if (unhealthy) {
+                        Err(warp_utils::reject::not_synced(format!(


could have returned a different error message for each situation where unhealthy resolves to true, but im not sure how helpful that is

eserilev · 2023-05-21T07:11:38Z

test_get_node_health is failing. will need to figure out how to get that test to pass (assuming my implementation is correct)

paulhauner · 2023-05-22T01:34:47Z

Thanks for your contribution. I'd like to note that the Beacon-API specs state that:

206 Node is syncing but can serve incomplete data

So, moving to a NotSynced (503) would be going against the spec. I'm also not sure it's clear that syncing node is necessarily unhealthy. For example, a newly provisioned node that is syncing is "healthy" in the sense that it's working as intended and requires no intervention.

Fine-grained information is available at node/syncing.

I do agree that a node with a ee_offline is unhealthy, though. I'd be open to seeing an error returned when ee_offline == true.

michaelsproul · 2023-05-22T01:38:09Z

Yeah my bad, I didn't distinguish between syncing and EL offline in the original issue. I agree we should return a 206 for syncing/optimistic, and a 503 for el_offline.

eserilev · 2023-05-22T03:02:26Z

thanks for the feedback!

eserilev · 2023-05-22T13:29:58Z

beacon_node/http_api/tests/status_tests.rs

+    let mock_el = harness.mock_execution_layer.as_ref().unwrap();
+
+    // EL not synced
+    mock_el.server.set_syncing_response(Ok(true));


This test currently fails. I'm trying to test the situation where the node is syncing and the endpoint returns a 206 status code. However i'm not really sure how to mock syncing to true

@michaelsproul ideally i'd like to test the situation where the node health endpoint returns a 206, but I'm struggling trying to find a way to mock the situation where the node is syncing/optimistic

You can call mock_el.server.all_payloads_syncing(true), then add a block (e.g. with extend_chain). That new block will result in a SYNCING status from the EL on newPayload, which will put Lighthouse into optimistic sync

…date-node-health-endpoint

eserilev · 2023-05-30T17:42:31Z

@michaelsproul thanks for the feedback and help. this should be ready for another review

beacon_node/http_api/src/lib.rs

…date-node-health-endpoint

michaelsproul

Nice, thanks!

paulhauner · 2023-06-30T01:12:47Z

bors r+

## Issue Addressed [#4292](#4292) ## Proposed Changes Updated the node health endpoint will return a 200 status code if `!syncing && !el_offline && !optimistic` wil return a 206 if `(syncing || optimistic) && !el_offline` will return a 503 if `el_offline` ## Additional Info

bors · 2023-06-30T03:46:52Z

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

## Issue Addressed [sigp#4292](sigp#4292) ## Proposed Changes Updated the node health endpoint will return a 200 status code if `!syncing && !el_offline && !optimistic` wil return a 206 if `(syncing || optimistic) && !el_offline` will return a 503 if `el_offline` ## Additional Info

[sigp#4292](sigp#4292) Updated the node health endpoint will return a 200 status code if `!syncing && !el_offline && !optimistic` wil return a 206 if `(syncing || optimistic) && !el_offline` will return a 503 if `el_offline`

eserilev added 4 commits May 21, 2023 05:20

node health endpoint updated

dd36065

remove unneeded struct

ebd3e19

updated node health response

6afd1a9

updated err message

e388dba

eserilev commented May 21, 2023

View reviewed changes

build error fix

b8fbbe0

eserilev closed this May 21, 2023

build error fix

5c0f82d

eserilev reopened this May 21, 2023

cargo fmt

9947af8

paulhauner added the work-in-progress PR is a work-in-progress label May 22, 2023

eserilev added 3 commits May 22, 2023 12:49

return 206 on syncing/optimistic

66bc59b

updated error message

c8fc66f

add unit tests

cf55fb8

eserilev commented May 22, 2023

View reviewed changes

eserilev added 2 commits May 23, 2023 07:54

updated existing test

ca8fc00

Merge branch 'unstable' of https://github.com/sigp/lighthouse into up…

8d46989

…date-node-health-endpoint

michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. v4.3.0 Estimated Q2 2023 labels May 30, 2023

eserilev added 2 commits May 30, 2023 20:00

Merge branch 'unstable' of https://github.com/sigp/lighthouse into up…

d3efd73

…date-node-health-endpoint

added syncing test

8b0183f

michaelsproul added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels May 30, 2023

michaelsproul requested changes Jun 27, 2023

View reviewed changes

beacon_node/http_api/src/lib.rs Outdated Show resolved Hide resolved

beacon_node/http_api/src/lib.rs Outdated Show resolved Hide resolved

beacon_node/http_api/src/lib.rs Outdated Show resolved Hide resolved

michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jun 27, 2023

eserilev added 3 commits June 27, 2023 09:16

Merge branch 'unstable' of https://github.com/sigp/lighthouse into up…

78115c7

…date-node-health-endpoint

reinstate sync stalled behavior plus formatting

834dd90

fmt

b53b0ad

michaelsproul approved these changes Jun 29, 2023

View reviewed changes

michaelsproul added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Jun 29, 2023

paulhauner mentioned this pull request Jun 30, 2023

[Merged by Bors] - Release v4.3.0 #4452

Closed

bors bot changed the title ~~Update node health endpoint~~ [Merged by Bors] - Update node health endpoint Jun 30, 2023

bors bot closed this Jun 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Merged by Bors] - Update node health endpoint #4310

[Merged by Bors] - Update node health endpoint #4310

Uh oh!

eserilev commented May 21, 2023 •

edited

Loading

Uh oh!

eserilev May 21, 2023

Uh oh!

eserilev commented May 21, 2023 •

edited

Loading

Uh oh!

paulhauner commented May 22, 2023

Uh oh!

michaelsproul commented May 22, 2023

Uh oh!

eserilev commented May 22, 2023

Uh oh!

eserilev May 22, 2023 •

edited

Loading

Uh oh!

eserilev May 24, 2023

Uh oh!

michaelsproul May 30, 2023

Uh oh!

eserilev commented May 30, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michaelsproul left a comment

Uh oh!

paulhauner commented Jun 30, 2023

Uh oh!

bors bot commented Jun 30, 2023

Uh oh!

Uh oh!

[Merged by Bors] - Update node health endpoint #4310

[Merged by Bors] - Update node health endpoint #4310

Uh oh!

Conversation

eserilev commented May 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue Addressed

Proposed Changes

Additional Info

Uh oh!

eserilev May 21, 2023

Choose a reason for hiding this comment

Uh oh!

eserilev commented May 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paulhauner commented May 22, 2023

Uh oh!

michaelsproul commented May 22, 2023

Uh oh!

eserilev commented May 22, 2023

Uh oh!

eserilev May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eserilev May 24, 2023

Choose a reason for hiding this comment

Uh oh!

michaelsproul May 30, 2023

Choose a reason for hiding this comment

Uh oh!

eserilev commented May 30, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michaelsproul left a comment

Choose a reason for hiding this comment

Uh oh!

paulhauner commented Jun 30, 2023

Uh oh!

bors bot commented Jun 30, 2023

Uh oh!

Uh oh!

eserilev commented May 21, 2023 •

edited

Loading

eserilev commented May 21, 2023 •

edited

Loading

eserilev May 22, 2023 •

edited

Loading