zhaohai666 opened a new pull request, #1405:
URL: https://github.com/apache/rocketmq-clients/pull/1405
# fix(nodejs): create the first-settings promise before any await in startup
## Summary
`BaseClient.#startup()` can hang for exactly **5 minutes** (or forever) with
two or more
topics, because the promise resolvers for "wait for the first
`onSettingsCommand`" are
registered only **after** `updateRoutes()` has finished — while the first
settings callback
can already fire **inside** `updateRoutes()`. This PR closes that race by
creating the
promise (and its resolvers) before any `await` in the startup flow.
**Branch**: `fix/nodejs-startup-settings-race` (1 commit,
`nodejs/src/client/BaseClient.ts`,
+17/−9)
## Root cause
The first `Settings` command is produced on the path that `startup()` itself
awaits:
```
#startup()
└─ await updateRoutes()
└─ #fetchTopicRoute(topic)
└─ await rpcClientManager.queryRoute() (network RTT)
└─ [synchronous] getTelemetrySession(ep).syncSettings()
└─ TelemetrySession opens the gRPC bidirectional stream
└─ write(settingsCommand) (fire-and-forget,
not awaited)
```
The proxy answers a telemetry stream with a pushed `Settings` right away,
which is delivered
asynchronously: `TelemetrySession.#onData` → `BaseClient.onSettingsCommand` →
`this.#startupResolve?.()`.
The resolvers, however, are only registered *later* in `#startup()`:
```ts
await this.updateRoutes();
// ... register the 30s/5m/10s timers ...
if (this.topics.size > 0) {
await new Promise((resolve, reject) => {
this.#startupReject = reject;
this.#startupResolve = resolve; // <-- registered here, too late
});
}
```
With **two or more topics**, `updateRoutes()` yields the event loop between
the topics
(`await` on the next `queryRoute`, network RTT) and inside the retry backoff
(`await this.sleep(backoffMs)`). A server-pushed `Settings` arriving in one
of those windows
hits `this.#startupResolve === undefined`, so the `?.()` call silently does
nothing and the
first callback is lost. `startup()` then hangs on the await until the
periodic settings sync
it registered (every **5 minutes**) fires the next `onSettingsCommand` — or
forever, if that
sync fails.
With a single topic the code path from the last `queryRoute` resolution to
the resolver
registration is purely synchronous (microtask chain, no macrotask can
interleave), which is
why the bug only shows up with multiple topics.
The same race applies to `onUnknownCommand` (`this.#startupReject?.()`),
whose rejection is
dropped by the identical window.
## Fix
Create the promise before any `await` in `#startup()`, so the resolvers are
in place no
matter when the first settings callback arrives:
```ts
async #startup() {
// Create the first-settings promise before any await: updateRoutes()
below opens
// telemetry sessions and syncs settings while it is still iterating the
topics, so a
// server-pushed Settings callback can fire long before the code below
reaches the
// await. Registering the resolvers only at the await point would silently
drop that
// first callback and leave startup hanging until the periodic settings
sync fires
// (5 minutes later).
const firstSettingsPromise = this.topics.size > 0
? new Promise<void>((resolve, reject) => {
this.#startupReject = reject;
this.#startupResolve = resolve;
})
: undefined;
// ... updateRoutes / timers, unchanged ...
if (firstSettingsPromise) {
await firstSettingsPromise;
}
this.#startupReject = undefined;
this.#startupResolve = undefined;
this.#running = true;
}
```
The rejection path (`onUnknownCommand` → `#startupReject`) is covered by the
same change.
## Reproduction and verification (live cluster)
Reproduced against a live RocketMQ 5.5.1 cluster (remoting broker 10911 +
gRPC proxy 8081)
with a stress script that starts a `Producer` with 4 topics and 10
consecutive
startup/shutdown rounds, recording each round's duration. A race hit shows
up as a round
taking ~300s — the 5-minute periodic settings-sync timer, which is the only
thing that can
still deliver an `onSettingsCommand` after the first one was dropped.
| Code version | Result |
|---|---|
| `master` (unfixed) | **Hit on round 1**: startup took **300099ms** —
exactly the 5-minute periodic sync timer, confirming the first callback was
dropped and startup slept until the timer fired |
| this fix (20 rounds over two runs) | all rounds **22–113ms**, zero hangs |
The `300099ms` figure is the smoking gun: it is not "slow", it is pinned to
`setInterval(5 * 60000)` from the client's own startup flow.
Reproduction requirements: **two or more topics** (the `updateRoutes()` loop
yields the
event loop between topics); a single topic cannot reproduce because the path
is a synchronous
microtask chain.
## Testing
- `tsc -p tsconfig.prod.json --noEmit` and the full `tsconfig.test.json`
build: pass
- `eslint src/client/BaseClient.ts`: 0 errors (3 pre-existing jsdoc warnings)
- Offline test suite: **150/150 pass**
(`node --test --test-concurrency=1 --test-force-exit`, integration tests
excluded)
- Live-cluster A/B verification as described above
--
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]
