change(state): Remove active_value field from ChainTipSender
#4175
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jvff
added
A-rust
jvff
changed the title
active_value field from ChainTipSender
teor2345
reviewed
Apr 24, 2022
I ran the unit tests and I started the full sync test but only ran for a few minutes to check if the chain tip was being updated. I can't run more than that locally because I'm having some internet issues :( |
|
@Mergifyio update |
✅ Branch has been successfully updated |
teor2345
reviewed
Apr 25, 2022
Record the fields using a helper method. This reduces duplicate code and prepares for removing the `active_value` field from the type, because the replacement for it has to be atomically read to avoid a deadlock when reading from the `watch::Sender` endpoint.
Now that Tokio has been updated to a version in which `watch::Sender` has a `borrow` method, we can remove the `active_value` field. To prevent a deadlock like the one that happened with the synchronizer some time ago, the method instrumentation was slightly refactored to have the fields recorded in helper methods that avoid obtaining a read-lock twice.
They are resolved with the current changes.
This is a minor optimization to remove an unnecessary second call to `tracing::Span::current()`.
Explain why a binding can't be created for the old tip reference.
teor2345
force-pushed
the
remove-active-value-field
branch
from
9bf6c07 to
4e871e5
Compare
|
Does this close #2573? |
40 tasks
47 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The
ChainTipSendertype had an extra field as a workaround to when Zebra was using an older version of Tokio that didn't have thewatch::Sender::borrow()method. Zebra now depends on a newer version of Tokio, so that field can be removed.This was a cleanup I did some time ago, and I was afraid I'd forget to push this.
Depends-On: #4198
Solution
Remove the field, replacing the usages with
watcher::Sender::borrow().As part of the changes, I also simplified the instrumentation attribute a bit.
Review
Reviewer Checklist
Follow Up Work