-
Notifications
You must be signed in to change notification settings - Fork 765
feat(compute_ctl): run lease lsn in tokio and in parallel #12786
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
Signed-off-by: Alex Chi Z <chi@neon.tech>
15732e8
to
81d0f5d
Compare
9075 tests run: 8402 passed, 0 failed, 673 skipped (full report)Code coverage* (full report)
* collected from Rust tests only The comment gets automatically updated with the latest test results
5622bf0 at 2025-07-30T20:02:06.957Z :recycle: |
need to asyncify |
ffd12aa
to
2a4feb6
Compare
Signed-off-by: Alex Chi Z <chi@neon.tech>
2a4feb6
to
5622bf0
Compare
cc @alexanderlaw where can I trigger 100 runs of a specific test? I'd like to see if the previous flakyness can be fixed without changing the test case. Thanks! |
Please try something like this:
|
} | ||
} | ||
|
||
/// Acquires lsn lease in a retry loop. Returns the expiration time if a lease is granted. | ||
/// Returns an error if a lease is explicitly not granted. Otherwise, we keep sending requests. | ||
fn acquire_lsn_lease_with_retry( | ||
async fn acquire_lsn_lease_with_retry( |
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 needs spawn_blocking
for wait_timeout_while_pageserver_connstr_unchanged
too.
} | ||
} | ||
|
||
Ok(leases.into_iter().min().flatten()) | ||
let mut stream = futures::stream::iter(jobs).buffer_unordered(MAX_CONCURRENT_LEASE_CONNECTIONS); |
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 fine to just use an unbounded FuturedUnordered
here. If the tenant has many shards, they are mostly likely running on many Pageservers, so the concurrency limiting doesn't really do anything for the server. For the client side, we don't care about opening O(16) concurrent connections.
Problem
Part of LKB-2634
The main motivation of this patch is to make the lease LSN requests in parallel. On that, it seems like a refactor to make it running in async is a good choice.
Summary of changes
futures::stream::buffered_unordered
.test_readonly_node_gc
to issue lease requests only from the compute