Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimise no-op PATCH ops in VC HTTP API #5064

Merged
merged 3 commits into from
Feb 29, 2024

Conversation

michaelsproul
Copy link
Member

Issue Addressed

Closes #4936

Proposed Changes

Optimise PATCH /lighthouse/validators/{validator_pubkey} so that it is faster when processing redundant changes. I suspect this will help VCs running thousands of keys from becoming overwhelmed when processing lots of these API queries.

Longer-term we should still think about optimising the key cache and API more for actual changes. I suspect the O(n) loop over the validators for each request is part of the problem, here:

.find(|def| def.voting_public_key == *voting_public_key)

Addressing this by using a persistent map would both make the API faster, and help to resolve atomicity issues like #4984.

@michaelsproul michaelsproul added val-client Relates to the validator client binary optimization Something to make Lighthouse run more efficiently. HTTP-API labels Jan 15, 2024
@michaelsproul
Copy link
Member Author

When spamming requests, (h)top shows Lighthouse using 20% CPU rather than 100% after this patch. I also took some screenshots of system-wide CPU metrics, but it's a bit hard to see an improvement there, because I think the system just uses the same amount of CPU to run more spam queries 😅

cpu_usage

  • green (left) = non-idle CPU
  • blue = system (kernel)
  • yellow (left) = user
  • green (right) = user
  • blue = system (kernel)
  • yellow (right) = iowait

Spam script is something like:

#!/usr/bin/env bash

while [ true ]
do
	curl -s -X PATCH --data '{"builder_proposals": false}' -H "Content-Type: application/json" -H "Authorization: Bearer api-token-0x02430fd0af4a262df8be03e2f2fad494162d1d22488720fc8254639d7371b2ff7b" "http://localhost:5062/lighthouse/validators/0x90e3b075e5261c4ba40e0d33480ae16251f9383e30e1b541bd8ddfb3b04a5e64965fb5d3cabc7ff9bf52a2b028c3b9a5"
done

@dapplion dapplion added the ready-for-review The code is ready for review label Jan 29, 2024
@michaelsproul 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 Jan 31, 2024
@michaelsproul michaelsproul added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Feb 9, 2024
Copy link
Member

@macladson macladson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me and I see no reason not to do it, but I also agree that some kind of map seems like a natural solution for the long term (although I'm not sure about the implications of such a change).

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. v5.1.0 Q2 2024 and removed ready-for-review The code is ready for review labels Feb 27, 2024
@michaelsproul
Copy link
Member Author

@Mergifyio queue

Copy link

mergify bot commented Feb 28, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 88b37a1

mergify bot added a commit that referenced this pull request Feb 28, 2024
mergify bot added a commit that referenced this pull request Feb 29, 2024
@mergify mergify bot merged commit 88b37a1 into sigp:unstable Feb 29, 2024
29 checks passed
pawanjay176 pushed a commit to pawanjay176/lighthouse that referenced this pull request Mar 5, 2024
* Optimise no-op changes in VC API

* Handle another no-op case

* Merge remote-tracking branch 'origin/unstable' into opt-vc-patch-api
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HTTP-API optimization Something to make Lighthouse run more efficiently. ready-for-merge This PR is ready to merge. v5.1.0 Q2 2024 val-client Relates to the validator client binary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants