From 4a4030ee0db04c62a885df37f8ff5de8968f8063 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 27 Nov 2023 16:40:27 -0500 Subject: [PATCH 1/2] chore: clean up BigtableIO client creation code --- .../bigtable/BigtableConfigTranslator.java | 7 ++++++ .../gcp/bigtable/BigtableServiceFactory.java | 7 +----- .../io/gcp/bigtable/BigtableServiceImpl.java | 24 ++----------------- 3 files changed, 10 insertions(+), 28 deletions(-) diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableConfigTranslator.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableConfigTranslator.java index 5100b36f23b4..598672797b9c 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableConfigTranslator.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableConfigTranslator.java @@ -256,6 +256,13 @@ private static BigtableDataSettings configureReadSettings( Duration.ofMillis(readOptions.getOperationTimeout().getMillis())); } + if (readOptions.getWaitTimeout() != null) { + settings + .stubSettings() + .readRowsSettings() + .setWaitTimeout(Duration.ofMillis(readOptions.getWaitTimeout().getMillis())); + } + settings.stubSettings().readRowsSettings().setRetrySettings(retrySettings.build()); return settings.build(); diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceFactory.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceFactory.java index 635856430fb3..aa369d1f7ca8 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceFactory.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceFactory.java @@ -128,12 +128,7 @@ BigtableServiceEntry getServiceForReading( BigtableDataSettings.enableBuiltinMetrics(); } - BigtableService service; - if (opts.getWaitTimeout() != null) { - service = new BigtableServiceImpl(settings, opts.getWaitTimeout()); - } else { - service = new BigtableServiceImpl(settings); - } + BigtableService service = new BigtableServiceImpl(settings); entry = BigtableServiceEntry.create(configId, service); entries.put(configId.id(), entry); refCounts.put(configId.id(), new AtomicInteger(1)); diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceImpl.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceImpl.java index 31a317352beb..b31c6377e43b 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceImpl.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceImpl.java @@ -101,33 +101,13 @@ class BigtableServiceImpl implements BigtableService { private static final long MIN_BYTE_BUFFER_SIZE = 100 * 1024 * 1024; // 100MB BigtableServiceImpl(BigtableDataSettings settings) throws IOException { - this(settings, null); - } - - // TODO remove this constructor once https://github.com/googleapis/gapic-generator-java/pull/1473 - // is resolved. readWaitTimeout is a hack to workaround incorrect mapping from attempt timeout to - // Watchdog's wait timeout. - BigtableServiceImpl(BigtableDataSettings settings, Duration readWaitTimeout) throws IOException { this.projectId = settings.getProjectId(); this.instanceId = settings.getInstanceId(); RetrySettings retry = settings.getStubSettings().readRowsSettings().getRetrySettings(); this.readAttemptTimeout = Duration.millis(retry.getInitialRpcTimeout().toMillis()); this.readOperationTimeout = Duration.millis(retry.getTotalTimeout().toMillis()); - BigtableDataSettings.Builder builder = settings.toBuilder(); - if (readWaitTimeout != null) { - builder - .stubSettings() - .readRowsSettings() - .setRetrySettings( - retry - .toBuilder() - .setInitialRpcTimeout( - org.threeten.bp.Duration.ofMillis(readWaitTimeout.getMillis())) - .setMaxRpcTimeout(org.threeten.bp.Duration.ofMillis(readWaitTimeout.getMillis())) - .build()); - } - LOG.info("Started Bigtable service with settings " + builder.build()); - this.client = BigtableDataClient.create(builder.build()); + LOG.info("Started Bigtable service with settings " + settings); + this.client = BigtableDataClient.create(settings); } private final BigtableDataClient client; From aafc9c9ee18c08dc2030d6a29037e4fc9806b82a Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 28 Nov 2023 10:39:03 -0500 Subject: [PATCH 2/2] update logging format --- .../apache/beam/sdk/io/gcp/bigtable/BigtableServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceImpl.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceImpl.java index b31c6377e43b..6e98b7e7866a 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceImpl.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceImpl.java @@ -106,7 +106,7 @@ class BigtableServiceImpl implements BigtableService { RetrySettings retry = settings.getStubSettings().readRowsSettings().getRetrySettings(); this.readAttemptTimeout = Duration.millis(retry.getInitialRpcTimeout().toMillis()); this.readOperationTimeout = Duration.millis(retry.getTotalTimeout().toMillis()); - LOG.info("Started Bigtable service with settings " + settings); + LOG.info("Started Bigtable service with settings {}", settings); this.client = BigtableDataClient.create(settings); }