Skip to content

Commit

Permalink
storcon: send heartbeats concurrently (#10954)
Browse files Browse the repository at this point in the history
## Problem

While looking at logs I noticed that heartbeats are sent sequentially.
The loop polling the UnorderedSet is at the wrong level of identation.
Instead of doing it after we have the full set, we did after each entry.

## Summary of Changes

Poll the UnorderedSet properly.
  • Loading branch information
VladLazar authored Feb 25, 2025
1 parent 6621be6 commit 5d17640
Showing 1 changed file with 26 additions and 26 deletions.
52 changes: 26 additions & 26 deletions storage_controller/src/heartbeater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,21 +223,21 @@ impl HeartBeat<Node, PageserverState> for HeartbeaterTask<Node, PageserverState>
Some((*node_id, status))
}
});
}

loop {
let maybe_status = tokio::select! {
next = heartbeat_futs.next() => {
match next {
Some(result) => result,
None => { break; }
}
},
_ = self.cancel.cancelled() => { return Err(HeartbeaterError::Cancel); }
};
loop {
let maybe_status = tokio::select! {
next = heartbeat_futs.next() => {
match next {
Some(result) => result,
None => { break; }
}
},
_ = self.cancel.cancelled() => { return Err(HeartbeaterError::Cancel); }
};

if let Some((node_id, status)) = maybe_status {
new_state.insert(node_id, status);
}
if let Some((node_id, status)) = maybe_status {
new_state.insert(node_id, status);
}
}

Expand Down Expand Up @@ -363,21 +363,21 @@ impl HeartBeat<Safekeeper, SafekeeperState> for HeartbeaterTask<Safekeeper, Safe
Some((*node_id, status))
}
});
}

loop {
let maybe_status = tokio::select! {
next = heartbeat_futs.next() => {
match next {
Some(result) => result,
None => { break; }
}
},
_ = self.cancel.cancelled() => { return Err(HeartbeaterError::Cancel); }
};
loop {
let maybe_status = tokio::select! {
next = heartbeat_futs.next() => {
match next {
Some(result) => result,
None => { break; }
}
},
_ = self.cancel.cancelled() => { return Err(HeartbeaterError::Cancel); }
};

if let Some((node_id, status)) = maybe_status {
new_state.insert(node_id, status);
}
if let Some((node_id, status)) = maybe_status {
new_state.insert(node_id, status);
}
}

Expand Down

1 comment on commit 5d17640

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7862 tests run: 7463 passed, 13 failed, 386 skipped (full report)


Failures on Postgres 16

  • test_throughput[github-actions-selfhosted-50-pipelining_config7-30-100-128-batchable {'max_batch_size': 1, 'execution': 'tasks', 'mode': 'pipelined'}]: release-x86-64-with-lfc
  • test_throughput[github-actions-selfhosted-50-pipelining_config8-30-100-128-batchable {'max_batch_size': 2, 'execution': 'concurrent-futures', 'mode': 'pipelined'}]: release-x86-64-with-lfc
  • test_throughput[github-actions-selfhosted-50-pipelining_config9-30-100-128-batchable {'max_batch_size': 2, 'execution': 'tasks', 'mode': 'pipelined'}]: release-x86-64-with-lfc
  • test_throughput[github-actions-selfhosted-50-pipelining_config10-30-100-128-batchable {'max_batch_size': 4, 'execution': 'concurrent-futures', 'mode': 'pipelined'}]: release-x86-64-with-lfc
  • test_throughput[github-actions-selfhosted-50-pipelining_config11-30-100-128-batchable {'max_batch_size': 4, 'execution': 'tasks', 'mode': 'pipelined'}]: release-x86-64-with-lfc
  • test_throughput[github-actions-selfhosted-50-pipelining_config12-30-100-128-batchable {'max_batch_size': 8, 'execution': 'concurrent-futures', 'mode': 'pipelined'}]: release-x86-64-with-lfc
  • test_throughput[github-actions-selfhosted-50-pipelining_config13-30-100-128-batchable {'max_batch_size': 8, 'execution': 'tasks', 'mode': 'pipelined'}]: release-x86-64-with-lfc
  • test_throughput[github-actions-selfhosted-50-pipelining_config14-30-100-128-batchable {'max_batch_size': 16, 'execution': 'concurrent-futures', 'mode': 'pipelined'}]: release-x86-64-with-lfc
  • test_throughput[github-actions-selfhosted-50-pipelining_config15-30-100-128-batchable {'max_batch_size': 16, 'execution': 'tasks', 'mode': 'pipelined'}]: release-x86-64-with-lfc
  • test_throughput[github-actions-selfhosted-50-pipelining_config16-30-100-128-batchable {'max_batch_size': 32, 'execution': 'concurrent-futures', 'mode': 'pipelined'}]: release-x86-64-with-lfc
  • test_throughput[github-actions-selfhosted-50-pipelining_config17-30-100-128-batchable {'max_batch_size': 32, 'execution': 'tasks', 'mode': 'pipelined'}]: release-x86-64-with-lfc
  • test_throughput[github-actions-selfhosted-50-pipelining_config5-30-100-128-batchable {'mode': 'serial'}]: release-x86-64-with-lfc
  • test_throughput[github-actions-selfhosted-50-pipelining_config6-30-100-128-batchable {'max_batch_size': 1, 'execution': 'concurrent-futures', 'mode': 'pipelined'}]: release-x86-64-with-lfc
# Run all failed tests locally:
scripts/pytest -vv -n $(nproc) -k "test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config7-30-100-128-batchable {'max_batch_size': 1, 'execution': 'tasks', 'mode': 'pipelined'}] or test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config8-30-100-128-batchable {'max_batch_size': 2, 'execution': 'concurrent-futures', 'mode': 'pipelined'}] or test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config9-30-100-128-batchable {'max_batch_size': 2, 'execution': 'tasks', 'mode': 'pipelined'}] or test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config10-30-100-128-batchable {'max_batch_size': 4, 'execution': 'concurrent-futures', 'mode': 'pipelined'}] or test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config11-30-100-128-batchable {'max_batch_size': 4, 'execution': 'tasks', 'mode': 'pipelined'}] or test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config12-30-100-128-batchable {'max_batch_size': 8, 'execution': 'concurrent-futures', 'mode': 'pipelined'}] or test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config13-30-100-128-batchable {'max_batch_size': 8, 'execution': 'tasks', 'mode': 'pipelined'}] or test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config14-30-100-128-batchable {'max_batch_size': 16, 'execution': 'concurrent-futures', 'mode': 'pipelined'}] or test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config15-30-100-128-batchable {'max_batch_size': 16, 'execution': 'tasks', 'mode': 'pipelined'}] or test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config16-30-100-128-batchable {'max_batch_size': 32, 'execution': 'concurrent-futures', 'mode': 'pipelined'}] or test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config17-30-100-128-batchable {'max_batch_size': 32, 'execution': 'tasks', 'mode': 'pipelined'}] or test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config5-30-100-128-batchable {'mode': 'serial'}] or test_throughput[release-pg16-github-actions-selfhosted-50-pipelining_config6-30-100-128-batchable {'max_batch_size': 1, 'execution': 'concurrent-futures', 'mode': 'pipelined'}]"
Flaky tests (8)

Postgres 17

Postgres 14

Code coverage* (full report)

  • functions: 32.8% (8632 of 26298 functions)
  • lines: 48.6% (72622 of 149428 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
5d17640 at 2025-02-25T12:20:10.479Z :recycle:

Please sign in to comment.