-
Notifications
You must be signed in to change notification settings - Fork 636
feat: make handshake cancelable (backport #857) #1013
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
Conversation
it'll make the handshake work with graceful shutdown(see: cosmos/cosmos-sdk#16202) handshake could be a long running process if there are many local blocks to replay, for example we use it to do profiling. Hope we can backport this to 0.34.x. --- - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments
@@ -265,7 +271,7 @@ func (h *Handshaker) Handshake(proxyApp proxy.AppConns) error { | |||
} | |||
|
|||
// Replay blocks up to the latest in the blockstore. | |||
_, err = h.ReplayBlocks(h.initialState, appHash, blockHeight, proxyApp) | |||
appHash, err = h.ReplayBlocksWithContext(ctx, h.initialState, appHash, blockHeight, proxyApp) |
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.
I know in future branches the original here re-writes appHash
, but not here. I'd leave it as is unless we understand why we are making that change.
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.
@yihuang any thoughts?
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.
I think it's kind of bug fix, previously the log don't output latest hash, but I'm ok that we change that in a separate PR
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.
hi @sergio-mena , do you want me to revert this line?
This makes the log outputs the latest hash, and it only affects the log content.
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 pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
it'll make the handshake work with graceful shutdown(see: cosmos/cosmos-sdk#16202) handshake could be a long running process if there are many local blocks to replay, for example we use it to do profiling. Hope we can backport this to 0.34.x. --- - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments
it'll make the handshake work with graceful shutdown(see: cosmos/cosmos-sdk#16202)
handshake could be a long running process if there are many local blocks to replay, for example we use it to do profiling.
.changelog
(we use unclog to manage our changelog)docs/
orspec/
) and code commentsPR checklist
.changelog
(we use unclog to manage our changelog)docs/
orspec/
) and code comments