Abacn commented on code in PR #32442:
URL: https://github.com/apache/beam/pull/32442#discussion_r1759092293
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableIO.java:
##########
@@ -1116,52 +1110,27 @@ public Write withMaxOutstandingBytes(long bytes) {
* always enabled on batch writes and limits the number of outstanding
requests to the Bigtable
* server.
*
- * <p>When enabled, will also set default {@link
#withThrottlingReportTargetMs} to 1 minute.
- * This enables runner react with increased latency in flush call due to
flow control.
- *
* <p>Does not modify this object.
*/
public Write withFlowControl(boolean enableFlowControl) {
BigtableWriteOptions options = getBigtableWriteOptions();
- BigtableWriteOptions.Builder builder =
options.toBuilder().setFlowControl(enableFlowControl);
- if (enableFlowControl) {
- builder = builder.setThrottlingReportTargetMs(60_000);
- }
- return toBuilder().setBigtableWriteOptions(builder.build()).build();
+ return toBuilder()
+
.setBigtableWriteOptions(options.toBuilder().setFlowControl(enableFlowControl).build())
+ .build();
}
- /**
- * Returns a new {@link BigtableIO.Write} with client side latency based
throttling enabled.
- *
- * <p>Will also set {@link #withThrottlingReportTargetMs} to the same
value.
- */
+ /** Returns a new {@link BigtableIO.Write} with client side latency based
throttling enabled. */
Review Comment:
`withThrottlingTargetMs` can be preserved. It was a config existed in
BigtableWriteOptions but missing the correspondent in transform configuration.
It was a inconsistency in BigtableIO configuration setting and not related to
throttling counter change.
--
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]
