You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "tisonkun (via GitHub)" <gi...@apache.org> on 2026/09/08 08:06:22 UTC
[PR] perf(azure-storage): coalesce user delegation key requests [opendal-reqsign]
tisonkun opened a new pull request, #870:
URL: https://github.com/apache/opendal-reqsign/pull/870
## Summary
Concurrent `UserDelegationSasGranter` cache misses can issue identical user delegation key requests. Coalesce those requests with `asyncband::singleflight::Group`, partitioned by account, endpoint, source-token fingerprint, service version, and the requested key start and expiry.
Keep the bounded key cache and generate each caller's SAS separately, checking its expiration and key coverage after waiting. Use `try_work` so a failed key request returns its original error to that caller while waiting callers can retry; cancellation also lets a waiter take over.
## Testing
Four deterministic tests cover sharing across different grants, independent requests, failure/cancellation recovery, and per-caller validity checks. Three fail against the original implementation because it starts duplicate HTTP requests; all pass with this change.
- `cargo fmt --all -- --check`
- `cargo clippy --workspace --all-targets --all-features -- -D warnings`
- `cargo test --no-fail-fast`
- `cargo test --doc --all-features --workspace`
- `cargo build --manifest-path reqsign/Cargo.toml --target wasm32-unknown-unknown --no-default-features --features azure`
- `hawkeye check`
Concurrency tests use a controlled HTTP transport. Live Azure validation has not been performed.
AI assisted with the implementation, tests, and PR draft.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: unify async synchronization with AsyncBand [opendal-reqsign]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #870:
URL: https://github.com/apache/opendal-reqsign/pull/870#discussion_r3956080409
##########
context/http-send-reqwest/src/lib.rs:
##########
@@ -92,9 +92,9 @@
//! // Use the custom client
//! let http_send = ReqwestHttpSend::new(client);
//! ```
-use bytes::Bytes;
#[cfg(target_arch = "wasm32")]
-use futures_channel::oneshot;
+use asyncband::oneshot;
+use bytes::Bytes;
#[cfg(not(target_arch = "wasm32"))]
use http_body_util::BodyExt;
use reqsign_core::{Error, HttpSend, Result};
Review Comment:
`asyncband::oneshot` is imported unconditionally, but `asyncband` is only declared under `[target.'cfg(target_arch = \"wasm32\")'.dependencies]` in this crate. This will fail to compile for non-wasm targets. Fix by gating the `use asyncband::oneshot;` import behind `#[cfg(target_arch = \"wasm32\")]` (matching the dependency), or make `asyncband` a non-conditional dependency if it’s needed on all targets.
##########
services/azure-storage/src/user_delegation_sas.rs:
##########
@@ -1346,6 +1385,249 @@ mod tests {
assert_eq!(http.calls.load(Ordering::SeqCst), 3);
}
+ #[tokio::test]
+ async fn coalesces_user_delegation_key_requests_across_grants() {
+ let now = timestamp("2030-01-01T00:00:00Z");
+ let gate = Arc::new(Semaphore::new(0));
+ let http = MockUserDelegationHttpSend::new([delegation_key_response(
+ now,
+ now + MAX_USER_DELEGATION_LIFETIME,
+ "oid",
+ "tid",
+ "a2V5",
+ )])
+ .with_response_gate(gate.clone());
+ let ctx = Context::new().with_http_send(http.clone());
+ let source = bearer("source");
+ let first = UserDelegationSasGranter::new(
+ "account",
+ blob_grant("first", UserDelegationSasPermissions::READ),
+ )
+ .with_time(now);
+ let second = first
+ .clone()
+ .with_time(now + Duration::from_millis(500))
+ .with_grant(blob_grant("second", UserDelegationSasPermissions::WRITE));
+ let mut first_request =
+ Box::pin(first.grant_credential(&ctx, &source, Some(Duration::from_secs(300))));
+ let mut second_request =
+ Box::pin(second.grant_credential(&ctx, &source, Some(Duration::from_secs(600))));
+ let mut cancelled_waiter =
+ Box::pin(first.grant_credential(&ctx, &source, Some(Duration::from_secs(300))));
+ let mut cx = TaskContext::from_waker(Waker::noop());
Review Comment:
`std::task::Context::from_waker` expects a `&Waker`, but `Waker::noop()` produces a `Waker` value; passing it directly is a type mismatch. Store the noop waker in a local and pass it by reference (or use a `noop_waker_ref` helper) so the `Context` holds a valid borrowed waker with an appropriate lifetime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: unify async synchronization with AsyncBand [opendal-reqsign]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #870:
URL: https://github.com/apache/opendal-reqsign/pull/870
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org