From d274fc29767c512eb4b92f71822ed8c86a083319 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 16 Aug 2023 10:19:32 -0400 Subject: [PATCH 01/22] feat: client sends retry cookie back to server --- .../data/v2/stub/CookieInterceptor.java | 83 +++++++++++ .../bigtable/data/v2/stub/CookiesHolder.java | 43 ++++++ .../stub/CookiesServerStreamingCallable.java | 44 ++++++ .../data/v2/stub/CookiesUnaryCallable.java | 41 ++++++ .../data/v2/stub/EnhancedBigtableStub.java | 12 +- .../v2/stub/EnhancedBigtableStubSettings.java | 3 +- .../data/v2/stub/CookieHolderTest.java | 131 ++++++++++++++++++ 7 files changed, 353 insertions(+), 4 deletions(-) create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesServerStreamingCallable.java create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesUnaryCallable.java create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java new file mode 100644 index 0000000000..55df349bb9 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java @@ -0,0 +1,83 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.COOKIES_HOLDER_KEY; +import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.ROUTING_COOKIE_KEY; +import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.ROUTING_COOKIE_METADATA_KEY; + +import com.google.protobuf.ByteString; +import com.google.rpc.ErrorInfo; +import io.grpc.CallOptions; +import io.grpc.Channel; +import io.grpc.ClientCall; +import io.grpc.ClientInterceptor; +import io.grpc.ForwardingClientCall; +import io.grpc.ForwardingClientCallListener; +import io.grpc.Metadata; +import io.grpc.MethodDescriptor; +import io.grpc.Status; +import io.grpc.protobuf.ProtoUtils; + +/** + * A cookie interceptor that checks the cookie value from returned ErrorInfo, updates the cookie + * holder, and inject it in the header of the next request. + */ +class CookieInterceptor implements ClientInterceptor { + + static final Metadata.Key ERROR_INFO_KEY = + ProtoUtils.keyForProto(ErrorInfo.getDefaultInstance()); + + @Override + public ClientCall interceptCall( + MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { + return new ForwardingClientCall.SimpleForwardingClientCall( + channel.newCall(methodDescriptor, callOptions)) { + @Override + public void start(Listener responseListener, Metadata headers) { + CookiesHolder cookie = callOptions.getOption(COOKIES_HOLDER_KEY); + if (cookie != null && cookie.getRoutingCookie() != null) { + headers.put(ROUTING_COOKIE_METADATA_KEY, cookie.getRoutingCookie().toByteArray()); + } + super.start(new UpdateCookieListener<>(responseListener, callOptions), headers); + } + }; + } + + static class UpdateCookieListener + extends ForwardingClientCallListener.SimpleForwardingClientCallListener { + + private final CallOptions callOptions; + + UpdateCookieListener(ClientCall.Listener delegate, CallOptions callOptions) { + super(delegate); + this.callOptions = callOptions; + } + + @Override + public void onClose(Status status, Metadata trailers) { + if (status != Status.OK && trailers != null) { + ErrorInfo errorInfo = trailers.get(ERROR_INFO_KEY); + if (errorInfo != null) { + CookiesHolder cookieHolder = callOptions.getOption(COOKIES_HOLDER_KEY); + cookieHolder.setRoutingCookie( + ByteString.copyFromUtf8(errorInfo.getMetadataMap().get(ROUTING_COOKIE_KEY))); + } + } + super.onClose(status, trailers); + } + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java new file mode 100644 index 0000000000..9a60de15dc --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java @@ -0,0 +1,43 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import com.google.protobuf.ByteString; +import io.grpc.CallOptions; +import io.grpc.Metadata; +import javax.annotation.Nullable; + +/** A cookie that holds information for the retry */ +class CookiesHolder { + + static final CallOptions.Key COOKIES_HOLDER_KEY = + CallOptions.Key.create("bigtable-cookies"); + + static final String ROUTING_COOKIE_KEY = "bigtable-routing-cookie"; + + static final Metadata.Key ROUTING_COOKIE_METADATA_KEY = + Metadata.Key.of("bigtable-routing-cookie-bin", Metadata.BINARY_BYTE_MARSHALLER); + + @Nullable private ByteString routingCookie; + + void setRoutingCookie(@Nullable ByteString routingCookie) { + this.routingCookie = routingCookie; + } + + ByteString getRoutingCookie() { + return this.routingCookie; + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesServerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesServerStreamingCallable.java new file mode 100644 index 0000000000..1d897f18cf --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesServerStreamingCallable.java @@ -0,0 +1,44 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.COOKIES_HOLDER_KEY; + +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.rpc.ApiCallContext; +import com.google.api.gax.rpc.ResponseObserver; +import com.google.api.gax.rpc.ServerStreamingCallable; + +public class CookiesServerStreamingCallable + extends ServerStreamingCallable { + + private final ServerStreamingCallable callable; + + CookiesServerStreamingCallable(ServerStreamingCallable innerCallable) { + this.callable = innerCallable; + } + + @Override + public void call( + RequestT request, ResponseObserver responseObserver, ApiCallContext context) { + GrpcCallContext grpcCallContext = (GrpcCallContext) context; + callable.call( + request, + responseObserver, + grpcCallContext.withCallOptions( + grpcCallContext.getCallOptions().withOption(COOKIES_HOLDER_KEY, new CookiesHolder()))); + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesUnaryCallable.java new file mode 100644 index 0000000000..5e0b33d03a --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesUnaryCallable.java @@ -0,0 +1,41 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.COOKIES_HOLDER_KEY; + +import com.google.api.core.ApiFuture; +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.rpc.ApiCallContext; +import com.google.api.gax.rpc.UnaryCallable; + +/** Cookie callable injects a placeholder for bigtable retry cookie. */ +class CookiesUnaryCallable extends UnaryCallable { + private UnaryCallable innerCallable; + + CookiesUnaryCallable(UnaryCallable callable) { + this.innerCallable = callable; + } + + @Override + public ApiFuture futureCall(RequestT request, ApiCallContext context) { + GrpcCallContext grpcCallContext = (GrpcCallContext) context; + return innerCallable.futureCall( + request, + grpcCallContext.withCallOptions( + grpcCallContext.getCallOptions().withOption(COOKIES_HOLDER_KEY, new CookiesHolder()))); + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 0420e47dcf..01e6855185 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -365,7 +365,9 @@ public ServerStreamingCallable createReadRowsCallable( new TracedServerStreamingCallable<>( readRowsUserCallable, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + ServerStreamingCallable withCookie = new CookiesServerStreamingCallable<>(traced); + + return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -401,7 +403,9 @@ public UnaryCallable createReadRowCallable(RowAdapter new TracedUnaryCallable<>( firstRow, clientContext.getTracerFactory(), getSpanName("ReadRow")); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + UnaryCallable withCookie = new CookiesUnaryCallable<>(traced); + + return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -1017,7 +1021,9 @@ private UnaryCallable createUserFacin UnaryCallable traced = new TracedUnaryCallable<>(inner, clientContext.getTracerFactory(), getSpanName(methodName)); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + UnaryCallable withCookie = new CookiesUnaryCallable<>(traced); + + return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); } private UnaryCallable createPingAndWarmCallable() { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index cffd9c85df..0e27c713fe 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java @@ -329,7 +329,8 @@ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProvi Duration.ofSeconds(10)) // wait this long before considering the connection dead // Attempts direct access to CBT service over gRPC to improve throughput, // whether the attempt is allowed is totally controlled by service owner. - .setAttemptDirectPath(true); + .setAttemptDirectPath(true) + .setInterceptorProvider(() -> ImmutableList.of(new CookieInterceptor())); } @SuppressWarnings("WeakerAccess") diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java new file mode 100644 index 0000000000..5321144f95 --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java @@ -0,0 +1,131 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import static com.google.cloud.bigtable.data.v2.stub.CookieInterceptor.ERROR_INFO_KEY; +import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.ROUTING_COOKIE_KEY; +import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.ROUTING_COOKIE_METADATA_KEY; +import static com.google.common.truth.Truth.assertThat; + +import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; +import com.google.bigtable.v2.BigtableGrpc; +import com.google.bigtable.v2.MutateRowRequest; +import com.google.bigtable.v2.MutateRowResponse; +import com.google.cloud.bigtable.data.v2.BigtableDataClient; +import com.google.cloud.bigtable.data.v2.BigtableDataSettings; +import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; +import com.google.cloud.bigtable.data.v2.models.RowMutation; +import com.google.common.collect.ImmutableList; +import com.google.rpc.ErrorInfo; +import io.grpc.Metadata; +import io.grpc.Server; +import io.grpc.ServerCall; +import io.grpc.ServerCallHandler; +import io.grpc.ServerInterceptor; +import io.grpc.Status; +import io.grpc.StatusRuntimeException; +import io.grpc.stub.StreamObserver; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class CookieHolderTest { + private Server server; + private FakeService fakeService = new FakeService(); + + private BigtableDataClient client; + + private List serverMetadata = new ArrayList<>(); + + @Before + public void setup() throws Exception { + ServerInterceptor serverInterceptor = + new ServerInterceptor() { + @Override + public ServerCall.Listener interceptCall( + ServerCall serverCall, + Metadata metadata, + ServerCallHandler serverCallHandler) { + serverMetadata.add(metadata); + return serverCallHandler.startCall(serverCall, metadata); + } + }; + + server = FakeServiceBuilder.create(fakeService).intercept(serverInterceptor).start(); + + BigtableDataSettings.Builder settings = + BigtableDataSettings.newBuilderForEmulator(server.getPort()) + .setProjectId("fake-project") + .setInstanceId("fake-instance"); + + InstantiatingGrpcChannelProvider channelProvider = + (InstantiatingGrpcChannelProvider) settings.stubSettings().getTransportChannelProvider(); + settings + .stubSettings() + .setTransportChannelProvider( + channelProvider + .toBuilder() + .setInterceptorProvider(() -> ImmutableList.of(new CookieInterceptor())) + .build()); + + client = BigtableDataClient.create(settings.build()); + } + + @Test + public void testRetryCookieIsForwarded() { + client.mutateRow(RowMutation.create("fake-table", "fake-row").setCell("cf", "q", "v")); + + assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + byte[] bytes = serverMetadata.get(1).get(ROUTING_COOKIE_METADATA_KEY); + assertThat(new String(bytes, StandardCharsets.UTF_8)).isEqualTo("test-routing-cookie"); + + serverMetadata.clear(); + } + + @After + public void tearDown() throws Exception { + client.close(); + server.shutdown(); + } + + class FakeService extends BigtableGrpc.BigtableImplBase { + + private AtomicInteger count = new AtomicInteger(); + + @Override + public void mutateRow( + MutateRowRequest request, StreamObserver responseObserver) { + if (count.getAndIncrement() < 1) { + Metadata trailers = new Metadata(); + ErrorInfo errorInfo = + ErrorInfo.newBuilder().putMetadata(ROUTING_COOKIE_KEY, "test-routing-cookie").build(); + trailers.put(ERROR_INFO_KEY, errorInfo); + StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); + responseObserver.onError(exception); + return; + } + responseObserver.onNext(MutateRowResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } + } +} From 9333ba03bf5638dfb873adb925b674745de61f9c Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 17 Oct 2023 17:03:00 -0400 Subject: [PATCH 02/22] udpate to use trailer instead of error info --- .../data/v2/stub/CookieInterceptor.java | 17 ++---- .../bigtable/data/v2/stub/CookiesHolder.java | 4 +- .../stub/CookiesServerStreamingCallable.java | 3 +- .../data/v2/stub/EnhancedBigtableStub.java | 4 ++ .../data/v2/stub/CookieHolderTest.java | 60 ++++++++++++++----- 5 files changed, 58 insertions(+), 30 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java index 55df349bb9..de8c60173c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java @@ -16,11 +16,9 @@ package com.google.cloud.bigtable.data.v2.stub; import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.COOKIES_HOLDER_KEY; -import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.ROUTING_COOKIE_KEY; import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.ROUTING_COOKIE_METADATA_KEY; import com.google.protobuf.ByteString; -import com.google.rpc.ErrorInfo; import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ClientCall; @@ -30,7 +28,6 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Status; -import io.grpc.protobuf.ProtoUtils; /** * A cookie interceptor that checks the cookie value from returned ErrorInfo, updates the cookie @@ -38,9 +35,6 @@ */ class CookieInterceptor implements ClientInterceptor { - static final Metadata.Key ERROR_INFO_KEY = - ProtoUtils.keyForProto(ErrorInfo.getDefaultInstance()); - @Override public ClientCall interceptCall( MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { @@ -69,13 +63,10 @@ static class UpdateCookieListener @Override public void onClose(Status status, Metadata trailers) { - if (status != Status.OK && trailers != null) { - ErrorInfo errorInfo = trailers.get(ERROR_INFO_KEY); - if (errorInfo != null) { - CookiesHolder cookieHolder = callOptions.getOption(COOKIES_HOLDER_KEY); - cookieHolder.setRoutingCookie( - ByteString.copyFromUtf8(errorInfo.getMetadataMap().get(ROUTING_COOKIE_KEY))); - } + if (trailers != null && trailers.containsKey(ROUTING_COOKIE_METADATA_KEY)) { + byte[] cookie = trailers.get(ROUTING_COOKIE_METADATA_KEY); + CookiesHolder cookieHolder = callOptions.getOption(COOKIES_HOLDER_KEY); + cookieHolder.setRoutingCookie(ByteString.copyFrom(cookie)); } super.onClose(status, trailers); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java index 9a60de15dc..bc058000d9 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java @@ -26,10 +26,10 @@ class CookiesHolder { static final CallOptions.Key COOKIES_HOLDER_KEY = CallOptions.Key.create("bigtable-cookies"); - static final String ROUTING_COOKIE_KEY = "bigtable-routing-cookie"; + static final String ROUTING_COOKIE_KEY = "x-goog-cbt-cookie-routing"; static final Metadata.Key ROUTING_COOKIE_METADATA_KEY = - Metadata.Key.of("bigtable-routing-cookie-bin", Metadata.BINARY_BYTE_MARSHALLER); + Metadata.Key.of(ROUTING_COOKIE_KEY + "-bin", Metadata.BINARY_BYTE_MARSHALLER); @Nullable private ByteString routingCookie; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesServerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesServerStreamingCallable.java index 1d897f18cf..f8f62a5150 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesServerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesServerStreamingCallable.java @@ -22,7 +22,8 @@ import com.google.api.gax.rpc.ResponseObserver; import com.google.api.gax.rpc.ServerStreamingCallable; -public class CookiesServerStreamingCallable +/** Cookie callable injects a placeholder for bigtable retry cookie. */ +class CookiesServerStreamingCallable extends ServerStreamingCallable { private final ServerStreamingCallable callable; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 01e6855185..8d20c917fd 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -365,6 +365,8 @@ public ServerStreamingCallable createReadRowsCallable( new TracedServerStreamingCallable<>( readRowsUserCallable, clientContext.getTracerFactory(), span); + // CookieHolder needs to be injected to the CallOptions outside of retries, otherwise retry + // attempts won't see a CookieHolder. ServerStreamingCallable withCookie = new CookiesServerStreamingCallable<>(traced); return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); @@ -1021,6 +1023,8 @@ private UnaryCallable createUserFacin UnaryCallable traced = new TracedUnaryCallable<>(inner, clientContext.getTracerFactory(), getSpanName(methodName)); + // CookieHolder needs to be injected to the CallOptions outside of retries, otherwise retry + // attempts won't see a CookieHolder. UnaryCallable withCookie = new CookiesUnaryCallable<>(traced); return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java index 5321144f95..de48f0546a 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java @@ -15,21 +15,21 @@ */ package com.google.cloud.bigtable.data.v2.stub; -import static com.google.cloud.bigtable.data.v2.stub.CookieInterceptor.ERROR_INFO_KEY; -import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.ROUTING_COOKIE_KEY; import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.ROUTING_COOKIE_METADATA_KEY; import static com.google.common.truth.Truth.assertThat; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.bigtable.v2.BigtableGrpc; -import com.google.bigtable.v2.MutateRowRequest; -import com.google.bigtable.v2.MutateRowResponse; +import com.google.bigtable.v2.ReadRowsRequest; +import com.google.bigtable.v2.ReadRowsResponse; +import com.google.bigtable.v2.SampleRowKeysRequest; +import com.google.bigtable.v2.SampleRowKeysResponse; import com.google.cloud.bigtable.data.v2.BigtableDataClient; import com.google.cloud.bigtable.data.v2.BigtableDataSettings; import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; -import com.google.cloud.bigtable.data.v2.models.RowMutation; +import com.google.cloud.bigtable.data.v2.models.Query; import com.google.common.collect.ImmutableList; -import com.google.rpc.ErrorInfo; +import com.google.protobuf.ByteString; import io.grpc.Metadata; import io.grpc.Server; import io.grpc.ServerCall; @@ -92,11 +92,27 @@ public ServerCall.Listener interceptCall( } @Test - public void testRetryCookieIsForwarded() { - client.mutateRow(RowMutation.create("fake-table", "fake-row").setCell("cf", "q", "v")); + public void testReadRowsRetryCookieIsForwarded() { + client.readRows(Query.create("fake-table")).iterator().hasNext(); + assertThat(fakeService.count.get()).isGreaterThan(1); assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); byte[] bytes = serverMetadata.get(1).get(ROUTING_COOKIE_METADATA_KEY); + assertThat(bytes).isNotNull(); + assertThat(new String(bytes, StandardCharsets.UTF_8)).isEqualTo("test-routing-cookie"); + + serverMetadata.clear(); + } + + @Test + public void testSampleRowKeysRetryCookieIsForwarded() { + + client.sampleRowKeys("fake-table"); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + byte[] bytes = serverMetadata.get(1).get(ROUTING_COOKIE_METADATA_KEY); + assertThat(bytes).isNotNull(); assertThat(new String(bytes, StandardCharsets.UTF_8)).isEqualTo("test-routing-cookie"); serverMetadata.clear(); @@ -113,18 +129,34 @@ class FakeService extends BigtableGrpc.BigtableImplBase { private AtomicInteger count = new AtomicInteger(); @Override - public void mutateRow( - MutateRowRequest request, StreamObserver responseObserver) { + public void readRows( + ReadRowsRequest request, StreamObserver responseObserver) { + if (count.getAndIncrement() < 1) { + Metadata trailers = new Metadata(); + trailers.put( + ROUTING_COOKIE_METADATA_KEY, + ByteString.copyFromUtf8("test-routing-cookie").toByteArray()); + StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); + responseObserver.onError(exception); + return; + } + responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } + + @Override + public void sampleRowKeys( + SampleRowKeysRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - ErrorInfo errorInfo = - ErrorInfo.newBuilder().putMetadata(ROUTING_COOKIE_KEY, "test-routing-cookie").build(); - trailers.put(ERROR_INFO_KEY, errorInfo); + trailers.put( + ROUTING_COOKIE_METADATA_KEY, + ByteString.copyFromUtf8("test-routing-cookie").toByteArray()); StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; } - responseObserver.onNext(MutateRowResponse.getDefaultInstance()); + responseObserver.onNext(SampleRowKeysResponse.getDefaultInstance()); responseObserver.onCompleted(); } } From 678bc35b2c67042158b49bae5f14e2c7d40fa29f Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 19 Oct 2023 12:46:17 -0400 Subject: [PATCH 03/22] updating the header name --- .../data/v2/stub/CookieInterceptor.java | 13 ++++++----- .../bigtable/data/v2/stub/CookiesHolder.java | 11 +++++----- .../data/v2/stub/CookieHolderTest.java | 22 +++++++------------ 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java index de8c60173c..4b426bf77d 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java @@ -18,7 +18,6 @@ import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.COOKIES_HOLDER_KEY; import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.ROUTING_COOKIE_METADATA_KEY; -import com.google.protobuf.ByteString; import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ClientCall; @@ -44,7 +43,7 @@ public ClientCall interceptCall( public void start(Listener responseListener, Metadata headers) { CookiesHolder cookie = callOptions.getOption(COOKIES_HOLDER_KEY); if (cookie != null && cookie.getRoutingCookie() != null) { - headers.put(ROUTING_COOKIE_METADATA_KEY, cookie.getRoutingCookie().toByteArray()); + headers.put(ROUTING_COOKIE_METADATA_KEY, cookie.getRoutingCookie()); } super.start(new UpdateCookieListener<>(responseListener, callOptions), headers); } @@ -63,10 +62,12 @@ static class UpdateCookieListener @Override public void onClose(Status status, Metadata trailers) { - if (trailers != null && trailers.containsKey(ROUTING_COOKIE_METADATA_KEY)) { - byte[] cookie = trailers.get(ROUTING_COOKIE_METADATA_KEY); - CookiesHolder cookieHolder = callOptions.getOption(COOKIES_HOLDER_KEY); - cookieHolder.setRoutingCookie(ByteString.copyFrom(cookie)); + CookiesHolder cookiesHolder = callOptions.getOption(COOKIES_HOLDER_KEY); + if (cookiesHolder != null + && trailers != null + && trailers.containsKey(ROUTING_COOKIE_METADATA_KEY)) { + String cookie = trailers.get(ROUTING_COOKIE_METADATA_KEY); + cookiesHolder.setRoutingCookie(cookie); } super.onClose(status, trailers); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java index bc058000d9..a929f71288 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java @@ -15,7 +15,6 @@ */ package com.google.cloud.bigtable.data.v2.stub; -import com.google.protobuf.ByteString; import io.grpc.CallOptions; import io.grpc.Metadata; import javax.annotation.Nullable; @@ -28,16 +27,16 @@ class CookiesHolder { static final String ROUTING_COOKIE_KEY = "x-goog-cbt-cookie-routing"; - static final Metadata.Key ROUTING_COOKIE_METADATA_KEY = - Metadata.Key.of(ROUTING_COOKIE_KEY + "-bin", Metadata.BINARY_BYTE_MARSHALLER); + static final Metadata.Key ROUTING_COOKIE_METADATA_KEY = + Metadata.Key.of(ROUTING_COOKIE_KEY, Metadata.ASCII_STRING_MARSHALLER); - @Nullable private ByteString routingCookie; + @Nullable private String routingCookie; - void setRoutingCookie(@Nullable ByteString routingCookie) { + void setRoutingCookie(@Nullable String routingCookie) { this.routingCookie = routingCookie; } - ByteString getRoutingCookie() { + String getRoutingCookie() { return this.routingCookie; } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java index de48f0546a..0c90da3369 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java @@ -29,7 +29,6 @@ import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; import com.google.cloud.bigtable.data.v2.models.Query; import com.google.common.collect.ImmutableList; -import com.google.protobuf.ByteString; import io.grpc.Metadata; import io.grpc.Server; import io.grpc.ServerCall; @@ -38,7 +37,6 @@ import io.grpc.Status; import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; @@ -52,10 +50,9 @@ public class CookieHolderTest { private Server server; private FakeService fakeService = new FakeService(); - private BigtableDataClient client; - private List serverMetadata = new ArrayList<>(); + private String testCookie = "test-routing-cookie"; @Before public void setup() throws Exception { @@ -97,9 +94,9 @@ public void testReadRowsRetryCookieIsForwarded() { assertThat(fakeService.count.get()).isGreaterThan(1); assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); - byte[] bytes = serverMetadata.get(1).get(ROUTING_COOKIE_METADATA_KEY); + String bytes = serverMetadata.get(1).get(ROUTING_COOKIE_METADATA_KEY); assertThat(bytes).isNotNull(); - assertThat(new String(bytes, StandardCharsets.UTF_8)).isEqualTo("test-routing-cookie"); + assertThat(bytes).isEqualTo(testCookie); serverMetadata.clear(); } @@ -111,9 +108,10 @@ public void testSampleRowKeysRetryCookieIsForwarded() { assertThat(fakeService.count.get()).isGreaterThan(1); assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); - byte[] bytes = serverMetadata.get(1).get(ROUTING_COOKIE_METADATA_KEY); + String bytes = serverMetadata.get(1).get(ROUTING_COOKIE_METADATA_KEY); + assertThat(bytes).isNotNull(); - assertThat(new String(bytes, StandardCharsets.UTF_8)).isEqualTo("test-routing-cookie"); + assertThat(bytes).isEqualTo(testCookie); serverMetadata.clear(); } @@ -133,9 +131,7 @@ public void readRows( ReadRowsRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - trailers.put( - ROUTING_COOKIE_METADATA_KEY, - ByteString.copyFromUtf8("test-routing-cookie").toByteArray()); + trailers.put(ROUTING_COOKIE_METADATA_KEY, testCookie); StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; @@ -149,9 +145,7 @@ public void sampleRowKeys( SampleRowKeysRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - trailers.put( - ROUTING_COOKIE_METADATA_KEY, - ByteString.copyFromUtf8("test-routing-cookie").toByteArray()); + trailers.put(ROUTING_COOKIE_METADATA_KEY, testCookie); StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; From e6ab87f2b026bc86072bcfbcf7c5d6d1816fe06c Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 6 Nov 2023 15:57:53 -0500 Subject: [PATCH 04/22] address some comments --- .../data/v2/stub/CookieInterceptor.java | 18 ++++++++--------- .../bigtable/data/v2/stub/CookiesHolder.java | 20 +++++++++++++++---- .../data/v2/stub/CookieHolderTest.java | 13 ++++++++++++ 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java index 4b426bf77d..9ace9efd09 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java @@ -41,11 +41,12 @@ public ClientCall interceptCall( channel.newCall(methodDescriptor, callOptions)) { @Override public void start(Listener responseListener, Metadata headers) { - CookiesHolder cookie = callOptions.getOption(COOKIES_HOLDER_KEY); - if (cookie != null && cookie.getRoutingCookie() != null) { - headers.put(ROUTING_COOKIE_METADATA_KEY, cookie.getRoutingCookie()); + CookiesHolder cookie = CookiesHolder.fromCallOptions(callOptions); + if (cookie != null) { + cookie.addRoutingCookieToHeaders(headers); + responseListener = new UpdateCookieListener<>(responseListener, callOptions); } - super.start(new UpdateCookieListener<>(responseListener, callOptions), headers); + super.start(responseListener, headers); } }; } @@ -62,12 +63,9 @@ static class UpdateCookieListener @Override public void onClose(Status status, Metadata trailers) { - CookiesHolder cookiesHolder = callOptions.getOption(COOKIES_HOLDER_KEY); - if (cookiesHolder != null - && trailers != null - && trailers.containsKey(ROUTING_COOKIE_METADATA_KEY)) { - String cookie = trailers.get(ROUTING_COOKIE_METADATA_KEY); - cookiesHolder.setRoutingCookie(cookie); + CookiesHolder cookiesHolder = CookiesHolder.fromCallOptions(callOptions); + if (cookiesHolder != null) { + cookiesHolder.setRoutingCookieFromTrailers(trailers); } super.onClose(status, trailers); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java index a929f71288..45d6fb20ed 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java @@ -32,11 +32,23 @@ class CookiesHolder { @Nullable private String routingCookie; - void setRoutingCookie(@Nullable String routingCookie) { - this.routingCookie = routingCookie; + /** Returns CookiesHolder if presents in CallOptions. Otherwise returns null. */ + static CookiesHolder fromCallOptions(CallOptions options) { + return options.getOption(COOKIES_HOLDER_KEY); } - String getRoutingCookie() { - return this.routingCookie; + /** Adds routing cookie to header if routing cookie is not null. */ + Metadata addRoutingCookieToHeaders(Metadata headers) { + if (headers != null && routingCookie != null) { + headers.put(ROUTING_COOKIE_METADATA_KEY, routingCookie); + } + return headers; + } + + /** Set the routing cookie from trailers to this CookiesHolder. */ + void setRoutingCookieFromTrailers(Metadata trailers) { + if (trailers != null && trailers.containsKey(ROUTING_COOKIE_METADATA_KEY)) { + this.routingCookie = trailers.get(ROUTING_COOKIE_METADATA_KEY); + } } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java index 0c90da3369..129bc678c6 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java @@ -29,7 +29,13 @@ import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; import com.google.cloud.bigtable.data.v2.models.Query; import com.google.common.collect.ImmutableList; +import io.grpc.CallOptions; +import io.grpc.Channel; +import io.grpc.ClientCall; +import io.grpc.ClientInterceptor; +import io.grpc.ForwardingServerCallListener; import io.grpc.Metadata; +import io.grpc.MethodDescriptor; import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; @@ -54,6 +60,8 @@ public class CookieHolderTest { private List serverMetadata = new ArrayList<>(); private String testCookie = "test-routing-cookie"; + private List methods = new ArrayList<>(); + @Before public void setup() throws Exception { ServerInterceptor serverInterceptor = @@ -122,6 +130,11 @@ public void tearDown() throws Exception { server.shutdown(); } + @Test + public void responsesAreNotDropped() { + + } + class FakeService extends BigtableGrpc.BigtableImplBase { private AtomicInteger count = new AtomicInteger(); From ba373a7b4d923b564e2302cebc4a6bd60928b221 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 7 Nov 2023 17:05:38 -0500 Subject: [PATCH 05/22] udpate --- .../data/v2/stub/CookieInterceptor.java | 3 - .../data/v2/stub/CookieHolderTest.java | 62 +++++++++++++------ 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java index 9ace9efd09..cf803d613c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java @@ -15,9 +15,6 @@ */ package com.google.cloud.bigtable.data.v2.stub; -import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.COOKIES_HOLDER_KEY; -import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.ROUTING_COOKIE_METADATA_KEY; - import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ClientCall; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java index 129bc678c6..ab725cb07b 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java @@ -20,6 +20,10 @@ import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.bigtable.v2.BigtableGrpc; +import com.google.bigtable.v2.MutateRowRequest; +import com.google.bigtable.v2.MutateRowResponse; +import com.google.bigtable.v2.MutateRowsRequest; +import com.google.bigtable.v2.MutateRowsResponse; import com.google.bigtable.v2.ReadRowsRequest; import com.google.bigtable.v2.ReadRowsResponse; import com.google.bigtable.v2.SampleRowKeysRequest; @@ -29,13 +33,7 @@ import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; import com.google.cloud.bigtable.data.v2.models.Query; import com.google.common.collect.ImmutableList; -import io.grpc.CallOptions; -import io.grpc.Channel; -import io.grpc.ClientCall; -import io.grpc.ClientInterceptor; -import io.grpc.ForwardingServerCallListener; import io.grpc.Metadata; -import io.grpc.MethodDescriptor; import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; @@ -60,8 +58,6 @@ public class CookieHolderTest { private List serverMetadata = new ArrayList<>(); private String testCookie = "test-routing-cookie"; - private List methods = new ArrayList<>(); - @Before public void setup() throws Exception { ServerInterceptor serverInterceptor = @@ -85,6 +81,7 @@ public ServerCall.Listener interceptCall( InstantiatingGrpcChannelProvider channelProvider = (InstantiatingGrpcChannelProvider) settings.stubSettings().getTransportChannelProvider(); + // We need to add the interceptor manually for emulator grpc channel settings .stubSettings() .setTransportChannelProvider( @@ -97,7 +94,7 @@ public ServerCall.Listener interceptCall( } @Test - public void testReadRowsRetryCookieIsForwarded() { + public void testReadRows() { client.readRows(Query.create("fake-table")).iterator().hasNext(); assertThat(fakeService.count.get()).isGreaterThan(1); @@ -110,7 +107,13 @@ public void testReadRowsRetryCookieIsForwarded() { } @Test - public void testSampleRowKeysRetryCookieIsForwarded() { + public void testMutateRows() {} + + @Test + public void testMutateRow() {} + + @Test + public void testSampleRowKeys() { client.sampleRowKeys("fake-table"); @@ -119,7 +122,7 @@ public void testSampleRowKeysRetryCookieIsForwarded() { String bytes = serverMetadata.get(1).get(ROUTING_COOKIE_METADATA_KEY); assertThat(bytes).isNotNull(); - assertThat(bytes).isEqualTo(testCookie); + assertThat(bytes).isEqualTo("sampleRowKeys"); serverMetadata.clear(); } @@ -130,11 +133,6 @@ public void tearDown() throws Exception { server.shutdown(); } - @Test - public void responsesAreNotDropped() { - - } - class FakeService extends BigtableGrpc.BigtableImplBase { private AtomicInteger count = new AtomicInteger(); @@ -144,7 +142,7 @@ public void readRows( ReadRowsRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - trailers.put(ROUTING_COOKIE_METADATA_KEY, testCookie); + trailers.put(ROUTING_COOKIE_METADATA_KEY, "readRows"); StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; @@ -153,12 +151,40 @@ public void readRows( responseObserver.onCompleted(); } + @Override + public void mutateRow( + MutateRowRequest request, StreamObserver responseObserver) { + if (count.getAndIncrement() < 1) { + Metadata trailers = new Metadata(); + trailers.put(ROUTING_COOKIE_METADATA_KEY, "mutateRow"); + StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); + responseObserver.onError(exception); + return; + } + responseObserver.onNext(MutateRowResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } + + @Override + public void mutateRows( + MutateRowsRequest request, StreamObserver responseObserver) { + if (count.getAndIncrement() < 1) { + Metadata trailers = new Metadata(); + trailers.put(ROUTING_COOKIE_METADATA_KEY, "mutateRows"); + StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); + responseObserver.onError(exception); + return; + } + responseObserver.onNext(MutateRowsResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } + @Override public void sampleRowKeys( SampleRowKeysRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - trailers.put(ROUTING_COOKIE_METADATA_KEY, testCookie); + trailers.put(ROUTING_COOKIE_METADATA_KEY, "sampleRowKeys"); StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; From be27ca87fffd80850bab063c9a89d8dd15be1153 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 9 Nov 2023 15:44:22 -0500 Subject: [PATCH 06/22] update tests and handling of retry cookie --- .../bigtable/data/v2/stub/CookiesHolder.java | 35 ++-- .../data/v2/stub/EnhancedBigtableStub.java | 4 +- .../data/v2/stub/CookieHolderTest.java | 198 ++++++++++++++++-- 3 files changed, 208 insertions(+), 29 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java index 45d6fb20ed..cf0d32cd04 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java @@ -17,38 +17,47 @@ import io.grpc.CallOptions; import io.grpc.Metadata; -import javax.annotation.Nullable; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; -/** A cookie that holds information for the retry */ +/** A cookie that holds information for retry or routing */ class CookiesHolder { static final CallOptions.Key COOKIES_HOLDER_KEY = CallOptions.Key.create("bigtable-cookies"); - static final String ROUTING_COOKIE_KEY = "x-goog-cbt-cookie-routing"; + /** Routing cookie key prefix. */ + static final String COOKIE_KEY_PREFIX = "x-goog-cbt-cookie"; - static final Metadata.Key ROUTING_COOKIE_METADATA_KEY = - Metadata.Key.of(ROUTING_COOKIE_KEY, Metadata.ASCII_STRING_MARSHALLER); - - @Nullable private String routingCookie; + /** A map that stores all the routing cookies. */ + private final Map, String> cookies = new ConcurrentHashMap<>(); /** Returns CookiesHolder if presents in CallOptions. Otherwise returns null. */ static CookiesHolder fromCallOptions(CallOptions options) { return options.getOption(COOKIES_HOLDER_KEY); } - /** Adds routing cookie to header if routing cookie is not null. */ + /** Add all the routing cookies to headers if any. */ Metadata addRoutingCookieToHeaders(Metadata headers) { - if (headers != null && routingCookie != null) { - headers.put(ROUTING_COOKIE_METADATA_KEY, routingCookie); + if (headers != null && !cookies.isEmpty()) { + for (Metadata.Key key : cookies.keySet()) headers.put(key, cookies.get(key)); } return headers; } - /** Set the routing cookie from trailers to this CookiesHolder. */ + /** + * Iterate through all the keys in trailing metadata, and add all the keys that match + * COOKIE_KEY_PREFIX to cookies. + */ void setRoutingCookieFromTrailers(Metadata trailers) { - if (trailers != null && trailers.containsKey(ROUTING_COOKIE_METADATA_KEY)) { - this.routingCookie = trailers.get(ROUTING_COOKIE_METADATA_KEY); + if (trailers != null) { + for (String key : trailers.keys()) { + if (key.startsWith(COOKIE_KEY_PREFIX)) { + Metadata.Key metadataKey = Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER); + String value = trailers.get(metadataKey); + cookies.put(metadataKey, value); + } + } } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 8d20c917fd..3539aa8ae5 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -648,7 +648,9 @@ private UnaryCallable createBulkMutateRowsCallable() { new TracedUnaryCallable<>( tracedBatcherUnaryCallable, clientContext.getTracerFactory(), spanName); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + UnaryCallable withCookie = new CookiesUnaryCallable<>(traced); + + return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java index ab725cb07b..65c381ae82 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java @@ -15,7 +15,6 @@ */ package com.google.cloud.bigtable.data.v2.stub; -import static com.google.cloud.bigtable.data.v2.stub.CookiesHolder.ROUTING_COOKIE_METADATA_KEY; import static com.google.common.truth.Truth.assertThat; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; @@ -31,7 +30,10 @@ import com.google.cloud.bigtable.data.v2.BigtableDataClient; import com.google.cloud.bigtable.data.v2.BigtableDataSettings; import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; +import com.google.cloud.bigtable.data.v2.models.BulkMutation; import com.google.cloud.bigtable.data.v2.models.Query; +import com.google.cloud.bigtable.data.v2.models.RowMutation; +import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; import com.google.common.collect.ImmutableList; import io.grpc.Metadata; import io.grpc.Server; @@ -58,6 +60,13 @@ public class CookieHolderTest { private List serverMetadata = new ArrayList<>(); private String testCookie = "test-routing-cookie"; + private Metadata.Key ROUTING_COOKIE_1 = + Metadata.Key.of("x-goog-cbt-cookie-routing", Metadata.ASCII_STRING_MARSHALLER); + private Metadata.Key ROUTING_COOKIE_2 = + Metadata.Key.of("x-goog-cbt-cookie-random", Metadata.ASCII_STRING_MARSHALLER); + private Metadata.Key BAD_KEY = + Metadata.Key.of("x-goog-cbt-not-cookie", Metadata.ASCII_STRING_MARSHALLER); + @Before public void setup() throws Exception { ServerInterceptor serverInterceptor = @@ -99,18 +108,77 @@ public void testReadRows() { assertThat(fakeService.count.get()).isGreaterThan(1); assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); - String bytes = serverMetadata.get(1).get(ROUTING_COOKIE_METADATA_KEY); - assertThat(bytes).isNotNull(); - assertThat(bytes).isEqualTo(testCookie); + String bytes1 = serverMetadata.get(1).get(ROUTING_COOKIE_1); + String bytes2 = serverMetadata.get(1).get(ROUTING_COOKIE_2); + assertThat(bytes1).isNotNull(); + assertThat(bytes1).isEqualTo("readRows"); + assertThat(bytes2).isNotNull(); + assertThat(bytes2).isEqualTo(testCookie); + + // make sure bad key is not added + assertThat(serverMetadata.get(1).get(BAD_KEY)).isNull(); serverMetadata.clear(); } @Test - public void testMutateRows() {} + public void testReadRow() { + client.readRow("fake-table", "key"); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + String bytes1 = serverMetadata.get(1).get(ROUTING_COOKIE_1); + String bytes2 = serverMetadata.get(1).get(ROUTING_COOKIE_2); + assertThat(bytes1).isNotNull(); + assertThat(bytes1).isEqualTo("readRows"); + assertThat(bytes2).isNotNull(); + assertThat(bytes2).isEqualTo(testCookie); + + // make sure bad key is not added + assertThat(serverMetadata.get(1).get(BAD_KEY)).isNull(); + + serverMetadata.clear(); + } @Test - public void testMutateRow() {} + public void testMutateRows() { + client.bulkMutateRows( + BulkMutation.create("fake-table") + .add(RowMutationEntry.create("key").setCell("cf", "q", "v"))); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + String bytes1 = serverMetadata.get(1).get(ROUTING_COOKIE_1); + String bytes2 = serverMetadata.get(1).get(ROUTING_COOKIE_2); + assertThat(bytes1).isNotNull(); + assertThat(bytes1).isEqualTo("mutateRows"); + assertThat(bytes2).isNotNull(); + assertThat(bytes2).isEqualTo(testCookie); + + // make sure bad key is not added + assertThat(serverMetadata.get(1).get(BAD_KEY)).isNull(); + + serverMetadata.clear(); + } + + @Test + public void testMutateRow() { + client.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v")); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + String bytes1 = serverMetadata.get(1).get(ROUTING_COOKIE_1); + String bytes2 = serverMetadata.get(1).get(ROUTING_COOKIE_2); + assertThat(bytes1).isNotNull(); + assertThat(bytes1).isEqualTo("mutateRow"); + assertThat(bytes2).isNotNull(); + assertThat(bytes2).isEqualTo(testCookie); + + // make sure bad key is not added + assertThat(serverMetadata.get(1).get(BAD_KEY)).isNull(); + + serverMetadata.clear(); + } @Test public void testSampleRowKeys() { @@ -119,10 +187,90 @@ public void testSampleRowKeys() { assertThat(fakeService.count.get()).isGreaterThan(1); assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); - String bytes = serverMetadata.get(1).get(ROUTING_COOKIE_METADATA_KEY); + String bytes1 = serverMetadata.get(1).get(ROUTING_COOKIE_1); + String bytes2 = serverMetadata.get(1).get(ROUTING_COOKIE_2); + assertThat(bytes1).isNotNull(); + assertThat(bytes1).isEqualTo("sampleRowKeys"); + assertThat(bytes2).isNotNull(); + assertThat(bytes2).isEqualTo(testCookie); + + // make sure bad key is not added + assertThat(serverMetadata.get(1).get(BAD_KEY)).isNull(); + + serverMetadata.clear(); + } + + @Test + public void testNoCookieSucceedReadRows() { + fakeService.returnCookie = false; + + client.readRows(Query.create("fake-table")).iterator().hasNext(); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + + assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_1)).isNull(); + assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_2)).isNull(); + serverMetadata.clear(); + } + + @Test + public void testNoCookieSucceedReadRow() { + fakeService.returnCookie = false; + + client.readRow("fake-table", "key"); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + + assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_1)).isNull(); + assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_2)).isNull(); + serverMetadata.clear(); + } + + @Test + public void testNoCookieSucceedMutateRows() { + fakeService.returnCookie = false; + + client.bulkMutateRows( + BulkMutation.create("fake-table") + .add(RowMutationEntry.create("key").setCell("cf", "q", "v"))); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + + assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_1)).isNull(); + assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_2)).isNull(); + + serverMetadata.clear(); + } + + @Test + public void testNoCookieSucceedMutateRow() { + fakeService.returnCookie = false; + + client.mutateRow(RowMutation.create("fake-table", "key").setCell("cf", "q", "v")); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + + assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_1)).isNull(); + assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_2)).isNull(); + + serverMetadata.clear(); + } + + @Test + public void testNoCookieSucceedSampleRowKeys() { + fakeService.returnCookie = false; + + client.sampleRowKeys("fake-table"); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); - assertThat(bytes).isNotNull(); - assertThat(bytes).isEqualTo("sampleRowKeys"); + assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_1)).isNull(); + assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_2)).isNull(); serverMetadata.clear(); } @@ -135,14 +283,19 @@ public void tearDown() throws Exception { class FakeService extends BigtableGrpc.BigtableImplBase { - private AtomicInteger count = new AtomicInteger(); + private boolean returnCookie = true; + private final AtomicInteger count = new AtomicInteger(); @Override public void readRows( ReadRowsRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - trailers.put(ROUTING_COOKIE_METADATA_KEY, "readRows"); + if (returnCookie) { + trailers.put(ROUTING_COOKIE_1, "readRows"); + trailers.put(ROUTING_COOKIE_2, testCookie); + trailers.put(BAD_KEY, "bad-key"); + } StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; @@ -156,7 +309,11 @@ public void mutateRow( MutateRowRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - trailers.put(ROUTING_COOKIE_METADATA_KEY, "mutateRow"); + if (returnCookie) { + trailers.put(ROUTING_COOKIE_1, "mutateRow"); + trailers.put(ROUTING_COOKIE_2, testCookie); + trailers.put(BAD_KEY, "bad-key"); + } StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; @@ -170,12 +327,19 @@ public void mutateRows( MutateRowsRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - trailers.put(ROUTING_COOKIE_METADATA_KEY, "mutateRows"); + if (returnCookie) { + trailers.put(ROUTING_COOKIE_1, "mutateRows"); + trailers.put(ROUTING_COOKIE_2, testCookie); + trailers.put(BAD_KEY, "bad-key"); + } StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; } - responseObserver.onNext(MutateRowsResponse.getDefaultInstance()); + responseObserver.onNext( + MutateRowsResponse.newBuilder() + .addEntries(MutateRowsResponse.Entry.getDefaultInstance()) + .build()); responseObserver.onCompleted(); } @@ -184,7 +348,11 @@ public void sampleRowKeys( SampleRowKeysRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - trailers.put(ROUTING_COOKIE_METADATA_KEY, "sampleRowKeys"); + if (returnCookie) { + trailers.put(ROUTING_COOKIE_1, "sampleRowKeys"); + trailers.put(ROUTING_COOKIE_2, testCookie); + trailers.put(BAD_KEY, "bad-key"); + } StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; From 1fa268d64473ca1c0a30fc5b4d3edec8a6d0d151 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 16 Nov 2023 10:21:45 -0500 Subject: [PATCH 07/22] address comments --- .../bigtable/data/v2/stub/CookiesHolder.java | 27 ++-- ...terceptor.java => CookiesInterceptor.java} | 23 +-- .../stub/CookiesServerStreamingCallable.java | 5 +- .../data/v2/stub/CookiesUnaryCallable.java | 7 +- .../data/v2/stub/EnhancedBigtableStub.java | 34 +++-- .../v2/stub/EnhancedBigtableStubSettings.java | 3 +- ...HolderTest.java => CookiesHolderTest.java} | 132 +++++++++++++++--- 7 files changed, 171 insertions(+), 60 deletions(-) rename google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/{CookieInterceptor.java => CookiesInterceptor.java} (75%) rename google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/{CookieHolderTest.java => CookiesHolderTest.java} (72%) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java index cf0d32cd04..6727cccfd1 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java @@ -17,8 +17,8 @@ import io.grpc.CallOptions; import io.grpc.Metadata; +import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; /** A cookie that holds information for retry or routing */ class CookiesHolder { @@ -30,7 +30,7 @@ class CookiesHolder { static final String COOKIE_KEY_PREFIX = "x-goog-cbt-cookie"; /** A map that stores all the routing cookies. */ - private final Map, String> cookies = new ConcurrentHashMap<>(); + private final Map, String> cookies = new HashMap<>(); /** Returns CookiesHolder if presents in CallOptions. Otherwise returns null. */ static CookiesHolder fromCallOptions(CallOptions options) { @@ -38,9 +38,9 @@ static CookiesHolder fromCallOptions(CallOptions options) { } /** Add all the routing cookies to headers if any. */ - Metadata addRoutingCookieToHeaders(Metadata headers) { - if (headers != null && !cookies.isEmpty()) { - for (Metadata.Key key : cookies.keySet()) headers.put(key, cookies.get(key)); + Metadata injectCookiesInRequestHeaders(Metadata headers) { + for (Metadata.Key key : cookies.keySet()) { + headers.put(key, cookies.get(key)); } return headers; } @@ -49,14 +49,15 @@ Metadata addRoutingCookieToHeaders(Metadata headers) { * Iterate through all the keys in trailing metadata, and add all the keys that match * COOKIE_KEY_PREFIX to cookies. */ - void setRoutingCookieFromTrailers(Metadata trailers) { - if (trailers != null) { - for (String key : trailers.keys()) { - if (key.startsWith(COOKIE_KEY_PREFIX)) { - Metadata.Key metadataKey = Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER); - String value = trailers.get(metadataKey); - cookies.put(metadataKey, value); - } + void extractCookiesFromResponseTrailers(Metadata trailers) { + if (trailers == null) { + return; + } + for (String key : trailers.keys()) { + if (key.startsWith(COOKIE_KEY_PREFIX)) { + Metadata.Key metadataKey = Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER); + String value = trailers.get(metadataKey); + cookies.put(metadataKey, value); } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java similarity index 75% rename from google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java rename to google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java index cf803d613c..d1d944420f 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookieInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java @@ -26,10 +26,10 @@ import io.grpc.Status; /** - * A cookie interceptor that checks the cookie value from returned ErrorInfo, updates the cookie + * A cookie interceptor that checks the cookie value from returned trailer, updates the cookie * holder, and inject it in the header of the next request. */ -class CookieInterceptor implements ClientInterceptor { +class CookiesInterceptor implements ClientInterceptor { @Override public ClientCall interceptCall( @@ -38,32 +38,33 @@ public ClientCall interceptCall( channel.newCall(methodDescriptor, callOptions)) { @Override public void start(Listener responseListener, Metadata headers) { + // Gets the CookiesHolder added from CookiesServerStreamingCallable and + // CookiesUnaryCallable. + // Add CookiesHolder content to request headers if there's any. CookiesHolder cookie = CookiesHolder.fromCallOptions(callOptions); if (cookie != null) { - cookie.addRoutingCookieToHeaders(headers); - responseListener = new UpdateCookieListener<>(responseListener, callOptions); + cookie.injectCookiesInRequestHeaders(headers); + responseListener = new UpdateCookieListener<>(responseListener, cookie); } super.start(responseListener, headers); } }; } + /** Add trailers to CookiesHolder if there's any. * */ static class UpdateCookieListener extends ForwardingClientCallListener.SimpleForwardingClientCallListener { - private final CallOptions callOptions; + private final CookiesHolder cookie; - UpdateCookieListener(ClientCall.Listener delegate, CallOptions callOptions) { + UpdateCookieListener(ClientCall.Listener delegate, CookiesHolder cookiesHolder) { super(delegate); - this.callOptions = callOptions; + this.cookie = cookiesHolder; } @Override public void onClose(Status status, Metadata trailers) { - CookiesHolder cookiesHolder = CookiesHolder.fromCallOptions(callOptions); - if (cookiesHolder != null) { - cookiesHolder.setRoutingCookieFromTrailers(trailers); - } + cookie.extractCookiesFromResponseTrailers(trailers); super.onClose(status, trailers); } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesServerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesServerStreamingCallable.java index f8f62a5150..0d012b8ea0 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesServerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesServerStreamingCallable.java @@ -22,7 +22,10 @@ import com.google.api.gax.rpc.ResponseObserver; import com.google.api.gax.rpc.ServerStreamingCallable; -/** Cookie callable injects a placeholder for bigtable retry cookie. */ +/** + * The cookie holder will act as operation scoped storage for all retry attempts. Each attempt's + * cookies will be merged into the value holder and will be sent out with the next retry attempt. + */ class CookiesServerStreamingCallable extends ServerStreamingCallable { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesUnaryCallable.java index 5e0b33d03a..b0d42d5955 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesUnaryCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesUnaryCallable.java @@ -22,9 +22,12 @@ import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.UnaryCallable; -/** Cookie callable injects a placeholder for bigtable retry cookie. */ +/** + * The cookie holder will act as operation scoped storage for all retry attempts. Each attempt's + * cookies will be merged into the value holder and will be sent out with the next retry attempt. + */ class CookiesUnaryCallable extends UnaryCallable { - private UnaryCallable innerCallable; + private final UnaryCallable innerCallable; CookiesUnaryCallable(UnaryCallable callable) { this.innerCallable = callable; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 3539aa8ae5..ff37ced33d 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -185,6 +185,14 @@ public static EnhancedBigtableStubSettings finalizeSettings( // workaround JWT audience issues patchCredentials(builder); + // patch cookies interceptor + InstantiatingGrpcChannelProvider.Builder transportProvider = null; + if (builder.getTransportChannelProvider() instanceof InstantiatingGrpcChannelProvider) { + transportProvider = + ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()).toBuilder(); + transportProvider.setInterceptorProvider(() -> ImmutableList.of(new CookiesInterceptor())); + } + // Inject channel priming if (settings.isRefreshingChannel()) { // Fix the credentials so that they can be shared @@ -194,20 +202,18 @@ public static EnhancedBigtableStubSettings finalizeSettings( } builder.setCredentialsProvider(FixedCredentialsProvider.create(credentials)); - // Inject the primer - InstantiatingGrpcChannelProvider transportProvider = - (InstantiatingGrpcChannelProvider) settings.getTransportChannelProvider(); - - builder.setTransportChannelProvider( - transportProvider - .toBuilder() - .setChannelPrimer( - BigtableChannelPrimer.create( - credentials, - settings.getProjectId(), - settings.getInstanceId(), - settings.getAppProfileId())) - .build()); + if (transportProvider != null) { + transportProvider.setChannelPrimer( + BigtableChannelPrimer.create( + credentials, + settings.getProjectId(), + settings.getInstanceId(), + settings.getAppProfileId())); + } + } + + if (transportProvider != null) { + builder.setTransportChannelProvider(transportProvider.build()); } ImmutableMap attributes = diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index 0e27c713fe..cffd9c85df 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java @@ -329,8 +329,7 @@ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProvi Duration.ofSeconds(10)) // wait this long before considering the connection dead // Attempts direct access to CBT service over gRPC to improve throughput, // whether the attempt is allowed is totally controlled by service owner. - .setAttemptDirectPath(true) - .setInterceptorProvider(() -> ImmutableList.of(new CookieInterceptor())); + .setAttemptDirectPath(true); } @SuppressWarnings("WeakerAccess") diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java similarity index 72% rename from google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java rename to google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index 65c381ae82..370f0dbd58 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookieHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -17,12 +17,17 @@ import static com.google.common.truth.Truth.assertThat; -import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; +import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.rpc.StatusCode; import com.google.bigtable.v2.BigtableGrpc; +import com.google.bigtable.v2.CheckAndMutateRowRequest; +import com.google.bigtable.v2.CheckAndMutateRowResponse; import com.google.bigtable.v2.MutateRowRequest; import com.google.bigtable.v2.MutateRowResponse; import com.google.bigtable.v2.MutateRowsRequest; import com.google.bigtable.v2.MutateRowsResponse; +import com.google.bigtable.v2.ReadModifyWriteRowRequest; +import com.google.bigtable.v2.ReadModifyWriteRowResponse; import com.google.bigtable.v2.ReadRowsRequest; import com.google.bigtable.v2.ReadRowsResponse; import com.google.bigtable.v2.SampleRowKeysRequest; @@ -31,11 +36,14 @@ import com.google.cloud.bigtable.data.v2.BigtableDataSettings; import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; import com.google.cloud.bigtable.data.v2.models.BulkMutation; +import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation; +import com.google.cloud.bigtable.data.v2.models.Mutation; import com.google.cloud.bigtable.data.v2.models.Query; +import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; -import com.google.common.collect.ImmutableList; import io.grpc.Metadata; +import io.grpc.MethodDescriptor; import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; @@ -44,27 +52,34 @@ import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.threeten.bp.Duration; @RunWith(JUnit4.class) -public class CookieHolderTest { +public class CookiesHolderTest { private Server server; - private FakeService fakeService = new FakeService(); + private final FakeService fakeService = new FakeService(); private BigtableDataClient client; - private List serverMetadata = new ArrayList<>(); - private String testCookie = "test-routing-cookie"; + private final List serverMetadata = new ArrayList<>(); + private final String testCookie = "test-routing-cookie"; - private Metadata.Key ROUTING_COOKIE_1 = + private final Set methods = new HashSet<>(); + + private final Metadata.Key ROUTING_COOKIE_1 = Metadata.Key.of("x-goog-cbt-cookie-routing", Metadata.ASCII_STRING_MARSHALLER); - private Metadata.Key ROUTING_COOKIE_2 = + private final Metadata.Key ROUTING_COOKIE_2 = Metadata.Key.of("x-goog-cbt-cookie-random", Metadata.ASCII_STRING_MARSHALLER); - private Metadata.Key BAD_KEY = + private final Metadata.Key BAD_KEY = Metadata.Key.of("x-goog-cbt-not-cookie", Metadata.ASCII_STRING_MARSHALLER); @Before @@ -77,6 +92,9 @@ public ServerCall.Listener interceptCall( Metadata metadata, ServerCallHandler serverCallHandler) { serverMetadata.add(metadata); + if (metadata.containsKey(ROUTING_COOKIE_1)) { + methods.add(serverCall.getMethodDescriptor().getBareMethodName()); + } return serverCallHandler.startCall(serverCall, metadata); } }; @@ -88,16 +106,27 @@ public ServerCall.Listener interceptCall( .setProjectId("fake-project") .setInstanceId("fake-instance"); - InstantiatingGrpcChannelProvider channelProvider = - (InstantiatingGrpcChannelProvider) settings.stubSettings().getTransportChannelProvider(); - // We need to add the interceptor manually for emulator grpc channel settings .stubSettings() - .setTransportChannelProvider( - channelProvider - .toBuilder() - .setInterceptorProvider(() -> ImmutableList.of(new CookieInterceptor())) - .build()); + .checkAndMutateRowSettings() + .setRetrySettings( + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofMillis(10)) + .setMaxRetryDelay(Duration.ofMinutes(1)) + .setMaxAttempts(2) + .build()) + .setRetryableCodes(StatusCode.Code.UNAVAILABLE); + + settings + .stubSettings() + .readModifyWriteRowSettings() + .setRetrySettings( + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofMillis(10)) + .setMaxRetryDelay(Duration.ofMinutes(1)) + .setMaxAttempts(2) + .build()) + .setRetryableCodes(StatusCode.Code.UNAVAILABLE); client = BigtableDataClient.create(settings.build()); } @@ -275,6 +304,37 @@ public void testNoCookieSucceedSampleRowKeys() { serverMetadata.clear(); } + @Test + public void testAllMethodsAreCalled() { + client.readRows(Query.create("fake-table")).iterator().hasNext(); + fakeService.count.set(0); + client.mutateRow(RowMutation.create("fake-table", "key").setCell("cf", "q", "v")); + fakeService.count.set(0); + client.bulkMutateRows( + BulkMutation.create("fake-table") + .add(RowMutationEntry.create("key").setCell("cf", "q", "v"))); + fakeService.count.set(0); + client.sampleRowKeys("fake-table"); + fakeService.count.set(0); + client.checkAndMutateRow( + ConditionalRowMutation.create("fake-table", "key") + .then(Mutation.create().setCell("cf", "q", "v"))); + fakeService.count.set(0); + client.readModifyWriteRow( + ReadModifyWriteRow.create("fake-table", "key").append("cf", "q", "v")); + + Set expected = + BigtableGrpc.getServiceDescriptor().getMethods().stream() + .map(MethodDescriptor::getBareMethodName) + .collect(Collectors.toSet()); + + // Exclude methods that are not supported by routing cookie + methods.addAll( + Arrays.asList("PingAndWarm", "GenerateInitialChangeStreamPartitions", "ReadChangeStream")); + + assertThat(methods).containsExactlyElementsIn(expected); + } + @After public void tearDown() throws Exception { client.close(); @@ -360,5 +420,43 @@ public void sampleRowKeys( responseObserver.onNext(SampleRowKeysResponse.getDefaultInstance()); responseObserver.onCompleted(); } + + @Override + public void checkAndMutateRow( + CheckAndMutateRowRequest request, + StreamObserver responseObserver) { + if (count.getAndIncrement() < 1) { + Metadata trailers = new Metadata(); + if (returnCookie) { + trailers.put(ROUTING_COOKIE_1, "checkAndMutate"); + trailers.put(ROUTING_COOKIE_2, testCookie); + trailers.put(BAD_KEY, "bad-key"); + } + StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); + responseObserver.onError(exception); + return; + } + responseObserver.onNext(CheckAndMutateRowResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } + + @Override + public void readModifyWriteRow( + ReadModifyWriteRowRequest request, + StreamObserver responseObserver) { + if (count.getAndIncrement() < 1) { + Metadata trailers = new Metadata(); + if (returnCookie) { + trailers.put(ROUTING_COOKIE_1, "readModifyWrite"); + trailers.put(ROUTING_COOKIE_2, testCookie); + trailers.put(BAD_KEY, "bad-key"); + } + StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); + responseObserver.onError(exception); + return; + } + responseObserver.onNext(ReadModifyWriteRowResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } } } From 91989cd7bc927774dbea1eb42764bd7479fc8c01 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 16 Nov 2023 14:14:43 -0500 Subject: [PATCH 08/22] address comments --- .../bigtable/data/v2/stub/CookiesHolder.java | 10 +++- .../data/v2/stub/CookiesInterceptor.java | 27 +++++++--- .../data/v2/stub/CookiesHolderTest.java | 52 ++++++++----------- 3 files changed, 50 insertions(+), 39 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java index 6727cccfd1..88daad5464 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java @@ -19,6 +19,7 @@ import io.grpc.Metadata; import java.util.HashMap; import java.util.Map; +import javax.annotation.Nullable; /** A cookie that holds information for retry or routing */ class CookiesHolder { @@ -32,8 +33,13 @@ class CookiesHolder { /** A map that stores all the routing cookies. */ private final Map, String> cookies = new HashMap<>(); - /** Returns CookiesHolder if presents in CallOptions. Otherwise returns null. */ + /** Returns CookiesHolder if presents in CallOptions, otherwise returns null. */ + @Nullable static CookiesHolder fromCallOptions(CallOptions options) { + // CookiesHolder should be added by CookiesServerStreamingCallable and + // CookiesUnaryCallable for most methods. However, methods like PingAndWarm + // or ReadChangeStream doesn't support routing cookie, in which case this + // method will return null. return options.getOption(COOKIES_HOLDER_KEY); } @@ -49,7 +55,7 @@ Metadata injectCookiesInRequestHeaders(Metadata headers) { * Iterate through all the keys in trailing metadata, and add all the keys that match * COOKIE_KEY_PREFIX to cookies. */ - void extractCookiesFromResponseTrailers(Metadata trailers) { + void extractCookiesFromResponseTrailers(@Nullable Metadata trailers) { if (trailers == null) { return; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java index d1d944420f..91bee7ca4a 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java @@ -24,6 +24,7 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Status; +import java.util.logging.Logger; /** * A cookie interceptor that checks the cookie value from returned trailer, updates the cookie @@ -31,6 +32,8 @@ */ class CookiesInterceptor implements ClientInterceptor { + private static final Logger LOG = Logger.getLogger(CookiesInterceptor.class.getName()); + @Override public ClientCall interceptCall( MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { @@ -41,12 +44,17 @@ public void start(Listener responseListener, Metadata headers) { // Gets the CookiesHolder added from CookiesServerStreamingCallable and // CookiesUnaryCallable. // Add CookiesHolder content to request headers if there's any. - CookiesHolder cookie = CookiesHolder.fromCallOptions(callOptions); - if (cookie != null) { - cookie.injectCookiesInRequestHeaders(headers); - responseListener = new UpdateCookieListener<>(responseListener, cookie); + try { + CookiesHolder cookie = CookiesHolder.fromCallOptions(callOptions); + if (cookie != null) { + cookie.injectCookiesInRequestHeaders(headers); + responseListener = new UpdateCookieListener<>(responseListener, cookie); + } + } catch (Throwable e) { + LOG.warning("Failed to inject cookie to request headers: " + e); + } finally { + super.start(responseListener, headers); } - super.start(responseListener, headers); } }; } @@ -64,8 +72,13 @@ static class UpdateCookieListener @Override public void onClose(Status status, Metadata trailers) { - cookie.extractCookiesFromResponseTrailers(trailers); - super.onClose(status, trailers); + try { + cookie.extractCookiesFromResponseTrailers(trailers); + } catch (Throwable e) { + LOG.warning("Failed to extract cookie from response trailers: " + e); + } finally { + super.onClose(status, trailers); + } } } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index 370f0dbd58..41182564ab 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -106,6 +106,10 @@ public ServerCall.Listener interceptCall( .setProjectId("fake-project") .setInstanceId("fake-instance"); + // Override CheckAndMutate and ReadModifyWrite retry settings here. These operations + // are currently not retryable but this could change in the future after we + // have routing cookie sends back meaningful information and changes how retry works. + // Routing cookie still needs to be respected and handled by the callables. settings .stubSettings() .checkAndMutateRowSettings() @@ -306,6 +310,10 @@ public void testNoCookieSucceedSampleRowKeys() { @Test public void testAllMethodsAreCalled() { + // This test ensures that all methods respect the retry cookie except for the ones that are + // explicitly added to the methods list. It requires that any newly method is exercised in this + // test. + // This is enforced by introspecting grpc method descriptors. client.readRows(Query.create("fake-table")).iterator().hasNext(); fakeService.count.set(0); client.mutateRow(RowMutation.create("fake-table", "key").setCell("cf", "q", "v")); @@ -351,11 +359,7 @@ public void readRows( ReadRowsRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - if (returnCookie) { - trailers.put(ROUTING_COOKIE_1, "readRows"); - trailers.put(ROUTING_COOKIE_2, testCookie); - trailers.put(BAD_KEY, "bad-key"); - } + maybePopulateCookie(trailers, "readRows"); StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; @@ -369,11 +373,7 @@ public void mutateRow( MutateRowRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - if (returnCookie) { - trailers.put(ROUTING_COOKIE_1, "mutateRow"); - trailers.put(ROUTING_COOKIE_2, testCookie); - trailers.put(BAD_KEY, "bad-key"); - } + maybePopulateCookie(trailers, "mutateRow"); StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; @@ -387,11 +387,7 @@ public void mutateRows( MutateRowsRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - if (returnCookie) { - trailers.put(ROUTING_COOKIE_1, "mutateRows"); - trailers.put(ROUTING_COOKIE_2, testCookie); - trailers.put(BAD_KEY, "bad-key"); - } + maybePopulateCookie(trailers, "mutateRows"); StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; @@ -408,11 +404,7 @@ public void sampleRowKeys( SampleRowKeysRequest request, StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - if (returnCookie) { - trailers.put(ROUTING_COOKIE_1, "sampleRowKeys"); - trailers.put(ROUTING_COOKIE_2, testCookie); - trailers.put(BAD_KEY, "bad-key"); - } + maybePopulateCookie(trailers, "sampleRowKeys"); StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; @@ -427,11 +419,7 @@ public void checkAndMutateRow( StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - if (returnCookie) { - trailers.put(ROUTING_COOKIE_1, "checkAndMutate"); - trailers.put(ROUTING_COOKIE_2, testCookie); - trailers.put(BAD_KEY, "bad-key"); - } + maybePopulateCookie(trailers, "checkAndMutate"); StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; @@ -446,11 +434,7 @@ public void readModifyWriteRow( StreamObserver responseObserver) { if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); - if (returnCookie) { - trailers.put(ROUTING_COOKIE_1, "readModifyWrite"); - trailers.put(ROUTING_COOKIE_2, testCookie); - trailers.put(BAD_KEY, "bad-key"); - } + maybePopulateCookie(trailers, "readModifyWrite"); StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; @@ -458,5 +442,13 @@ public void readModifyWriteRow( responseObserver.onNext(ReadModifyWriteRowResponse.getDefaultInstance()); responseObserver.onCompleted(); } + + private void maybePopulateCookie(Metadata trailers, String label) { + if (returnCookie) { + trailers.put(ROUTING_COOKIE_1, label); + trailers.put(ROUTING_COOKIE_2, testCookie); + trailers.put(BAD_KEY, "bad-key"); + } + } } } From 1422950de931a33fa8069b40d93af8f6b24f044f Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 16 Nov 2023 16:48:32 -0500 Subject: [PATCH 09/22] add cookie to readChangeStream --- .../bigtable/data/v2/stub/CookiesHolder.java | 3 +- .../data/v2/stub/EnhancedBigtableStub.java | 10 +++- .../data/v2/stub/CookiesHolderTest.java | 55 +++++++++++++++++-- 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java index 88daad5464..2c71d6a528 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java @@ -38,8 +38,7 @@ class CookiesHolder { static CookiesHolder fromCallOptions(CallOptions options) { // CookiesHolder should be added by CookiesServerStreamingCallable and // CookiesUnaryCallable for most methods. However, methods like PingAndWarm - // or ReadChangeStream doesn't support routing cookie, in which case this - // method will return null. + // doesn't support routing cookie, in which case this will return null. return options.getOption(COOKIES_HOLDER_KEY); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index ff37ced33d..705b3027ed 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -938,7 +938,10 @@ public Map extract( ServerStreamingCallable traced = new TracedServerStreamingCallable<>(retrying, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + ServerStreamingCallable withCookie = + new CookiesServerStreamingCallable<>(traced); + + return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -1018,7 +1021,10 @@ public Map extract( new TracedServerStreamingCallable<>( readChangeStreamUserCallable, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + ServerStreamingCallable withCookie = + new CookiesServerStreamingCallable<>(traced); + + return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index 41182564ab..bad40e7423 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -22,10 +22,14 @@ import com.google.bigtable.v2.BigtableGrpc; import com.google.bigtable.v2.CheckAndMutateRowRequest; import com.google.bigtable.v2.CheckAndMutateRowResponse; +import com.google.bigtable.v2.GenerateInitialChangeStreamPartitionsRequest; +import com.google.bigtable.v2.GenerateInitialChangeStreamPartitionsResponse; import com.google.bigtable.v2.MutateRowRequest; import com.google.bigtable.v2.MutateRowResponse; import com.google.bigtable.v2.MutateRowsRequest; import com.google.bigtable.v2.MutateRowsResponse; +import com.google.bigtable.v2.ReadChangeStreamRequest; +import com.google.bigtable.v2.ReadChangeStreamResponse; import com.google.bigtable.v2.ReadModifyWriteRowRequest; import com.google.bigtable.v2.ReadModifyWriteRowResponse; import com.google.bigtable.v2.ReadRowsRequest; @@ -39,6 +43,7 @@ import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation; import com.google.cloud.bigtable.data.v2.models.Mutation; import com.google.cloud.bigtable.data.v2.models.Query; +import com.google.cloud.bigtable.data.v2.models.ReadChangeStreamQuery; import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; @@ -52,7 +57,6 @@ import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -309,36 +313,46 @@ public void testNoCookieSucceedSampleRowKeys() { } @Test - public void testAllMethodsAreCalled() { + public void testAllMethodsAreCalled() throws InterruptedException { // This test ensures that all methods respect the retry cookie except for the ones that are // explicitly added to the methods list. It requires that any newly method is exercised in this // test. // This is enforced by introspecting grpc method descriptors. client.readRows(Query.create("fake-table")).iterator().hasNext(); + fakeService.count.set(0); client.mutateRow(RowMutation.create("fake-table", "key").setCell("cf", "q", "v")); + fakeService.count.set(0); client.bulkMutateRows( BulkMutation.create("fake-table") .add(RowMutationEntry.create("key").setCell("cf", "q", "v"))); + fakeService.count.set(0); client.sampleRowKeys("fake-table"); + fakeService.count.set(0); client.checkAndMutateRow( ConditionalRowMutation.create("fake-table", "key") .then(Mutation.create().setCell("cf", "q", "v"))); + fakeService.count.set(0); client.readModifyWriteRow( ReadModifyWriteRow.create("fake-table", "key").append("cf", "q", "v")); + fakeService.count.set(0); + client.generateInitialChangeStreamPartitions("fake-table").iterator().hasNext(); + + fakeService.count.set(0); + client.readChangeStream(ReadChangeStreamQuery.create("fake-table")).iterator().hasNext(); + Set expected = BigtableGrpc.getServiceDescriptor().getMethods().stream() .map(MethodDescriptor::getBareMethodName) .collect(Collectors.toSet()); // Exclude methods that are not supported by routing cookie - methods.addAll( - Arrays.asList("PingAndWarm", "GenerateInitialChangeStreamPartitions", "ReadChangeStream")); + methods.add("PingAndWarm"); assertThat(methods).containsExactlyElementsIn(expected); } @@ -443,6 +457,39 @@ public void readModifyWriteRow( responseObserver.onCompleted(); } + @Override + public void readChangeStream( + ReadChangeStreamRequest request, + StreamObserver responseObserver) { + if (count.getAndIncrement() < 1) { + Metadata trailers = new Metadata(); + maybePopulateCookie(trailers, "readChangeStream"); + StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); + responseObserver.onError(exception); + return; + } + responseObserver.onNext( + ReadChangeStreamResponse.newBuilder() + .setCloseStream(ReadChangeStreamResponse.CloseStream.getDefaultInstance()) + .build()); + responseObserver.onCompleted(); + } + + @Override + public void generateInitialChangeStreamPartitions( + GenerateInitialChangeStreamPartitionsRequest request, + StreamObserver responseObserver) { + if (count.getAndIncrement() < 1) { + Metadata trailers = new Metadata(); + maybePopulateCookie(trailers, "generateInitialChangeStreamPartitions"); + StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); + responseObserver.onError(exception); + return; + } + responseObserver.onNext(GenerateInitialChangeStreamPartitionsResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } + private void maybePopulateCookie(Metadata trailers, String label) { if (returnCookie) { trailers.put(ROUTING_COOKIE_1, label); From 67779d5391651bee59b9bc5b5afd6890097929cb Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 17 Nov 2023 11:26:27 -0500 Subject: [PATCH 10/22] also check headers and add a test --- .../bigtable/data/v2/stub/CookiesHolder.java | 20 ++++++- .../data/v2/stub/CookiesInterceptor.java | 15 +++++- .../data/v2/stub/CookiesHolderTest.java | 54 ++++++++++++++++++- 3 files changed, 84 insertions(+), 5 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java index 2c71d6a528..a6802c4aab 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java @@ -51,9 +51,27 @@ Metadata injectCookiesInRequestHeaders(Metadata headers) { } /** - * Iterate through all the keys in trailing metadata, and add all the keys that match + * Iterate through all the keys in initial metadata, and add all the keys that match * COOKIE_KEY_PREFIX to cookies. */ + void extractCookiesFromResponseHeaders(@Nullable Metadata headers) { + if (headers == null) { + return; + } + for (String key : headers.keys()) { + if (key.startsWith(COOKIE_KEY_PREFIX)) { + Metadata.Key metadataKey = Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER); + String value = headers.get(metadataKey); + cookies.put(metadataKey, value); + } + } + } + + /** + * Iterate through all the keys in trailing metadata, and add all the keys that match + * COOKIE_KEY_PREFIX to cookies. Values in trailers will override the value set in initial + * metadata for the same keys. + */ void extractCookiesFromResponseTrailers(@Nullable Metadata trailers) { if (trailers == null) { return; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java index 91bee7ca4a..0e1231378c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java @@ -47,7 +47,7 @@ public void start(Listener responseListener, Metadata headers) { try { CookiesHolder cookie = CookiesHolder.fromCallOptions(callOptions); if (cookie != null) { - cookie.injectCookiesInRequestHeaders(headers); + headers = cookie.injectCookiesInRequestHeaders(headers); responseListener = new UpdateCookieListener<>(responseListener, cookie); } } catch (Throwable e) { @@ -59,7 +59,7 @@ public void start(Listener responseListener, Metadata headers) { }; } - /** Add trailers to CookiesHolder if there's any. * */ + /** Add headers and trailers to CookiesHolder if there's any. * */ static class UpdateCookieListener extends ForwardingClientCallListener.SimpleForwardingClientCallListener { @@ -70,6 +70,17 @@ static class UpdateCookieListener this.cookie = cookiesHolder; } + @Override + public void onHeaders(Metadata headers) { + try { + cookie.extractCookiesFromResponseHeaders(headers); + } catch (Throwable e) { + LOG.warning("Failed to extract cookie from response headers: " + e); + } finally { + super.onHeaders(headers); + } + } + @Override public void onClose(Status status, Metadata trailers) { try { diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index bad40e7423..53c3bd38dc 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -47,6 +47,7 @@ import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; +import io.grpc.ForwardingServerCall; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Server; @@ -312,12 +313,60 @@ public void testNoCookieSucceedSampleRowKeys() { serverMetadata.clear(); } + @Test + public void testCookiesInHeaders() throws Exception { + // Send 2 cookies in the headers, with routingCookieKey and ROUTING_COOKIE_2. ROUTING_COOKIE_2 + // is also sent in the trailers so the value should be overridden. + final Metadata.Key routingCookieKey = + Metadata.Key.of("x-goog-cbt-cookie-no-override", Metadata.ASCII_STRING_MARSHALLER); + final String routingCookieValue = "no-override"; + ServerInterceptor serverInterceptor = + new ServerInterceptor() { + @Override + public ServerCall.Listener interceptCall( + ServerCall serverCall, + Metadata metadata, + ServerCallHandler serverCallHandler) { + serverMetadata.add(metadata); + + metadata.put(routingCookieKey, routingCookieValue); + return serverCallHandler.startCall( + new ForwardingServerCall.SimpleForwardingServerCall(serverCall) { + @Override + public void sendHeaders(Metadata headers) { + headers.put(routingCookieKey, routingCookieValue); + headers.put(ROUTING_COOKIE_2, "will-be-overridden"); + super.sendHeaders(headers); + } + }, + metadata); + } + }; + + Server newServer = FakeServiceBuilder.create(fakeService).intercept(serverInterceptor).start(); + + BigtableDataSettings.Builder settings = + BigtableDataSettings.newBuilderForEmulator(newServer.getPort()) + .setProjectId("fake-project") + .setInstanceId("fake-instance"); + + BigtableDataClient client = BigtableDataClient.create(settings.build()); + + client.readRows(Query.create("table")).iterator().hasNext(); + + String bytes1 = serverMetadata.get(1).get(ROUTING_COOKIE_2); + assertThat(bytes1).isEqualTo(testCookie); + String bytes2 = serverMetadata.get(1).get(routingCookieKey); + assertThat(bytes2).isEqualTo(routingCookieValue); + + newServer.shutdown(); + } + @Test public void testAllMethodsAreCalled() throws InterruptedException { // This test ensures that all methods respect the retry cookie except for the ones that are // explicitly added to the methods list. It requires that any newly method is exercised in this - // test. - // This is enforced by introspecting grpc method descriptors. + // test. This is enforced by introspecting grpc method descriptors. client.readRows(Query.create("fake-table")).iterator().hasNext(); fakeService.count.set(0); @@ -374,6 +423,7 @@ public void readRows( if (count.getAndIncrement() < 1) { Metadata trailers = new Metadata(); maybePopulateCookie(trailers, "readRows"); + responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); responseObserver.onError(exception); return; From 92f759e9bfd80d1d8b0817bcb062844c75a20b42 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 17 Nov 2023 15:32:42 -0500 Subject: [PATCH 11/22] simplify code --- .../bigtable/data/v2/stub/CookiesHolder.java | 21 ++----------------- .../data/v2/stub/CookiesInterceptor.java | 9 ++++---- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java index a6802c4aab..7d7ca6a029 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java @@ -51,28 +51,11 @@ Metadata injectCookiesInRequestHeaders(Metadata headers) { } /** - * Iterate through all the keys in initial metadata, and add all the keys that match - * COOKIE_KEY_PREFIX to cookies. - */ - void extractCookiesFromResponseHeaders(@Nullable Metadata headers) { - if (headers == null) { - return; - } - for (String key : headers.keys()) { - if (key.startsWith(COOKIE_KEY_PREFIX)) { - Metadata.Key metadataKey = Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER); - String value = headers.get(metadataKey); - cookies.put(metadataKey, value); - } - } - } - - /** - * Iterate through all the keys in trailing metadata, and add all the keys that match + * Iterate through all the keys in initial or trailing metadata, and add all the keys that match * COOKIE_KEY_PREFIX to cookies. Values in trailers will override the value set in initial * metadata for the same keys. */ - void extractCookiesFromResponseTrailers(@Nullable Metadata trailers) { + void extractCookiesFromMetadata(@Nullable Metadata trailers) { if (trailers == null) { return; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java index 0e1231378c..77387851fa 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesInterceptor.java @@ -24,6 +24,7 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Status; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -73,9 +74,9 @@ static class UpdateCookieListener @Override public void onHeaders(Metadata headers) { try { - cookie.extractCookiesFromResponseHeaders(headers); + cookie.extractCookiesFromMetadata(headers); } catch (Throwable e) { - LOG.warning("Failed to extract cookie from response headers: " + e); + LOG.log(Level.WARNING, "Failed to extract cookie from response headers.", e); } finally { super.onHeaders(headers); } @@ -84,9 +85,9 @@ public void onHeaders(Metadata headers) { @Override public void onClose(Status status, Metadata trailers) { try { - cookie.extractCookiesFromResponseTrailers(trailers); + cookie.extractCookiesFromMetadata(trailers); } catch (Throwable e) { - LOG.warning("Failed to extract cookie from response trailers: " + e); + LOG.log(Level.WARNING, "Failed to extract cookie from response trailers.", e); } finally { super.onClose(status, trailers); } From d49ce7029372d06eed08b891391ecc34224aaa2a Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 21 Nov 2023 15:52:26 -0500 Subject: [PATCH 12/22] clean up test --- .../data/v2/stub/CookiesHolderTest.java | 114 ++++++++++-------- 1 file changed, 67 insertions(+), 47 deletions(-) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index 53c3bd38dc..6045a1fc3e 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -145,16 +145,17 @@ public void testReadRows() { client.readRows(Query.create("fake-table")).iterator().hasNext(); assertThat(fakeService.count.get()).isGreaterThan(1); - assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); - String bytes1 = serverMetadata.get(1).get(ROUTING_COOKIE_1); - String bytes2 = serverMetadata.get(1).get(ROUTING_COOKIE_2); - assertThat(bytes1).isNotNull(); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + + String bytes1 = lastMetadata.get(ROUTING_COOKIE_1); + String bytes2 = lastMetadata.get(ROUTING_COOKIE_2); assertThat(bytes1).isEqualTo("readRows"); - assertThat(bytes2).isNotNull(); assertThat(bytes2).isEqualTo(testCookie); // make sure bad key is not added - assertThat(serverMetadata.get(1).get(BAD_KEY)).isNull(); + assertThat(lastMetadata.containsKey(BAD_KEY)).isFalse(); serverMetadata.clear(); } @@ -164,16 +165,17 @@ public void testReadRow() { client.readRow("fake-table", "key"); assertThat(fakeService.count.get()).isGreaterThan(1); - assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); - String bytes1 = serverMetadata.get(1).get(ROUTING_COOKIE_1); - String bytes2 = serverMetadata.get(1).get(ROUTING_COOKIE_2); - assertThat(bytes1).isNotNull(); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + + String bytes1 = lastMetadata.get(ROUTING_COOKIE_1); + String bytes2 = lastMetadata.get(ROUTING_COOKIE_2); assertThat(bytes1).isEqualTo("readRows"); - assertThat(bytes2).isNotNull(); assertThat(bytes2).isEqualTo(testCookie); // make sure bad key is not added - assertThat(serverMetadata.get(1).get(BAD_KEY)).isNull(); + assertThat(lastMetadata.containsKey(BAD_KEY)).isFalse(); serverMetadata.clear(); } @@ -185,16 +187,17 @@ public void testMutateRows() { .add(RowMutationEntry.create("key").setCell("cf", "q", "v"))); assertThat(fakeService.count.get()).isGreaterThan(1); - assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); - String bytes1 = serverMetadata.get(1).get(ROUTING_COOKIE_1); - String bytes2 = serverMetadata.get(1).get(ROUTING_COOKIE_2); - assertThat(bytes1).isNotNull(); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + + String bytes1 = lastMetadata.get(ROUTING_COOKIE_1); + String bytes2 = lastMetadata.get(ROUTING_COOKIE_2); assertThat(bytes1).isEqualTo("mutateRows"); - assertThat(bytes2).isNotNull(); assertThat(bytes2).isEqualTo(testCookie); // make sure bad key is not added - assertThat(serverMetadata.get(1).get(BAD_KEY)).isNull(); + assertThat(lastMetadata.containsKey(BAD_KEY)).isFalse(); serverMetadata.clear(); } @@ -204,16 +207,17 @@ public void testMutateRow() { client.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v")); assertThat(fakeService.count.get()).isGreaterThan(1); - assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); - String bytes1 = serverMetadata.get(1).get(ROUTING_COOKIE_1); - String bytes2 = serverMetadata.get(1).get(ROUTING_COOKIE_2); - assertThat(bytes1).isNotNull(); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + + String bytes1 = lastMetadata.get(ROUTING_COOKIE_1); + String bytes2 = lastMetadata.get(ROUTING_COOKIE_2); assertThat(bytes1).isEqualTo("mutateRow"); - assertThat(bytes2).isNotNull(); assertThat(bytes2).isEqualTo(testCookie); // make sure bad key is not added - assertThat(serverMetadata.get(1).get(BAD_KEY)).isNull(); + assertThat(lastMetadata.containsKey(BAD_KEY)).isFalse(); serverMetadata.clear(); } @@ -224,16 +228,17 @@ public void testSampleRowKeys() { client.sampleRowKeys("fake-table"); assertThat(fakeService.count.get()).isGreaterThan(1); - assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); - String bytes1 = serverMetadata.get(1).get(ROUTING_COOKIE_1); - String bytes2 = serverMetadata.get(1).get(ROUTING_COOKIE_2); - assertThat(bytes1).isNotNull(); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + + String bytes1 = lastMetadata.get(ROUTING_COOKIE_1); + String bytes2 = lastMetadata.get(ROUTING_COOKIE_2); assertThat(bytes1).isEqualTo("sampleRowKeys"); - assertThat(bytes2).isNotNull(); assertThat(bytes2).isEqualTo(testCookie); // make sure bad key is not added - assertThat(serverMetadata.get(1).get(BAD_KEY)).isNull(); + assertThat(lastMetadata.containsKey(BAD_KEY)).isFalse(); serverMetadata.clear(); } @@ -245,10 +250,15 @@ public void testNoCookieSucceedReadRows() { client.readRows(Query.create("fake-table")).iterator().hasNext(); assertThat(fakeService.count.get()).isGreaterThan(1); - assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + + assertThat(lastMetadata.containsKey(ROUTING_COOKIE_1)).isFalse(); + assertThat(lastMetadata.containsKey(ROUTING_COOKIE_2)).isFalse(); - assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_1)).isNull(); - assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_2)).isNull(); + // make sure bad key is not added + assertThat(lastMetadata.containsKey(BAD_KEY)).isFalse(); serverMetadata.clear(); } @@ -259,10 +269,13 @@ public void testNoCookieSucceedReadRow() { client.readRow("fake-table", "key"); assertThat(fakeService.count.get()).isGreaterThan(1); - assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + + assertThat(lastMetadata.containsKey(ROUTING_COOKIE_1)).isFalse(); + assertThat(lastMetadata.containsKey(ROUTING_COOKIE_2)).isFalse(); - assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_1)).isNull(); - assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_2)).isNull(); serverMetadata.clear(); } @@ -275,10 +288,12 @@ public void testNoCookieSucceedMutateRows() { .add(RowMutationEntry.create("key").setCell("cf", "q", "v"))); assertThat(fakeService.count.get()).isGreaterThan(1); - assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_1)).isNull(); - assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_2)).isNull(); + assertThat(lastMetadata.containsKey(ROUTING_COOKIE_1)).isFalse(); + assertThat(lastMetadata.containsKey(ROUTING_COOKIE_2)).isFalse(); serverMetadata.clear(); } @@ -290,10 +305,12 @@ public void testNoCookieSucceedMutateRow() { client.mutateRow(RowMutation.create("fake-table", "key").setCell("cf", "q", "v")); assertThat(fakeService.count.get()).isGreaterThan(1); - assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + assertThat(serverMetadata).hasSize(fakeService.count.get()); - assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_1)).isNull(); - assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_2)).isNull(); + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + + assertThat(lastMetadata.containsKey(ROUTING_COOKIE_1)).isFalse(); + assertThat(lastMetadata.containsKey(ROUTING_COOKIE_2)).isFalse(); serverMetadata.clear(); } @@ -305,10 +322,12 @@ public void testNoCookieSucceedSampleRowKeys() { client.sampleRowKeys("fake-table"); assertThat(fakeService.count.get()).isGreaterThan(1); - assertThat(serverMetadata.size()).isEqualTo(fakeService.count.get()); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_1)).isNull(); - assertThat(serverMetadata.get(1).get(ROUTING_COOKIE_2)).isNull(); + assertThat(lastMetadata.containsKey(ROUTING_COOKIE_1)).isFalse(); + assertThat(lastMetadata.containsKey(ROUTING_COOKIE_2)).isFalse(); serverMetadata.clear(); } @@ -354,9 +373,10 @@ public void sendHeaders(Metadata headers) { client.readRows(Query.create("table")).iterator().hasNext(); - String bytes1 = serverMetadata.get(1).get(ROUTING_COOKIE_2); + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + String bytes1 = lastMetadata.get(ROUTING_COOKIE_2); assertThat(bytes1).isEqualTo(testCookie); - String bytes2 = serverMetadata.get(1).get(routingCookieKey); + String bytes2 = lastMetadata.get(routingCookieKey); assertThat(bytes2).isEqualTo(routingCookieValue); newServer.shutdown(); From 647946494de75500719457b57fbceb8ceefbc90a Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 22 Nov 2023 11:05:33 -0500 Subject: [PATCH 13/22] clean up test --- .../data/v2/stub/CookiesHolderTest.java | 122 ++++++++++-------- 1 file changed, 71 insertions(+), 51 deletions(-) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index 6045a1fc3e..b106194bac 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -15,6 +15,8 @@ */ package com.google.cloud.bigtable.data.v2.stub; +import static com.google.cloud.bigtable.data.v2.stub.CookiesHolderTest.MetadataSubject.assertThat; +import static com.google.common.truth.Truth.assertAbout; import static com.google.common.truth.Truth.assertThat; import com.google.api.gax.retrying.RetrySettings; @@ -47,6 +49,8 @@ import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; +import com.google.common.truth.FailureMetadata; +import com.google.common.truth.Subject; import io.grpc.ForwardingServerCall; import io.grpc.Metadata; import io.grpc.MethodDescriptor; @@ -63,6 +67,7 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; +import javax.annotation.Nullable; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -149,13 +154,9 @@ public void testReadRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - String bytes1 = lastMetadata.get(ROUTING_COOKIE_1); - String bytes2 = lastMetadata.get(ROUTING_COOKIE_2); - assertThat(bytes1).isEqualTo("readRows"); - assertThat(bytes2).isEqualTo(testCookie); - - // make sure bad key is not added - assertThat(lastMetadata.containsKey(BAD_KEY)).isFalse(); + assertThat(lastMetadata) + .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); + assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -169,13 +170,9 @@ public void testReadRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - String bytes1 = lastMetadata.get(ROUTING_COOKIE_1); - String bytes2 = lastMetadata.get(ROUTING_COOKIE_2); - assertThat(bytes1).isEqualTo("readRows"); - assertThat(bytes2).isEqualTo(testCookie); - - // make sure bad key is not added - assertThat(lastMetadata.containsKey(BAD_KEY)).isFalse(); + assertThat(lastMetadata) + .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); + assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -191,13 +188,10 @@ public void testMutateRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - String bytes1 = lastMetadata.get(ROUTING_COOKIE_1); - String bytes2 = lastMetadata.get(ROUTING_COOKIE_2); - assertThat(bytes1).isEqualTo("mutateRows"); - assertThat(bytes2).isEqualTo(testCookie); - - // make sure bad key is not added - assertThat(lastMetadata.containsKey(BAD_KEY)).isFalse(); + assertThat(lastMetadata) + .containsAtLeast( + ROUTING_COOKIE_1.name(), "mutateRows", ROUTING_COOKIE_2.name(), testCookie); + assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -211,13 +205,9 @@ public void testMutateRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - String bytes1 = lastMetadata.get(ROUTING_COOKIE_1); - String bytes2 = lastMetadata.get(ROUTING_COOKIE_2); - assertThat(bytes1).isEqualTo("mutateRow"); - assertThat(bytes2).isEqualTo(testCookie); - - // make sure bad key is not added - assertThat(lastMetadata.containsKey(BAD_KEY)).isFalse(); + assertThat(lastMetadata) + .containsAtLeast(ROUTING_COOKIE_1.name(), "mutateRow", ROUTING_COOKIE_2.name(), testCookie); + assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -232,13 +222,10 @@ public void testSampleRowKeys() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - String bytes1 = lastMetadata.get(ROUTING_COOKIE_1); - String bytes2 = lastMetadata.get(ROUTING_COOKIE_2); - assertThat(bytes1).isEqualTo("sampleRowKeys"); - assertThat(bytes2).isEqualTo(testCookie); - - // make sure bad key is not added - assertThat(lastMetadata.containsKey(BAD_KEY)).isFalse(); + assertThat(lastMetadata) + .containsAtLeast( + ROUTING_COOKIE_1.name(), "sampleRowKeys", ROUTING_COOKIE_2.name(), testCookie); + assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -254,11 +241,8 @@ public void testNoCookieSucceedReadRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata.containsKey(ROUTING_COOKIE_1)).isFalse(); - assertThat(lastMetadata.containsKey(ROUTING_COOKIE_2)).isFalse(); + assertThat(lastMetadata).doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name()); - // make sure bad key is not added - assertThat(lastMetadata.containsKey(BAD_KEY)).isFalse(); serverMetadata.clear(); } @@ -273,8 +257,8 @@ public void testNoCookieSucceedReadRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata.containsKey(ROUTING_COOKIE_1)).isFalse(); - assertThat(lastMetadata.containsKey(ROUTING_COOKIE_2)).isFalse(); + assertThat(lastMetadata) + .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); serverMetadata.clear(); } @@ -292,8 +276,8 @@ public void testNoCookieSucceedMutateRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata.containsKey(ROUTING_COOKIE_1)).isFalse(); - assertThat(lastMetadata.containsKey(ROUTING_COOKIE_2)).isFalse(); + assertThat(lastMetadata) + .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); serverMetadata.clear(); } @@ -309,8 +293,8 @@ public void testNoCookieSucceedMutateRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata.containsKey(ROUTING_COOKIE_1)).isFalse(); - assertThat(lastMetadata.containsKey(ROUTING_COOKIE_2)).isFalse(); + assertThat(lastMetadata) + .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); serverMetadata.clear(); } @@ -326,8 +310,8 @@ public void testNoCookieSucceedSampleRowKeys() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata.containsKey(ROUTING_COOKIE_1)).isFalse(); - assertThat(lastMetadata.containsKey(ROUTING_COOKIE_2)).isFalse(); + assertThat(lastMetadata) + .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); serverMetadata.clear(); } @@ -374,10 +358,10 @@ public void sendHeaders(Metadata headers) { client.readRows(Query.create("table")).iterator().hasNext(); Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - String bytes1 = lastMetadata.get(ROUTING_COOKIE_2); - assertThat(bytes1).isEqualTo(testCookie); - String bytes2 = lastMetadata.get(routingCookieKey); - assertThat(bytes2).isEqualTo(routingCookieValue); + + assertThat(lastMetadata) + .containsAtLeast( + ROUTING_COOKIE_2.name(), testCookie, routingCookieKey.name(), routingCookieValue); newServer.shutdown(); } @@ -568,4 +552,40 @@ private void maybePopulateCookie(Metadata trailers, String label) { } } } + + static final class MetadataSubject extends Subject { + + @Nullable private final Metadata actual; + + public static Factory metadata() { + return MetadataSubject::new; + } + + private MetadataSubject(FailureMetadata metadata, @Nullable Metadata actual) { + super(metadata, actual); + this.actual = actual; + } + + public static MetadataSubject assertThat(@Nullable Metadata actual) { + return assertAbout(metadata()).that(actual); + } + + public void containsAtLeast(String... keyValuePairs) { + assert actual != null; + for (int i = 0; i < keyValuePairs.length; i += 2) { + check("containsAtLeast()") + .that(actual.get(Metadata.Key.of(keyValuePairs[i], Metadata.ASCII_STRING_MARSHALLER))) + .isEqualTo(keyValuePairs[i + 1]); + } + } + + public void doesNotContainKeys(String... keys) { + assert actual != null; + for (String key : keys) { + check("doesNotContainKeys()") + .that(actual.containsKey(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER))) + .isFalse(); + } + } + } } From 3b4c17870d090b55f62df7a37d7dad61e2856c5d Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 22 Nov 2023 13:16:05 -0500 Subject: [PATCH 14/22] update dependency --- google-cloud-bigtable/pom.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml index 00049be30e..acde9888ce 100644 --- a/google-cloud-bigtable/pom.xml +++ b/google-cloud-bigtable/pom.xml @@ -289,7 +289,6 @@ com.google.truth truth - test From abad657a6f6f9f70cdd5730c68fa302f984d094e Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 22 Nov 2023 14:26:06 -0500 Subject: [PATCH 15/22] test --- .kokoro/build.sh | 2 + google-cloud-bigtable/pom.xml | 1 + .../data/v2/stub/CookiesHolderTest.java | 42 +++++++++---------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/.kokoro/build.sh b/.kokoro/build.sh index 605555ecae..833f28619a 100755 --- a/.kokoro/build.sh +++ b/.kokoro/build.sh @@ -27,6 +27,8 @@ source ${scriptDir}/common.sh mvn -version echo ${JOB_TYPE} +mvn dependency:tree + # attempt to install 3 times with exponential backoff (starting with 10 seconds) retry_with_backoff 3 10 \ mvn install -B -V -ntp \ diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml index acde9888ce..00049be30e 100644 --- a/google-cloud-bigtable/pom.xml +++ b/google-cloud-bigtable/pom.xml @@ -289,6 +289,7 @@ com.google.truth truth + test diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index b106194bac..05c74d3744 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -15,7 +15,6 @@ */ package com.google.cloud.bigtable.data.v2.stub; -import static com.google.cloud.bigtable.data.v2.stub.CookiesHolderTest.MetadataSubject.assertThat; import static com.google.common.truth.Truth.assertAbout; import static com.google.common.truth.Truth.assertThat; @@ -154,9 +153,9 @@ public void testReadRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) + MetadataSubject.assertThat(lastMetadata) .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); - assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); + MetadataSubject.assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -170,9 +169,9 @@ public void testReadRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) + MetadataSubject.assertThat(lastMetadata) .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); - assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); + MetadataSubject.assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -188,10 +187,9 @@ public void testMutateRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) - .containsAtLeast( - ROUTING_COOKIE_1.name(), "mutateRows", ROUTING_COOKIE_2.name(), testCookie); - assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); + MetadataSubject.assertThat(lastMetadata) + .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); + MetadataSubject.assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -205,9 +203,9 @@ public void testMutateRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) - .containsAtLeast(ROUTING_COOKIE_1.name(), "mutateRow", ROUTING_COOKIE_2.name(), testCookie); - assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); + MetadataSubject.assertThat(lastMetadata) + .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); + MetadataSubject.assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -222,10 +220,9 @@ public void testSampleRowKeys() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) - .containsAtLeast( - ROUTING_COOKIE_1.name(), "sampleRowKeys", ROUTING_COOKIE_2.name(), testCookie); - assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); + MetadataSubject.assertThat(lastMetadata) + .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); + MetadataSubject.assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -241,7 +238,8 @@ public void testNoCookieSucceedReadRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata).doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name()); + MetadataSubject.assertThat(lastMetadata) + .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name()); serverMetadata.clear(); } @@ -257,7 +255,7 @@ public void testNoCookieSucceedReadRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) + MetadataSubject.assertThat(lastMetadata) .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); serverMetadata.clear(); @@ -276,7 +274,7 @@ public void testNoCookieSucceedMutateRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) + MetadataSubject.assertThat(lastMetadata) .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); serverMetadata.clear(); @@ -293,7 +291,7 @@ public void testNoCookieSucceedMutateRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) + MetadataSubject.assertThat(lastMetadata) .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); serverMetadata.clear(); @@ -310,7 +308,7 @@ public void testNoCookieSucceedSampleRowKeys() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) + MetadataSubject.assertThat(lastMetadata) .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); serverMetadata.clear(); @@ -359,7 +357,7 @@ public void sendHeaders(Metadata headers) { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) + MetadataSubject.assertThat(lastMetadata) .containsAtLeast( ROUTING_COOKIE_2.name(), testCookie, routingCookieKey.name(), routingCookieValue); From 9d9a4299433fb43d6d9462ec5e9680c6d8c492c5 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 22 Nov 2023 14:45:58 -0500 Subject: [PATCH 16/22] move MetadataSubject to a separate file --- .kokoro/build.sh | 2 - .../data/v2/stub/CookiesHolderTest.java | 82 +++++-------------- 2 files changed, 22 insertions(+), 62 deletions(-) diff --git a/.kokoro/build.sh b/.kokoro/build.sh index 833f28619a..605555ecae 100755 --- a/.kokoro/build.sh +++ b/.kokoro/build.sh @@ -27,8 +27,6 @@ source ${scriptDir}/common.sh mvn -version echo ${JOB_TYPE} -mvn dependency:tree - # attempt to install 3 times with exponential backoff (starting with 10 seconds) retry_with_backoff 3 10 \ mvn install -B -V -ntp \ diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index 05c74d3744..80c94581b3 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -15,7 +15,7 @@ */ package com.google.cloud.bigtable.data.v2.stub; -import static com.google.common.truth.Truth.assertAbout; +import static com.google.cloud.bigtable.data.v2.stub.MetadataSubject.assertThat; import static com.google.common.truth.Truth.assertThat; import com.google.api.gax.retrying.RetrySettings; @@ -48,8 +48,6 @@ import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; -import com.google.common.truth.FailureMetadata; -import com.google.common.truth.Subject; import io.grpc.ForwardingServerCall; import io.grpc.Metadata; import io.grpc.MethodDescriptor; @@ -66,7 +64,6 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; -import javax.annotation.Nullable; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -153,9 +150,9 @@ public void testReadRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - MetadataSubject.assertThat(lastMetadata) + assertThat(lastMetadata) .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); - MetadataSubject.assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); + assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -169,9 +166,9 @@ public void testReadRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - MetadataSubject.assertThat(lastMetadata) + assertThat(lastMetadata) .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); - MetadataSubject.assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); + assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -187,9 +184,10 @@ public void testMutateRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - MetadataSubject.assertThat(lastMetadata) - .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); - MetadataSubject.assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); + assertThat(lastMetadata) + .containsAtLeast( + ROUTING_COOKIE_1.name(), "mutateRows", ROUTING_COOKIE_2.name(), testCookie); + assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -203,9 +201,9 @@ public void testMutateRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - MetadataSubject.assertThat(lastMetadata) - .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); - MetadataSubject.assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); + assertThat(lastMetadata) + .containsAtLeast(ROUTING_COOKIE_1.name(), "mutateRow", ROUTING_COOKIE_2.name(), testCookie); + assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -220,9 +218,10 @@ public void testSampleRowKeys() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - MetadataSubject.assertThat(lastMetadata) - .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); - MetadataSubject.assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); + assertThat(lastMetadata) + .containsAtLeast( + ROUTING_COOKIE_1.name(), "sampleRowKeys", ROUTING_COOKIE_2.name(), testCookie); + assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); } @@ -238,8 +237,7 @@ public void testNoCookieSucceedReadRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - MetadataSubject.assertThat(lastMetadata) - .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name()); + assertThat(lastMetadata).doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name()); serverMetadata.clear(); } @@ -255,7 +253,7 @@ public void testNoCookieSucceedReadRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - MetadataSubject.assertThat(lastMetadata) + assertThat(lastMetadata) .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); serverMetadata.clear(); @@ -274,7 +272,7 @@ public void testNoCookieSucceedMutateRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - MetadataSubject.assertThat(lastMetadata) + assertThat(lastMetadata) .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); serverMetadata.clear(); @@ -291,7 +289,7 @@ public void testNoCookieSucceedMutateRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - MetadataSubject.assertThat(lastMetadata) + assertThat(lastMetadata) .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); serverMetadata.clear(); @@ -308,7 +306,7 @@ public void testNoCookieSucceedSampleRowKeys() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - MetadataSubject.assertThat(lastMetadata) + assertThat(lastMetadata) .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); serverMetadata.clear(); @@ -357,7 +355,7 @@ public void sendHeaders(Metadata headers) { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - MetadataSubject.assertThat(lastMetadata) + assertThat(lastMetadata) .containsAtLeast( ROUTING_COOKIE_2.name(), testCookie, routingCookieKey.name(), routingCookieValue); @@ -550,40 +548,4 @@ private void maybePopulateCookie(Metadata trailers, String label) { } } } - - static final class MetadataSubject extends Subject { - - @Nullable private final Metadata actual; - - public static Factory metadata() { - return MetadataSubject::new; - } - - private MetadataSubject(FailureMetadata metadata, @Nullable Metadata actual) { - super(metadata, actual); - this.actual = actual; - } - - public static MetadataSubject assertThat(@Nullable Metadata actual) { - return assertAbout(metadata()).that(actual); - } - - public void containsAtLeast(String... keyValuePairs) { - assert actual != null; - for (int i = 0; i < keyValuePairs.length; i += 2) { - check("containsAtLeast()") - .that(actual.get(Metadata.Key.of(keyValuePairs[i], Metadata.ASCII_STRING_MARSHALLER))) - .isEqualTo(keyValuePairs[i + 1]); - } - } - - public void doesNotContainKeys(String... keys) { - assert actual != null; - for (String key : keys) { - check("doesNotContainKeys()") - .that(actual.containsKey(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER))) - .isFalse(); - } - } - } } From 660085bfdba787eba7152c5262690d8d92226da2 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 22 Nov 2023 14:47:26 -0500 Subject: [PATCH 17/22] add the file --- .../data/v2/stub/MetadataSubject.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MetadataSubject.java diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MetadataSubject.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MetadataSubject.java new file mode 100644 index 0000000000..81902c05e6 --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MetadataSubject.java @@ -0,0 +1,45 @@ +package com.google.cloud.bigtable.data.v2.stub; + +import static com.google.common.truth.Truth.assertAbout; + +import com.google.common.truth.FailureMetadata; +import com.google.common.truth.Subject; +import io.grpc.Metadata; +import javax.annotation.Nullable; + +/** Utility class to test key-value pairs in {@link io.grpc.Metadata}. */ +final class MetadataSubject extends Subject { + + @Nullable private final Metadata actual; + + public static Factory metadata() { + return MetadataSubject::new; + } + + private MetadataSubject(FailureMetadata metadata, @Nullable Metadata actual) { + super(metadata, actual); + this.actual = actual; + } + + public static MetadataSubject assertThat(@Nullable Metadata actual) { + return assertAbout(metadata()).that(actual); + } + + public void containsAtLeast(String... keyValuePairs) { + assert actual != null; + for (int i = 0; i < keyValuePairs.length; i += 2) { + check("containsAtLeast()") + .that(actual.get(Metadata.Key.of(keyValuePairs[i], Metadata.ASCII_STRING_MARSHALLER))) + .isEqualTo(keyValuePairs[i + 1]); + } + } + + public void doesNotContainKeys(String... keys) { + assert actual != null; + for (String key : keys) { + check("doesNotContainKeys()") + .that(actual.containsKey(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER))) + .isFalse(); + } + } +} From a0dd03d83a1913825d39b5945a8b426fe9d791c5 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 22 Nov 2023 14:56:01 -0500 Subject: [PATCH 18/22] add license --- .../bigtable/data/v2/stub/MetadataSubject.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MetadataSubject.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MetadataSubject.java index 81902c05e6..c1bb0d08ca 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MetadataSubject.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MetadataSubject.java @@ -1,3 +1,18 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package com.google.cloud.bigtable.data.v2.stub; import static com.google.common.truth.Truth.assertAbout; From 1aaaddcdfed47bc0ed546bb532bc7ed07b4b3158 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 22 Nov 2023 15:41:23 -0500 Subject: [PATCH 19/22] address comments --- .../data/v2/{stub => }/MetadataSubject.java | 4 +- .../data/v2/stub/CookiesHolderTest.java | 67 +++++++++++-------- 2 files changed, 40 insertions(+), 31 deletions(-) rename google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/{stub => }/MetadataSubject.java (95%) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MetadataSubject.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/MetadataSubject.java similarity index 95% rename from google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MetadataSubject.java rename to google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/MetadataSubject.java index c1bb0d08ca..5e76ce38cd 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MetadataSubject.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/MetadataSubject.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.google.cloud.bigtable.data.v2.stub; +package com.google.cloud.bigtable.data.v2; import static com.google.common.truth.Truth.assertAbout; @@ -23,7 +23,7 @@ import javax.annotation.Nullable; /** Utility class to test key-value pairs in {@link io.grpc.Metadata}. */ -final class MetadataSubject extends Subject { +public final class MetadataSubject extends Subject { @Nullable private final Metadata actual; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index 80c94581b3..3b71f5891b 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -15,7 +15,7 @@ */ package com.google.cloud.bigtable.data.v2.stub; -import static com.google.cloud.bigtable.data.v2.stub.MetadataSubject.assertThat; +import static com.google.cloud.bigtable.data.v2.MetadataSubject.assertThat; import static com.google.common.truth.Truth.assertThat; import com.google.api.gax.retrying.RetrySettings; @@ -73,21 +73,21 @@ @RunWith(JUnit4.class) public class CookiesHolderTest { + private static final Metadata.Key ROUTING_COOKIE_1 = + Metadata.Key.of("x-goog-cbt-cookie-routing", Metadata.ASCII_STRING_MARSHALLER); + private static final Metadata.Key ROUTING_COOKIE_2 = + Metadata.Key.of("x-goog-cbt-cookie-random", Metadata.ASCII_STRING_MARSHALLER); + private static final Metadata.Key BAD_KEY = + Metadata.Key.of("x-goog-cbt-not-cookie", Metadata.ASCII_STRING_MARSHALLER); + private static final String testCookie = "test-routing-cookie"; + private Server server; private final FakeService fakeService = new FakeService(); private BigtableDataClient client; private final List serverMetadata = new ArrayList<>(); - private final String testCookie = "test-routing-cookie"; private final Set methods = new HashSet<>(); - private final Metadata.Key ROUTING_COOKIE_1 = - Metadata.Key.of("x-goog-cbt-cookie-routing", Metadata.ASCII_STRING_MARSHALLER); - private final Metadata.Key ROUTING_COOKIE_2 = - Metadata.Key.of("x-goog-cbt-cookie-random", Metadata.ASCII_STRING_MARSHALLER); - private final Metadata.Key BAD_KEY = - Metadata.Key.of("x-goog-cbt-not-cookie", Metadata.ASCII_STRING_MARSHALLER); - @Before public void setup() throws Exception { ServerInterceptor serverInterceptor = @@ -141,6 +141,16 @@ public ServerCall.Listener interceptCall( client = BigtableDataClient.create(settings.build()); } + @After + public void tearDown() throws Exception { + if (client != null) { + client.close(); + } + if (server != null) { + server.shutdown(); + } + } + @Test public void testReadRows() { client.readRows(Query.create("fake-table")).iterator().hasNext(); @@ -342,24 +352,29 @@ public void sendHeaders(Metadata headers) { } }; - Server newServer = FakeServiceBuilder.create(fakeService).intercept(serverInterceptor).start(); + Server newServer = null; + try { + newServer = FakeServiceBuilder.create(fakeService).intercept(serverInterceptor).start(); - BigtableDataSettings.Builder settings = - BigtableDataSettings.newBuilderForEmulator(newServer.getPort()) - .setProjectId("fake-project") - .setInstanceId("fake-instance"); + BigtableDataSettings.Builder settings = + BigtableDataSettings.newBuilderForEmulator(newServer.getPort()) + .setProjectId("fake-project") + .setInstanceId("fake-instance"); - BigtableDataClient client = BigtableDataClient.create(settings.build()); + BigtableDataClient client = BigtableDataClient.create(settings.build()); - client.readRows(Query.create("table")).iterator().hasNext(); + client.readRows(Query.create("table")).iterator().hasNext(); - Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - - assertThat(lastMetadata) - .containsAtLeast( - ROUTING_COOKIE_2.name(), testCookie, routingCookieKey.name(), routingCookieValue); + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - newServer.shutdown(); + assertThat(lastMetadata) + .containsAtLeast( + ROUTING_COOKIE_2.name(), testCookie, routingCookieKey.name(), routingCookieValue); + } finally { + if (newServer != null) { + newServer.shutdown(); + } + } } @Test @@ -406,13 +421,7 @@ public void testAllMethodsAreCalled() throws InterruptedException { assertThat(methods).containsExactlyElementsIn(expected); } - @After - public void tearDown() throws Exception { - client.close(); - server.shutdown(); - } - - class FakeService extends BigtableGrpc.BigtableImplBase { + static class FakeService extends BigtableGrpc.BigtableImplBase { private boolean returnCookie = true; private final AtomicInteger count = new AtomicInteger(); From 00eb5dfa7cdc3478b517f314069073616ee571a1 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 22 Nov 2023 15:56:17 -0500 Subject: [PATCH 20/22] close client --- .../bigtable/data/v2/stub/CookiesHolderTest.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index 3b71f5891b..5dac053523 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -361,15 +361,16 @@ public void sendHeaders(Metadata headers) { .setProjectId("fake-project") .setInstanceId("fake-instance"); - BigtableDataClient client = BigtableDataClient.create(settings.build()); + try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { - client.readRows(Query.create("table")).iterator().hasNext(); + client.readRows(Query.create("table")).iterator().hasNext(); - Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) - .containsAtLeast( - ROUTING_COOKIE_2.name(), testCookie, routingCookieKey.name(), routingCookieValue); + assertThat(lastMetadata) + .containsAtLeast( + ROUTING_COOKIE_2.name(), testCookie, routingCookieKey.name(), routingCookieValue); + } } finally { if (newServer != null) { newServer.shutdown(); From 6a33c0a8609ce638de2d4b783b7ce189add01087 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Wed, 22 Nov 2023 21:17:35 +0000 Subject: [PATCH 21/22] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 0f12ce4ec0..99bb4df2a3 100644 --- a/README.md +++ b/README.md @@ -50,20 +50,20 @@ If you are using Maven without the BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.24.0') +implementation platform('com.google.cloud:libraries-bom:26.27.0') implementation 'com.google.cloud:google-cloud-bigtable' ``` If you are using Gradle without BOM, add this to your dependencies: ```Groovy -implementation 'com.google.cloud:google-cloud-bigtable:2.28.0' +implementation 'com.google.cloud:google-cloud-bigtable:2.29.1' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-bigtable" % "2.28.0" +libraryDependencies += "com.google.cloud" % "google-cloud-bigtable" % "2.29.1" ``` @@ -609,7 +609,7 @@ Java is a registered trademark of Oracle and/or its affiliates. [kokoro-badge-link-5]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-bigtable/java11.html [stability-image]: https://img.shields.io/badge/stability-stable-green [maven-version-image]: https://img.shields.io/maven-central/v/com.google.cloud/google-cloud-bigtable.svg -[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-bigtable/2.28.0 +[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-bigtable/2.29.1 [authentication]: https://github.com/googleapis/google-cloud-java#authentication [auth-scopes]: https://developers.google.com/identity/protocols/oauth2/scopes [predefined-iam-roles]: https://cloud.google.com/iam/docs/understanding-roles#predefined_roles From f76fac7cacf74fff7c859410501fa8b26725cdeb Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Wed, 22 Nov 2023 21:55:50 +0000 Subject: [PATCH 22/22] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 736624d00a..5caf555e94 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ If you are using Maven without the BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.26.0') +implementation platform('com.google.cloud:libraries-bom:26.27.0') implementation 'com.google.cloud:google-cloud-bigtable' ```