bitflicker64 commented on PR #3416: URL: https://github.com/apache/hugegraph/pull/3416#issuecomment-6464863712
**Review summary, R1 at `ec63a6c7`: 10/10, no confirmed regressions** Two independent whole-PR reviews (Codex and Claude) found no defects introduced by this PR. Copilot and an AGY lane are still pending; this comment will be updated if they report something new. - The blocking concern from the earlier review at `dcbed16b` is fixed at this head. `getAsyncStub(String)` no longer rebuilds with `IntStream.parallel()`, and both stub pools are now built in order on the caller's thread before `publishStubs`. - Partial-pool cleanup also changed. A `createChannel` failure on base was swallowed on pool threads, so a pool containing null channels could be published. Now the partial pool is cleaned up and the exception is rethrown. - Locally on JDK 17: `HugeSecurityManagerTest` 5/5, `AbstractGrpcClientTest` and `AbstractGrpcClientErrorTest` 10/10, and `HstoreChannelSecurityTest` 4/4. As a negative control, `HstoreChannelSecurityTest` fails 3/4 against the base `AbstractGrpcClient` with "Not allowed to access thread group via Gremlin". `git diff --check` is clean. Limits: the new tests only construct stubs. Nobody ran a real PD/Store restart with an RPC read-back, or checked whether the first RPC on a cold shared Netty event loop creates threads from a Gremlin thread; `docs/server-runtime.md` places both out of scope. The full Maven suite was not run. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
