Add CSM for batch write flow control #2685

kongweihan · 2025-10-06T16:18:19Z

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)
Rollback plan is reviewed and LGTMed
All new data plane features have a completed end to end testing plan

Fixes #<issue_number_goes_here> ☕️

If you write sample code, please follow the samples format.

...rc/main/java/com/google/cloud/bigtable/data/v2/stub/RateLimitingServerStreamingCallable.java

...ud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java

...le/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java

...rc/main/java/com/google/cloud/bigtable/data/v2/stub/RateLimitingServerStreamingCallable.java

...ud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java

...le/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java

...rc/main/java/com/google/cloud/bigtable/data/v2/stub/RateLimitingServerStreamingCallable.java

...table/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java

...rc/main/java/com/google/cloud/bigtable/data/v2/stub/RateLimitingServerStreamingCallable.java

...e/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java

mutianf · 2025-12-10T01:39:06Z

...e/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java

+      // closing the batcher to trigger the third flush
+      batcher.close();
+
+      MetricData applicationLatency = getMetricData(metricReader, BATCH_WRITE_FLOW_CONTROL_TARGET_QPS_NAME);


Suggested change

MetricData applicationLatency = getMetricData(metricReader, BATCH_WRITE_FLOW_CONTROL_TARGET_QPS_NAME);

MetricData targetQps = getMetricData(metricReader, BATCH_WRITE_FLOW_CONTROL_TARGET_QPS_NAME);

mutianf · 2025-12-10T01:41:31Z

...e/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java

        }
        builder.addEntriesBuilder().setIndex(i);
+
+        com.google.protobuf.Duration duration = builder.getRateLimitInfoBuilder().getPeriodBuilder().setSeconds(10).build();


nit: can you import com.google.bigtable.v2.RateLimitInfo so we don't need to write the full path here?

...e/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java

mutianf

Thank you for adding the tests, left some more nits but otherwise looks good!

mutianf · 2025-12-12T02:47:23Z

...le/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java

-import io.opentelemetry.sdk.metrics.InstrumentSelector;
-import io.opentelemetry.sdk.metrics.InstrumentType;
-import io.opentelemetry.sdk.metrics.View;
+import io.opentelemetry.sdk.metrics.*;


here also, no import *

mutianf · 2025-12-12T02:48:10Z

...rc/main/java/com/google/cloud/bigtable/data/v2/stub/RateLimitingServerStreamingCallable.java

 */
 package com.google.cloud.bigtable.data.v2.stub;

+import static com.google.cloud.bigtable.data.v2.stub.metrics.Util.extractStatus;


I don't think you need this import anymore? You can run mvn com.spotify.fmt:fmt-maven-plugin:format and that should clean up the unused imports.

mutianf · 2025-12-12T02:48:21Z

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java


  /** Convert an exception into a value that can be used to create an OpenCensus tag value. */
-  static String extractStatus(@Nullable Throwable error) {
+  public static String extractStatus(@Nullable Throwable error) {


this doesn't need to be public now

mutianf · 2025-12-12T02:51:08Z

...le/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTestUtils.java

+                .collect(Collectors.toList())
+                .get(0);
+        return hd.getSum() / hd.getCount();
+      case LONG_SUM:


is this right? this is long_sum, how could the value be double?

mutianf · 2025-12-12T15:10:00Z

...e/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java

+      batcher.add(
+          RowMutationEntry.create("batch-write-flow-control-success-12").setCell("f", "q", "v"));
+
+      // closing the batcher to trigger the third flush


nit:

Suggested change

// closing the batcher to trigger the third flush

// closing the batcher to trigger the flush

mutianf · 2025-12-12T15:10:10Z

...e/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java

+      batcher.add(
+          RowMutationEntry.create("batch-write-flow-control-success-08").setCell("f", "q", "v"));
+
+      // closing the batcher to trigger the third flush


nit:

Suggested change

// closing the batcher to trigger the third flush

// closing the batcher to trigger the flush

mutianf · 2025-12-12T15:10:34Z

...e/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java

+      batcher.add(
+          RowMutationEntry.create("batch-write-flow-control-success-18").setCell("f", "q", "v"));
+
+      // closing the batcher to trigger the third flush


nit:

Suggested change

// closing the batcher to trigger the third flush

// closing the batcher to trigger the flush

mutianf · 2025-12-12T15:11:13Z

...e/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java

+      batcher.add(
+          RowMutationEntry.create("batch-write-flow-control-success-05").setCell("f", "q", "v"));
+
+      // closing the batcher to trigger the third flush


Suggested change

// closing the batcher to trigger the third flush

// closing the batcher to trigger the flush

mutianf · 2025-12-12T15:11:32Z

...e/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java

+          RowMutationEntry.create("batch-write-flow-control-fail-unavailable")
+              .setCell("f", "q", "v"));
+
+      // closing the batcher to trigger the third flush


Suggested change

// closing the batcher to trigger the third flush

// closing the batcher to trigger the flush

mutianf · 2025-12-12T15:12:56Z

...e/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java

+
+      // Add RateLimitInfo for Batch Write Flow Control
+      if (receivedRowkey.equals("batch-write-flow-control-success-18")) {
+        com.google.protobuf.Duration duration =


nit: you can probably build the duration outside of the if statements to simplify the code

kongweihan requested review from a team as code owners October 6, 2025 16:18

product-auto-label bot added size: m Pull request size is medium. api: bigtable Issues related to the googleapis/java-bigtable API. labels Oct 6, 2025

kongweihan force-pushed the csm-batch-write-flow-control branch 2 times, most recently from 2ca884e to b4006a2 Compare October 6, 2025 16:54

mutianf reviewed Oct 6, 2025

View reviewed changes

kongweihan force-pushed the csm-batch-write-flow-control branch from b4006a2 to e8a246f Compare October 7, 2025 03:03

mutianf reviewed Oct 8, 2025

View reviewed changes

kongweihan force-pushed the csm-batch-write-flow-control branch from e8a246f to ae0ed5a Compare October 8, 2025 14:27

kongweihan force-pushed the csm-batch-write-flow-control branch 7 times, most recently from 7d3fdd2 to 5f07efc Compare December 10, 2025 00:37

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Dec 10, 2025

mutianf reviewed Dec 10, 2025

View reviewed changes

kongweihan force-pushed the csm-batch-write-flow-control branch 2 times, most recently from 6169b3c to 8e84e9f Compare December 11, 2025 21:16

feat: Add CSM for batch write flow control

7981240

kongweihan force-pushed the csm-batch-write-flow-control branch from 8e84e9f to 7981240 Compare December 11, 2025 22:36

mutianf reviewed Dec 12, 2025

View reviewed changes

	MetricData applicationLatency = getMetricData(metricReader, BATCH_WRITE_FLOW_CONTROL_TARGET_QPS_NAME);
	MetricData targetQps = getMetricData(metricReader, BATCH_WRITE_FLOW_CONTROL_TARGET_QPS_NAME);

	// closing the batcher to trigger the third flush
	// closing the batcher to trigger the flush

Add CSM for batch write flow control #2685

Are you sure you want to change the base?

Add CSM for batch write flow control #2685

Uh oh!

Conversation

kongweihan commented Oct 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mutianf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants