dmsolr opened a new pull request, #7675:
URL: https://github.com/apache/shenyu/pull/7675
## Background
Part of the performance program tracked in #7650. Fixes #7648.
`RemoteCatalogLifecycle.build` initialized every MCP target through
`concatMap`,
and `RemoteToolDirectory.discover` discovered targets serially as well, so a
refresh of N endpoints paid the sum of all handshakes plus all `list-tools`
round-trips. This only affects catalog refresh latency, not steady-state tool
invocation.
## What this PR does
Adds an **opt-in bounded concurrency** capability while **keeping the
historical
serialized behaviour as the default**, exactly as the measurement-gated
acceptance criteria require ("Retain serialized behavior if the improvement
is
within noise or lifecycle correctness cannot be maintained").
- `RemoteCatalogLifecycle` gains a `refreshConcurrency` constructor argument
(default `1`, bounded `1..8`). `build` now uses `flatMapSequential(mapper,
refreshConcurrency)`
instead of `concatMap`, which overlaps handshakes up to the bound while
preserving the deterministic target order of the previous serial pipeline.
The concurrency is forwarded to `RemoteToolDirectory.discover`.
- `RemoteToolDirectory.discover` gains a `concurrency` overload (default `1`)
that likewise uses `flatMapSequential`. Cursor pagination **within** a
target
stays sequential because `pages()` still recurses through a single
`flatMap`
chain with its own per-target `seen`/`result` state.
- The production construction site (`ManagedRemoteMcpCatalog`) is left at the
serialized default (`new RemoteCatalogLifecycle(3)`), so runtime behaviour
is
unchanged until measurements justify enabling overlap.
## Correctness preserved
- **Deterministic ordering** — `flatMapSequential` emits results in source
order,
identical to `concatMap`; `concurrency = 1` subscribes one target at a
time and
is behaviourally equivalent to the previous serial code.
- **Duplicate / limit validation** — unchanged; server-name, per-page, and
total
tool-limit checks still run in `build`.
- **Atomic publication** — a directory is still published only after the
whole
generation builds and passes the reserved-name collision check.
- **No client leak** — allocation and ownership remain atomic under the
lifecycle lock, so a failed or cancelled parallel refresh still closes
*every*
client added to the candidate generation via `abandon` → `closeIdle`, and
never
partially publishes.
## Tests
New deterministic coverage (no timing/flakiness dependence):
- `RemoteToolDirectoryTest`
- serial vs. bounded discovery produce identical directories
- bounded concurrency genuinely overlaps independent targets (shared
`CountDownLatch` barrier released only if all targets are subscribed at
once)
- default overload stays serialized
- duplicate server name rejected regardless of concurrency
- total tool limit enforced under bounded concurrency
- concurrency outside the bounded range rejected
- `RemoteCatalogLifecycleTest`
- refresh concurrency budget validation
- bounded parallel refresh publishes every target deterministically
- parallel refresh closes every allocated client when one target's
handshake
fails (no leak, no partial publication)
Full module suite: `Tests run: 485, Failures: 0, Errors: 0` with checkstyle
clean.
## Measurement note
This environment has no load generator or JFR profiling harness, so the
p95/p99
refresh-latency baseline for 1/4/8 targets requested in the issue is **not**
included here. The change is source-grounded and defaults to serialized; the
bounded knob is exposed so a benchmark can compare small concurrency values
and
decide whether to enable it in production.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
