request: replace try_join_all with JoinSet in plan handlers

[eduralph]

Closes #534. Supersedes #535 — offered so the fix isn't blocked on rebase logistics. It carries @pingyu's two commits unchanged, authorship and sign-off preserved, plus one follow-up commit.

Why superseding: #535's merge conflict consists entirely of its dependency-pin commit for rust-toolchain 1.84.1, obsolete since #530 moved the toolchain to 1.93. With that commit dropped, the change rebases onto master with no conflicts. @pingyu — if you'd rather rebase #535 itself, say the word and I'll close this one.

The follow-up commit: raises tokio's floor to 1.21 (JoinSet stabilized there; tokio = "1" permitted resolutions that can't compile this crate), applies cargo fmt, and adds the doc comment the #535 review asked for — stating the contract that an error return means no further effects: remaining tasks are aborted, never detached (the old detach-on-error could complete stray writes after an observed failure, beyond the #534 panic).

Verification: cargo test --lib 67/67 (including the spawn-order test); clippy + fmt clean at 1.93; and a differential parity harness (getwyrd/client-rust-test) driving client-go and client-rust through identical multi-region scenarios shows no wire-visible behavior change.

Summary by CodeRabbit

• Bug Fixes

  • Improved reliability for concurrent execution across multiple shards or stores.
  • Ensured results are returned in the same order as tasks were started.
  • Added safer handling for task failures, cancellations, and panics, preventing partial/incorrect aggregation.

• Chores

  • Updated the Tokio runtime dependency to 1.21.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kennytm for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 97c6ddb6-3912-4166-8c35-b5ea8a875dd8

📥 Commits

Reviewing files that changed from the base of the PR and between 95f74d2 and 4b45517.

📒 Files selected for processing (2)

• Cargo.toml
• src/request/plan.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• Cargo.toml
• src/request/plan.rs

---

📝 Walkthrough

Walkthrough

Changes

Concurrent request aggregation

Layer / File(s)	Summary
JoinSet collector and dependency update Cargo.toml, src/request/plan.rs	Tokio is updated to 1.21, and a shared helper collects indexed JoinSet results, logs join failures, shuts down remaining tasks, and returns Error::JoinError.
Multi-region shard aggregation src/request/plan.rs	RetryableMultiRegion uses JoinSet for shard tasks and restores results by shard index.
All-store aggregation and ordering validation src/request/plan.rs	RetryableAllStores uses the collector for per-store tasks, with a test covering spawn-order preservation across delayed tasks.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RequestPlan
  participant JoinSet
  participant WorkerTasks
  RequestPlan->>JoinSet: spawn indexed shard or store tasks
  JoinSet->>WorkerTasks: await task completion
  WorkerTasks-->>JoinSet: return indexed results
  JoinSet-->>RequestPlan: return ordered results or JoinError

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title concisely describes the main change: replacing try_join_all with JoinSet in plan handlers.
Linked Issues check	✅ Passed	The JoinSet refactor addresses #534 by tracking spawned tasks and shutting down remaining work on errors in both handlers.
Out of Scope Changes check	✅ Passed	The changes stay focused on the JoinSet migration, tokio version bump, and related test coverage with no unrelated additions.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[eduralph]

For the record, the earlier integration-test-txn failure was the pre-existing flake documented in #516 — same test (txn_cleanup_2pc_locks), same signature (hang until nextest's terminate kills it), observed there in January on #515 before this change existed. Mechanically, this PR only changes behavior on the join-failure and shard-iteration-error paths; that test's return-style failpoints are handled inside the per-shard tasks, so the success-path wait is identical to the old try_join_all — a hang in one would hang in both. It also passes repeatedly (3/3, <1s each) against a local api-v2 cluster on this branch, and the retriggered run is now green across the board.