-
Notifications
You must be signed in to change notification settings - Fork 10
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
Refactor re-execution #658
Refactor re-execution #658
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
} | ||
|
||
// emit block event and logs, only after we successfully commit the data | ||
e.blocksPublisher.Publish(events.Block()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who depends on this publisher? They need to be aware if a crash happens before this call and after the commit, then the subscribe will miss the data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only notifies the streaming api, that a new block is available.
If a crash happens after commit and before this (no errrors are expected but there could be a panic or outage), the clients on the streaming api will be disconnected, and they will need to handle getting missing blocks and re-subscribing once the GW is back up. This should be already standard procedure if a disconnect happens.
79c3012
into
feature/local-tx-reexecution
Description
I split up the function a bit, to hopefully make it a bit clearer.
I also noticed that there was a bug where if
events.Empty()
then the batch containing the new cadence height never got committed. This should be fixed.For contributor use:
master
branchFiles changed
in the Github PR explorer