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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - Implement API for block rewards #2628

Closed
wants to merge 8 commits into from
Closed

[Merged by Bors] - Implement API for block rewards #2628

wants to merge 8 commits into from

Conversation

michaelsproul
Copy link
Member

Proposed Changes

Add an API endpoint for retrieving detailed information about block rewards.

For information on usage see the docs, and the source.

@michaelsproul michaelsproul added the work-in-progress PR is a work-in-progress label Sep 26, 2021
@michaelsproul
Copy link
Member Author

TODO before this is ready to merge:

  • Basic API tests
  • Remove unwraps

@michaelsproul
Copy link
Member Author

Update: I've added an SSE event to the /eth/v1/events endpoint with the topic name block_reward. It outputs a BlockReward JSON object for each block that is processed, suitable for sending to the classifier. For a demo see the event listener in blockprint: https://github.com/sigp/blockprint/blob/b296431f706443c4a83969068bed532e6bd3e0c9/event_listener.py

@michaelsproul
Copy link
Member Author

I'm going to add the number of skipped slots from the parent to the reward information, because rewards at skipped slots are usually higher (and they should be). This will be a breaking change to the API between Lighthouse and blockprint but I'll update both together.

@michaelsproul
Copy link
Member Author

I'd been getting memory leaks* running large queries against this API and I think I've finally found the cause. Profiling with dhat-rs shows huge amounts of memory being allocated by the tree hash cache, on the order of 370GB in 30 minutes (12GB per minute).

thc_372gb

This is exacerbated by running on a machine with 32 cores, in which case GNU malloc will use up to 64 memory arenas. As a workaround setting MALLOC_ARENA_MAX=1 seems to stop the fragmentation and reduce the memory usage from 60GB+ (OOM) to <2GB when running for an extended period.

I'm planning to write a new generic BlockReplayer which can be used for this API and internally by the database. It will avoid re-calculating state roots unnecessarily as per #2431, and provide callbacks to hook into the block replay (e.g. for the block rewards we want to run a hook before each block is applied).

@michaelsproul michaelsproul mentioned this pull request Dec 14, 2021
Closed
@michaelsproul
Copy link
Member Author

Rebased on #2863, although it hasn't entirely solved the memory issues on high-core machines so I'm going to continue using a MALLOC_ARENA_MAX limit

bors bot pushed a commit that referenced this pull request Dec 21, 2021
@michaelsproul
Add configurable block replayer (#2863)
ad6f84c 
## Issue Addressed

Successor to #2431

## Proposed Changes

* Add a `BlockReplayer` struct to abstract over the intricacies of calling `per_slot_processing` and `per_block_processing` while avoiding unnecessary tree hashing.
* Add a variant of the forwards state root iterator that does not require an `end_state`.
* Use the `BlockReplayer` when reconstructing states in the database. Use the efficient forwards iterator for frozen states.
* Refactor the iterators to remove `Arc<HotColdDB>` (this seems to be neater than making _everything_ an `Arc<HotColdDB>` as I did in #2431).

Supplying the state roots allow us to avoid building a tree hash cache at all when reconstructing historic states, which saves around 1 second flat (regardless of `slots-per-restore-point`). This is a small percentage of worst-case state load times with 200K validators and SPRP=2048 (~15s vs ~16s) but a significant speed-up for more frequent restore points: state loads with SPRP=32 should be now consistently <500ms instead of 1.5s (a ~3x speedup).

## Additional Info

Required by #2628
bors bot pushed a commit that referenced this pull request Dec 21, 2021
@michaelsproul
Add configurable block replayer (#2863)
a290a3c 
## Issue Addressed

Successor to #2431

## Proposed Changes

* Add a `BlockReplayer` struct to abstract over the intricacies of calling `per_slot_processing` and `per_block_processing` while avoiding unnecessary tree hashing.
* Add a variant of the forwards state root iterator that does not require an `end_state`.
* Use the `BlockReplayer` when reconstructing states in the database. Use the efficient forwards iterator for frozen states.
* Refactor the iterators to remove `Arc<HotColdDB>` (this seems to be neater than making _everything_ an `Arc<HotColdDB>` as I did in #2431).

Supplying the state roots allow us to avoid building a tree hash cache at all when reconstructing historic states, which saves around 1 second flat (regardless of `slots-per-restore-point`). This is a small percentage of worst-case state load times with 200K validators and SPRP=2048 (~15s vs ~16s) but a significant speed-up for more frequent restore points: state loads with SPRP=32 should be now consistently <500ms instead of 1.5s (a ~3x speedup).

## Additional Info

Required by #2628
@michaelsproul michaelsproul added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Jan 10, 2022
paulhauner
paulhauner approved these changes Jan 27, 2022
View reviewed changes
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

LGTM!

@paulhauner paulhauner added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jan 27, 2022
@michaelsproul
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Jan 27, 2022
@michaelsproul
Implement API for block rewards (#2628)
79141ab 
## Proposed Changes

Add an API endpoint for retrieving detailed information about block rewards.

For information on usage see [the docs](https://github.com/sigp/lighthouse/blob/block-rewards-api/book/src/api-lighthouse.md#lighthouseblock_rewards), and the source.
@michaelsproul
Copy link
Member Author

bors r-

gonna batch

@bors
Copy link

bors bot commented Jan 27, 2022

Canceled.

@michaelsproul
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Jan 27, 2022
@michaelsproul
Implement API for block rewards (#2628)
e70daaa 
## Proposed Changes

Add an API endpoint for retrieving detailed information about block rewards.

For information on usage see [the docs](https://github.com/sigp/lighthouse/blob/block-rewards-api/book/src/api-lighthouse.md#lighthouseblock_rewards), and the source.
@bors
Copy link

bors bot commented Jan 27, 2022

Pull request successfully merged into unstable.

Build succeeded:

@bors bors bot changed the title Implement API for block rewards [Merged by Bors] - Implement API for block rewards Jan 27, 2022
@bors bors bot closed this Jan 27, 2022
@michaelsproul michaelsproul deleted the block-rewards-api branch January 27, 2022 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulhauner paulhauner paulhauner approved these changes

Assignees
No one assigned
Labels
ready-for-merge This PR is ready to merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@michaelsproul @paulhauner