igorbernstein2 commented on code in PR #32442:
URL: https://github.com/apache/beam/pull/32442#discussion_r1758950956
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableIO.java:
##########
@@ -1116,51 +1110,12 @@ 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();
- }
-
- /**
- * Returns a new {@link BigtableIO.Write} with client side latency based
throttling enabled.
- *
- * <p>Will also set {@link #withThrottlingReportTargetMs} to the same
value.
- */
- public Write withThrottlingTargetMs(int throttlingTargetMs) {
- BigtableWriteOptions options = getBigtableWriteOptions();
- return toBuilder()
- .setBigtableWriteOptions(
- options
- .toBuilder()
- .setThrottlingTargetMs(throttlingTargetMs)
- .setThrottlingReportTargetMs(throttlingTargetMs)
- .build())
- .build();
- }
-
- /**
- * Returns a new {@link BigtableIO.Write} with throttling time reporting
enabled. When write
- * request latency exceeded the set value, the amount greater than the
target will be considered
- * as throttling time and report back to runner.
- *
- * <p>If not set, defaults to 3 min for completed batch request. Client
side flowing control
- * configurations (e.g. {@link #withFlowControl}, {@link
#withThrottlingTargetMs} will adjust
- * the default value accordingly. Set to 0 to disable throttling time
reporting.
- */
- public Write withThrottlingReportTargetMs(int throttlingReportTargetMs) {
Review Comment:
same as above
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableIO.java:
##########
@@ -1116,51 +1110,12 @@ 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();
- }
-
- /**
- * Returns a new {@link BigtableIO.Write} with client side latency based
throttling enabled.
- *
- * <p>Will also set {@link #withThrottlingReportTargetMs} to the same
value.
- */
- public Write withThrottlingTargetMs(int throttlingTargetMs) {
Review Comment:
can you actually remove a public method? I suspect that you need to make it
a no-op that logs a warning
--
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]
