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

Restore Log on Error & Spawn Blocking in Streamer #5585

Merged
merged 5 commits into from
Apr 16, 2024
Merged

Restore Log on Error & Spawn Blocking in Streamer #5585

merged 5 commits into from
Apr 16, 2024

Conversation

ethDreamer
Copy link
Member

Issue Addressed

  • Some error cases in rpc_methods were no longer logging the results on error.
  • The BeaconBlockStreamer now runs the task of loading the blocks in a blocking thread as DB reads are slow

@realbigsean realbigsean added the ready-for-review The code is ready for review label Apr 16, 2024
realbigsean
realbigsean requested changes Apr 16, 2024
View reviewed changes
Copy link
Member

@realbigsean realbigsean left a comment

Choose a reason for hiding this comment

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

I like the logging message structure being standard across all these responses, howeve looks like blobs by root was missed because it required no other logging change, could I request its message be updated to:

BlobsByRoot outgoing response processed

beacon_node/network/src/network_beacon_processor/rpc_methods.rs Outdated Show resolved Hide resolved
beacon_node/network/src/network_beacon_processor/rpc_methods.rs Outdated Show resolved Hide resolved
beacon_node/network/src/network_beacon_processor/rpc_methods.rs Outdated Show resolved Hide resolved
beacon_node/network/src/network_beacon_processor/rpc_methods.rs Outdated Show resolved Hide resolved
@ethDreamer
Copy link
Member Author

I like the logging message structure being standard across all these responses, howeve looks like blobs by root was missed because it required no other logging change, could I request its message be updated to:

BlobsByRoot outgoing response processed

done!

@realbigsean
Copy link
Member

@Mergifyio queue

@realbigsean realbigsean added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Apr 16, 2024
@mergify Mergify
Copy link

mergify bot commented Apr 16, 2024 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at f689898

mergify bot added a commit that referenced this pull request Apr 16, 2024
@mergify
Merge of #5585
2c4b379
@mergify mergify bot mentioned this pull request Apr 16, 2024
Closed
5 tasks
@mergify mergify bot merged commit f689898 into sigp:unstable Apr 16, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@realbigsean realbigsean realbigsean 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.
2 participants
@ethDreamer @realbigsean