- Notifications
You must be signed in to change notification settings - Fork 102
feat(bigtable): expose a metric to track the number of outstanding rpcs (unary , streaming) in channel pool #2696
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
...oud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java Outdated Show resolved Hide resolved
...e/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ChannelPoolMetricsTracer.java Show resolved Hide resolved
...c/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/OutstandingRpcsMetricTracker.java Outdated Show resolved Hide resolved
...e/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ChannelPoolMetricsTracer.java Show resolved Hide resolved
...c/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/OutstandingRpcsMetricTracker.java Outdated Show resolved Hide resolved
...le-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableChannelPool.java Show resolved Hide resolved
| | ||
| @Override | ||
| public int getOutstandingStreamingRpcs() { | ||
| return 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return outstandingStreamingRpcs.get(); ?
And maybe add a test case? I'm a little concerned that this is not caught by any tests :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no tests for ChannelPool. added a test
...le/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java Show resolved Hide resolved
...e/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ChannelPoolMetricsTracer.java Show resolved Hide resolved
...able/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableTransportChannelProvider.java Outdated Show resolved Hide resolved
| Aggregation.explicitBucketHistogram( | ||
| ImmutableList.of( | ||
| 0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 12.0, 14.0, 16.0, 18.0, 20.0, | ||
| 25.0, 30.0, 35.0, 40.0, 45.0, 50.0, 60.0, 70.0, 80.0, 90.0, 100.0, 110.0)); // Added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, remove the // Added comment in the end?
Do we need to go over 100? cause the grpc limit is 100.
Also wondering if we should have equal size for the bucket, maybe [0, 2, 4, 6, 8 ..., 100]? or [0, 5, 10, 15 .. 100]?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the queuing of the stream is deep inside the http2 lib in grpc netty. so we could have more than 100 here.
...loud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableChannelObserver.java Outdated Show resolved Hide resolved
...le-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableChannelPool.java Outdated Show resolved Hide resolved
...le-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableChannelPool.java Show resolved Hide resolved
...le-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableChannelPool.java Outdated Show resolved Hide resolved
...le-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableChannelPool.java Outdated Show resolved Hide resolved
...able/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableTransportChannelProvider.java Outdated Show resolved Hide resolved
.../src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ChannelPoolMetricsTracker.java Outdated Show resolved Hide resolved
.../src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ChannelPoolMetricsTracker.java Outdated Show resolved Hide resolved
...loud-bigtable/src/test/java/com/google/cloud/bigtable/gaxx/grpc/BigtableChannelPoolTest.java Show resolved Hide resolved
| } | ||
| | ||
| void checkAndSetIsAlts(ClientCall<?, ?> call) { | ||
| // TODO(populate ALTS holder) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: do we still need the TODO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will add it in another PR. for now, we use false.
...le-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableChannelPool.java Show resolved Hide resolved
🤖 I have created a release *beep* *boop* --- ## [2.69.0](https://togithub.com/googleapis/java-bigtable/compare/v2.68.0...v2.69.0) (2025-11-17) ### Features * **bigtable:** Add internal grpc subconnections metric and add outstanding rpcs to INTERNAL_VIEW ([#2700](https://togithub.com/googleapis/java-bigtable/issues/2700)) ([e3e6e99](https://togithub.com/googleapis/java-bigtable/commit/e3e6e993ee197f897c166fb8959755db0cb9c3fc)) * **bigtable:** Expose a metric to track the number of outstanding rpcs (unary , streaming) in channel pool ([#2696](https://togithub.com/googleapis/java-bigtable/issues/2696)) ([140a1ad](https://togithub.com/googleapis/java-bigtable/commit/140a1ad81947da26c1539632ff04748dc3498d69)) * **bigtable:** Populate alts field in channel entry ([#2702](https://togithub.com/googleapis/java-bigtable/issues/2702)) ([1bfb763](https://togithub.com/googleapis/java-bigtable/commit/1bfb763e6e4fb6fe8c808abe5dbd4221d3a632c3)) * Enable ALTS hard bound token in Bigtable w/ direct access ([#2695](https://togithub.com/googleapis/java-bigtable/issues/2695)) ([d12b37d](https://togithub.com/googleapis/java-bigtable/commit/d12b37dacf8712d30be05175828999af74159819)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
expose a metric to track the number of outstanding rpcs (unary , streaming) in channel pool