-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[Fix] Clear cached validator requests on disconnect #3254
Conversation
node/bft/src/helpers/cache.rs
Outdated
/// Note: we don't clear this map based on time but only on peer disconnect. | ||
/// This is sufficient to avoid infinite growth as the committee has a fixed number | ||
/// of members. |
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.
/// Note: we don't clear this map based on time but only on peer disconnect. | |
/// This is sufficient to avoid infinite growth as the committee has a fixed number | |
/// of members. | |
/// The map of IPs to the number of validators requests |
This shouldn't be explained within the struct member comment, but instead on the invocation of clear_outbound_validators_requests
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.
LGTM,
The only use of seen_outbound_validators_requests
is when we check that an incoming ValidatorsResponse
corresponds to an existing ValidatorsRequest
.
…ache"" This reverts commit 0829e52.
This PR clears the validator requests cache map for a particular peer on disconnect. This ensures the collection is periodically cleaned up despite not having a time-based expiry and is sufficient to avoid infinite growth as the committee size is bounded (but the IP may change).
A small aside for future discussion: when time permits, we may want to revisit our approach to cache invalidation both in the gateway and the router. One option would be to use an LRU with a fixed size that we clear on specific state transitions (e.g. on round increment).