bitflicker64 opened a new issue, #3125:
URL: https://github.com/apache/hugegraph/issues/3125
## HugeGraph version
Current `master`, verified at commit
`b9710a7b0319ace41dd3c4ff5fb1165f53ee538e`.
## Problem
`AbstractGrpcClient` creates an array of 32 `ManagedChannel` instances per
target, apparently to distribute requests across a channel pool. However, both
the blocking-stub and asynchronous-stub initialization loops bind every array
entry to the same channel.
The loop variable is `i`, but each iteration selects `channels[index]`,
where `index` was calculated once before the loop:
```java
int index = (int) (l & (concurrency - 1));
HgPair<ManagedChannel, AbstractBlockingStub>[] value = new
HgPair[concurrency];
IntStream.range(0, concurrency).forEach(i -> {
ManagedChannel channel = channels[index];
AbstractBlockingStub stub = getBlockingStub(channel);
value[i] = new HgPair<>(channel, stub);
});
```
The asynchronous path contains the same indexing error:
```java
IntStream.range(0, concurrency).parallel().forEach(i -> {
ManagedChannel channel = channels[index];
AbstractAsyncStub stub = getAsyncStub(channel);
value[i] = new HgPair<>(channel, stub);
});
```
Source:
https://github.com/apache/hugegraph/blob/b9710a7b0319ace41dd3c4ff5fb1165f53ee538e/hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/grpc/AbstractGrpcClient.java#L93-L161
## Actual behavior
Although `getChannels(target)` creates 32 channels, all entries in a newly
initialized blocking or asynchronous stub array reference whichever single
channel happened to be selected by the first call's `index`.
Subsequent calls rotate across the stub-array indexes, but those stubs all
use the same underlying `ManagedChannel`. The remaining 31 created channels are
unused by that stub pool.
Consequences include:
- The intended channel-level request distribution does not occur.
- All traffic for that client and target is concentrated on one channel.
- Creating 31 additional channels consumes resources without providing
concurrency or isolation.
- A problem affecting the selected channel affects every entry in the
corresponding stub pool.
This defect is independent of DNS caching or Kubernetes Store replacement
behavior and can be verified without a cluster.
## Expected behavior
Each stub-pool entry should be paired with its corresponding channel-pool
entry:
```java
ManagedChannel channel = channels[i];
```
After initialization, the 32 stub entries should collectively reference all
32 channels created for the target.
## Minimal fix
Change both initialization paths from:
```java
ManagedChannel channel = channels[index];
```
to:
```java
ManagedChannel channel = channels[i];
```
## Suggested test
Add a unit test using a test subclass of `AbstractGrpcClient` that records
the `ManagedChannel` passed to `getBlockingStub(ManagedChannel)` and
`getAsyncStub(ManagedChannel)` during pool initialization.
For a new target, assert that:
- each stub factory is called `concurrency` times;
- the recorded channel identities contain `concurrency` distinct entries; and
- repeated public stub lookups rotate across stubs backed by those distinct
channels.
--
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]
