Skip to content

[Merged by Bors] - Configure the validator/register_validator batch size via the CLI #4399

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

Closed
wants to merge 3 commits into from

Conversation

paulhauner
Copy link
Member

@paulhauner paulhauner commented Jun 14, 2023

Issue Addressed

NA

Proposed Changes

Adds the --validator-registration-batch-size flag to the VC to allow runtime configuration of the number of validators POSTed to the validator/register_validator endpoint.

There are builders (Agnostic and Eden) that are timing out with regsiterValidator requests with ~400 validators, even with a 9 second timeout. Exposing the batch size will help tune batch sizes to (hopefully) avoid this.

This PR should not change the behavior of Lighthouse when the new flag is not provided (i.e., the same default value is used).

Additional Info

NA

@paulhauner paulhauner added work-in-progress PR is a work-in-progress v4.3.0 Estimated Q2 2023 labels Jun 14, 2023
@paulhauner paulhauner changed the title Allow configuring the validator/register_validator batch size via the CLI Configure the validator/register_validator batch size via the CLI Jun 14, 2023
@paulhauner
Copy link
Member Author

paulhauner commented Jun 16, 2023

I've been testing this on mainnet and I've found that the number of timeouts reduces significantly by reducing the batch size.

Here are some Kibana metrics on timeout logs for registerValidator:

Timeouts with 400 validators per request

These numbers are without the new flag supplied. The default is 500, however I know that the maximum request size was 400. We can see that the timeouts stop at one point; that's because I've started using the flag to reduce the batch size.

Screenshot 2023-06-16 at 11 04 02 am

Timeouts with 200 validators per request

--validator-registration-batch-size 200

Screenshot 2023-06-16 at 11 06 46 am

Timeouts with 100 validators per request

--validator-registration-batch-size 100

This flag was only activated about midday on June 15, that's why there are no events before that point.

Screenshot 2023-06-16 at 11 06 38 am

We can see it's still not perfect with 100 validators, however it's much better. My conclusions from these tests are:

  1. The flag works (i.e., it changes the size of the batches)
  2. Reducing the size of the batches helps reduce timeouts.

Therefore, I'm happy to propose that we go ahead with this PR.

@paulhauner paulhauner added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Jun 16, 2023
@paulhauner paulhauner marked this pull request as ready for review June 16, 2023 01:02
@jimmygchen
Copy link
Member

just a minor comment on one of the tests, otherwise looks great to me! 👍

Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Great

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jun 22, 2023
@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jun 22, 2023
…4399)

## Issue Addressed

NA

## Proposed Changes

Adds the `--validator-registration-batch-size` flag to the VC to allow runtime configuration of the number of validators POSTed to the [`validator/register_validator`](https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Validator/registerValidator) endpoint.

There are builders (Agnostic and Eden) that are timing out with `regsiterValidator` requests with ~400 validators, even with a 9 second timeout. Exposing the batch size will help tune batch sizes to (hopefully) avoid this.

This PR should not change the behavior of Lighthouse when the new flag is not provided (i.e., the same default value is used).

## Additional Info

NA
@bors
Copy link

bors bot commented Jun 22, 2023

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.

@bors bors bot changed the title Configure the validator/register_validator batch size via the CLI [Merged by Bors] - Configure the validator/register_validator batch size via the CLI Jun 22, 2023
@bors bors bot closed this Jun 22, 2023
ghost pushed a commit to oone-world/lighthouse that referenced this pull request Jul 13, 2023
…igp#4399)

## Issue Addressed

NA

## Proposed Changes

Adds the `--validator-registration-batch-size` flag to the VC to allow runtime configuration of the number of validators POSTed to the [`validator/register_validator`](https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Validator/registerValidator) endpoint.

There are builders (Agnostic and Eden) that are timing out with `regsiterValidator` requests with ~400 validators, even with a 9 second timeout. Exposing the batch size will help tune batch sizes to (hopefully) avoid this.

This PR should not change the behavior of Lighthouse when the new flag is not provided (i.e., the same default value is used).

## Additional Info

NA
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
…igp#4399)

## Issue Addressed

NA

## Proposed Changes

Adds the `--validator-registration-batch-size` flag to the VC to allow runtime configuration of the number of validators POSTed to the [`validator/register_validator`](https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Validator/registerValidator) endpoint.

There are builders (Agnostic and Eden) that are timing out with `regsiterValidator` requests with ~400 validators, even with a 9 second timeout. Exposing the batch size will help tune batch sizes to (hopefully) avoid this.

This PR should not change the behavior of Lighthouse when the new flag is not provided (i.e., the same default value is used).

## Additional Info

NA
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
…igp#4399)

## Issue Addressed

NA

## Proposed Changes

Adds the `--validator-registration-batch-size` flag to the VC to allow runtime configuration of the number of validators POSTed to the [`validator/register_validator`](https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Validator/registerValidator) endpoint.

There are builders (Agnostic and Eden) that are timing out with `regsiterValidator` requests with ~400 validators, even with a 9 second timeout. Exposing the batch size will help tune batch sizes to (hopefully) avoid this.

This PR should not change the behavior of Lighthouse when the new flag is not provided (i.e., the same default value is used).

## Additional Info

NA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge. v4.3.0 Estimated Q2 2023
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants