diff --git a/bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/BigtableDataSettingsFactory.java b/bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/BigtableDataSettingsFactory.java index 74a4ccbd95..832fcefa0b 100644 --- a/bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/BigtableDataSettingsFactory.java +++ b/bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/BigtableDataSettingsFactory.java @@ -15,6 +15,7 @@ */ package com.google.cloud.bigtable.hbase; +import static com.google.api.client.util.Preconditions.checkState; import static org.threeten.bp.Duration.ofMillis; import java.io.FileInputStream; @@ -32,7 +33,6 @@ import com.google.api.gax.core.NoCredentialsProvider; import com.google.api.gax.grpc.GrpcTransportChannel; import com.google.api.gax.retrying.RetrySettings; -import com.google.api.gax.rpc.FixedHeaderProvider; import com.google.api.gax.rpc.FixedTransportChannelProvider; import com.google.cloud.bigtable.config.BigtableOptions; import com.google.cloud.bigtable.config.BulkOptions; @@ -46,7 +46,6 @@ import com.google.cloud.bigtable.data.v2.stub.BigtableStubSettings; import io.grpc.ManagedChannelBuilder; -import io.grpc.internal.GrpcUtil; /** * Static methods to convert an instance of {@link Configuration} or {@link BigtableOptions} to a @@ -65,11 +64,8 @@ public class BigtableDataSettingsFactory { */ public static BigtableDataSettings fromBigtableOptions(final BigtableOptions options) throws IOException, GeneralSecurityException { - if (!options.getRetryOptions().enableRetries()) { - throw new IllegalStateException( - "Retry is must for BigtableDataSettings configuration from BigtableOptions."); - } - + checkState(!options.getRetryOptions().enableRetries(), "Disabling retries is not currently supported."); + BigtableDataSettings.Builder builder = BigtableDataSettings.newBuilder(); InstanceName instanceName = InstanceName.newBuilder().setProject(options.getProjectId()) @@ -77,37 +73,31 @@ public static BigtableDataSettings fromBigtableOptions(final BigtableOptions opt builder.setInstanceName(instanceName); builder.setAppProfileId(options.getAppProfileId()); - LOG.debug("endpoint host %s.", options.getDataHost()); - LOG.debug("endpoint host %s.", options.getPort()); builder.setEndpoint(options.getDataHost() + ":" + options.getPort()); - buildCredentialProvider(builder, options.getCredentialOptions()); - - buildBulkOptions(builder, options); + buildCredentialProviderSettings(builder, options.getCredentialOptions()); - buildCheckAndMutateRow(builder, options.getCallOptionsConfig().getShortRpcTimeoutMs()); + buildBulkMutationsSettings(builder, options); - buildReadModifyWrite(builder, options.getCallOptionsConfig().getShortRpcTimeoutMs()); + buildCheckAndMutateRowSettings(builder, options.getCallOptionsConfig().getShortRpcTimeoutMs()); - buildReadRows(builder, options); + buildReadModifyWriteSettings(builder, options.getCallOptionsConfig().getShortRpcTimeoutMs()); - buildMutateRow(builder, options); + buildReadRowsSettings(builder, options); - buildSampleRowKeys(builder, options); + buildMutateRowSettings(builder, options); - // TODO: would it map to GrpcHeaderInterceptor? or we should build userAgent - // using ManagedChannelBuilder AND - builder.setHeaderProvider( - FixedHeaderProvider.create(GrpcUtil.USER_AGENT_KEY.name(), options.getUserAgent())); + buildSampleRowKeysSettings(builder, options); // TODO: implementation for channelCount or channelPerCPU ManagedChannelBuilder channelBuilder = ManagedChannelBuilder - .forAddress(options.getDataHost(), options.getPort())// + .forAddress(options.getDataHost(), options.getPort()) .userAgent(options.getUserAgent()); if (options.usePlaintextNegotiation()) { channelBuilder.usePlaintext(); } + builder.setTransportChannelProvider( FixedTransportChannelProvider.create(GrpcTransportChannel.create(channelBuilder.build()))); @@ -120,14 +110,10 @@ public static BigtableDataSettings fromBigtableOptions(final BigtableOptions opt * @param builder a {@link BigtableDataSettings.Builder} object. * @param options a {@link BigtableOptions} object. */ - private static void buildBulkOptions(Builder builder, BigtableOptions options) { + private static void buildBulkMutationsSettings(Builder builder, BigtableOptions options) { BulkOptions bulkOptions = options.getBulkOptions(); BatchingSettings.Builder batchSettingsBuilder = BatchingSettings.newBuilder(); - FlowControlSettings.Builder flowControlBuilder = - FlowControlSettings.newBuilder() - .setMaxOutstandingRequestBytes(bulkOptions.getMaxMemory()); - long autoFlushMs = bulkOptions.getAutoflushMs(); long bulkMaxRowKeyCount = bulkOptions.getBulkMaxRowKeyCount(); long maxInflightRpcs = bulkOptions.getMaxInflightRpcs(); @@ -135,8 +121,11 @@ private static void buildBulkOptions(Builder builder, BigtableOptions options) { if (autoFlushMs > 0) { batchSettingsBuilder.setDelayThreshold(Duration.ofMillis(autoFlushMs)); } + FlowControlSettings.Builder flowControlBuilder = FlowControlSettings.newBuilder(); if (maxInflightRpcs > 0) { - flowControlBuilder.setMaxOutstandingElementCount(maxInflightRpcs * bulkMaxRowKeyCount); + flowControlBuilder + .setMaxOutstandingRequestBytes(bulkOptions.getMaxMemory()) + .setMaxOutstandingElementCount(maxInflightRpcs * bulkMaxRowKeyCount); } batchSettingsBuilder @@ -156,9 +145,9 @@ private static void buildBulkOptions(Builder builder, BigtableOptions options) { * @param builder a {@link BigtableDataSettings.Builder} object. * @param bulkMutation a {@link BulkOptions} object. */ - private static void buildSampleRowKeys(Builder builder, BigtableOptions options) { + private static void buildSampleRowKeysSettings(Builder builder, BigtableOptions options) { builder.sampleRowKeysSettings() - .setRetrySettings(defaultRetrySettings(options)); + .setRetrySettings(buildIdempotentRetrySettings(options)); } /** @@ -167,9 +156,9 @@ private static void buildSampleRowKeys(Builder builder, BigtableOptions options) * @param builder a {@link BigtableDataSettings.Builder} object. * @param bulkMutation a {@link BulkOptions} object. */ - private static void buildMutateRow(Builder builder, BigtableOptions options) { + private static void buildMutateRowSettings(Builder builder, BigtableOptions options) { builder.mutateRowSettings() - .setRetrySettings(defaultRetrySettings(options)); + .setRetrySettings(buildIdempotentRetrySettings(options)); } /** @@ -178,10 +167,24 @@ private static void buildMutateRow(Builder builder, BigtableOptions options) { * @param builder a {@link BigtableDataSettings.Builder} object. * @param bulkMutation a {@link BulkOptions} object. */ - private static void buildReadRows(Builder builder, BigtableOptions options) { - // TODO: set readPartialRowTimeout for watchdog timer, taken BigtableSession#setupWatchdog() + private static void buildReadRowsSettings(Builder builder, BigtableOptions options) { + RetryOptions retryOptions = options.getRetryOptions(); + + RetrySettings.Builder retryBuilder = RetrySettings.newBuilder() + .setInitialRetryDelay(ofMillis(retryOptions.getInitialBackoffMillis())) + .setRetryDelayMultiplier(retryOptions.getBackoffMultiplier()) + .setMaxRetryDelay(ofMillis(retryOptions.getMaxElapsedBackoffMillis())) + .setMaxAttempts(retryOptions.getMaxScanTimeoutRetries()); + + // configurations for RPC timeouts + Duration readPartialRowTimeout = ofMillis(retryOptions.getReadPartialRowTimeoutMillis()); + retryBuilder + .setInitialRpcTimeout(readPartialRowTimeout) + .setMaxRpcTimeout(readPartialRowTimeout) + .setTotalTimeout(ofMillis(options.getCallOptionsConfig().getLongRpcTimeoutMs())); + builder.readRowsSettings() - .setRetrySettings(defaultRetrySettings(options)); + .setRetrySettings(retryBuilder.build()); } /** @@ -190,7 +193,7 @@ private static void buildReadRows(Builder builder, BigtableOptions options) { * @param builder a {@link BigtableDataSettings.Builder} object. * @param bulkMutation a {@link BulkOptions} object. */ - private static void buildReadModifyWrite(Builder builder, long rpcTimeoutMs) { + private static void buildReadModifyWriteSettings(Builder builder, long rpcTimeoutMs) { builder.readModifyWriteRowSettings() .setSimpleTimeoutNoRetries(ofMillis(rpcTimeoutMs)); } @@ -201,7 +204,7 @@ private static void buildReadModifyWrite(Builder builder, long rpcTimeoutMs) { * @param builder a {@link BigtableDataSettings.Builder} object. * @param bulkMutation a {@link BulkOptions} object. */ - private static void buildCheckAndMutateRow(Builder builder, long rpcTimeoutMs) { + private static void buildCheckAndMutateRowSettings(Builder builder, long rpcTimeoutMs) { builder.checkAndMutateRowSettings() .setSimpleTimeoutNoRetries(ofMillis(rpcTimeoutMs)); } @@ -212,7 +215,7 @@ private static void buildCheckAndMutateRow(Builder builder, long rpcTimeoutMs) { * @param builder a {@link BigtableDataSettings.Builder} object. * @param bulkMutation a {@link BulkOptions} object. */ - private static RetrySettings defaultRetrySettings(BigtableOptions options) { + private static RetrySettings buildIdempotentRetrySettings(BigtableOptions options) { RetryOptions retryOptions = options.getRetryOptions(); RetrySettings.Builder retryBuilder = RetrySettings.newBuilder() @@ -222,12 +225,16 @@ private static RetrySettings defaultRetrySettings(BigtableOptions options) { .setMaxAttempts(retryOptions.getMaxScanTimeoutRetries()); // configurations for RPC timeouts + Duration shortRpcTimeout = ofMillis(options.getCallOptionsConfig().getShortRpcTimeoutMs()); retryBuilder - .setInitialRpcTimeout(ofMillis(options.getCallOptionsConfig().getShortRpcTimeoutMs())) - .setMaxRpcTimeout(ofMillis(retryOptions.getReadPartialRowTimeoutMillis())) + .setInitialRpcTimeout(shortRpcTimeout) + .setMaxRpcTimeout(shortRpcTimeout) .setTotalTimeout(ofMillis(options.getCallOptionsConfig().getLongRpcTimeoutMs())); - - // TODO: an option to set RetryOptions#allowRetriesWithoutTimestamp + + if (retryOptions.allowRetriesWithoutTimestamp()) { + //TODO: add instruction to create unsafeMutation. + LOG.warn("Retries without Timestamp doesn't support"); + } return retryBuilder.build(); } @@ -239,7 +246,7 @@ private static RetrySettings defaultRetrySettings(BigtableOptions options) { * @throws FileNotFoundException * @throws IOException */ - private static void buildCredentialProvider(Builder builder, CredentialOptions credentialOptions) + private static void buildCredentialProviderSettings(Builder builder, CredentialOptions credentialOptions) throws FileNotFoundException, IOException { CredentialsProvider credentialsProvider = NoCredentialsProvider.create(); diff --git a/bigtable-client-core-parent/bigtable-hbase/src/test/java/com/google/cloud/bigtable/hbase/TestBigtableDataSettingsFactory.java b/bigtable-client-core-parent/bigtable-hbase/src/test/java/com/google/cloud/bigtable/hbase/TestBigtableDataSettingsFactory.java index 46dd947ca2..7969e6c822 100644 --- a/bigtable-client-core-parent/bigtable-hbase/src/test/java/com/google/cloud/bigtable/hbase/TestBigtableDataSettingsFactory.java +++ b/bigtable-client-core-parent/bigtable-hbase/src/test/java/com/google/cloud/bigtable/hbase/TestBigtableDataSettingsFactory.java @@ -58,11 +58,14 @@ public void testInstanceIdIsRequired() throws IOException, GeneralSecurityExcept BigtableDataSettingsFactory.fromBigtableOptions(options); } - @Test(expected = NullPointerException.class) + @Test public void testWithoutUserAgent() throws IOException, GeneralSecurityException { BigtableOptions options = BigtableOptions.builder().setProjectId(TEST_PROJECT_ID) .setInstanceId(TEST_INSTANCE_ID).build(); - BigtableDataSettingsFactory.fromBigtableOptions(options); + BigtableDataSettings dataSettings = BigtableDataSettingsFactory.fromBigtableOptions(options); + // TODO: Need to assert UserAgent to null & add more test cases + Assert.assertEquals(TEST_PROJECT_ID, dataSettings.getInstanceName().getProject()); + Assert.assertEquals(TEST_INSTANCE_ID, dataSettings.getInstanceName().getInstance()); } @Test