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.

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

[consensus] Make CommitRange inclusive and use in commit syncer #17788

Merged
merged 4 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address review comments
  • Loading branch information
arun-koshy committed Jun 4, 2024
commit 98c5b59f73fa5ef44f2f0df0f5165aaf63715785
2 changes: 1 addition & 1 deletion consensus/core/src/authority_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ impl<C: CoreThreadDispatcher> NetworkService for AuthorityService<C> {
fail_point_async!("consensus-rpc-response");

// Compute an inclusive end index and bound the maximum number of commits scanned.
let inclusive_end = (commit_range.end()).min(
let inclusive_end = commit_range.end().min(
commit_range.start() + self.context.parameters.commit_sync_batch_size as CommitIndex
- 1,
);
Expand Down
21 changes: 17 additions & 4 deletions consensus/core/src/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,10 +565,14 @@ impl CommitRange {
*self.0.end()
}

/// Check if the provided range is sequentially after this range with the same
/// range length.
/// Check whether the two ranges have the same size.
pub(crate) fn is_equal_size(&self, other: &Self) -> bool {
self.0.size_hint() == other.0.size_hint()
}

/// Check if the provided range is sequentially after this range.
pub(crate) fn is_next_range(&self, other: &Self) -> bool {
self.0.size_hint() == other.0.size_hint() && self.end() + 1 == other.start()
&self.end() + 1 == other.start()
}
}

Expand Down Expand Up @@ -685,6 +689,7 @@ mod tests {

#[tokio::test]
async fn test_commit_range() {
telemetry_subscribers::init_for_testing();
let range1 = CommitRange::new(1..=5);
let range2 = CommitRange::new(2..=6);
let range3 = CommitRange::new(5..=10);
Expand All @@ -694,11 +699,19 @@ mod tests {
assert_eq!(range1.start(), 1);
assert_eq!(range1.end(), 5);

tracing::debug!("test");

// Test next range check
assert!(!range1.is_next_range(&range2));
assert!(!range1.is_next_range(&range3));
assert!(range1.is_next_range(&range4));
assert!(!range1.is_next_range(&range5));
assert!(range1.is_next_range(&range5));

// Test equal size range check
assert!(range1.is_equal_size(&range2));
assert!(!range1.is_equal_size(&range3));
assert!(range1.is_equal_size(&range4));
assert!(!range1.is_equal_size(&range5));

// Test range ordering
assert!(range1 < range2);
Expand Down
2 changes: 1 addition & 1 deletion consensus/core/src/leader_schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ impl LeaderSchedule {
// preceding commit range of the old swap table.
if *old_commit_range != CommitRange::default() {
assert!(
old_commit_range.is_next_range(new_commit_range),
old_commit_range.is_next_range(new_commit_range) && old_commit_range.is_equal_size(new_commit_range),
"The new LeaderSwapTable has an invalid CommitRange. Old LeaderSwapTable {old_commit_range:?} vs new LeaderSwapTable {new_commit_range:?}",
);
}
Expand Down