From 40573abe05b5c3eeab9560c13fa21e86a2765a13 Mon Sep 17 00:00:00 2001 From: Igor Berntein Date: Tue, 9 Jan 2024 11:01:09 -0500 Subject: [PATCH 1/3] feat: count row merging errors as internal errors Currently they dont have a status associated and thus get counted as UNKOWN Change-Id: Ida3470a0609f2e2ad51534eb3141db394af1dcdc --- .../data/v2/stub/readrows/StateMachine.java | 19 +++- .../data/v2/functional/ReadRowsTest.java | 99 +++++++++++++++++++ 2 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/functional/ReadRowsTest.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachine.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachine.java index 6791679829..32b35f0637 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachine.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachine.java @@ -15,6 +15,9 @@ */ package com.google.cloud.bigtable.data.v2.stub.readrows; +import com.google.api.gax.grpc.GrpcStatusCode; +import com.google.api.gax.rpc.ApiException; +import com.google.api.gax.rpc.InternalException; import com.google.bigtable.v2.ReadRowsResponse.CellChunk; import com.google.cloud.bigtable.data.v2.internal.ByteStringComparator; import com.google.cloud.bigtable.data.v2.models.RowAdapter.RowBuilder; @@ -22,6 +25,8 @@ import com.google.common.base.Preconditions; import com.google.common.collect.EvictingQueue; import com.google.protobuf.ByteString; +import io.grpc.Status; + import java.util.List; /** @@ -252,6 +257,16 @@ State handleChunk(CellChunk chunk) { new State() { @Override State handleLastScannedRow(ByteString rowKey) { + if (lastCompleteRowKey != null) { + int cmp = ByteStringComparator.INSTANCE.compare(lastCompleteRowKey, rowKey); + String direction = "increasing"; + if (reversed) { + cmp *= -1; + direction = "decreasing"; + } + + validate(cmp < 0, "AWAITING_NEW_ROW: key must be strictly " + direction); + } completeRow = adapter.createScanMarkerRow(rowKey); lastCompleteRowKey = rowKey; return AWAITING_ROW_CONSUME; @@ -468,9 +483,9 @@ private void validate(boolean condition, String message) { } } - static class InvalidInputException extends RuntimeException { + static class InvalidInputException extends InternalException { InvalidInputException(String message) { - super(message); + super(message, null, GrpcStatusCode.of(Status.Code.INTERNAL), false); } } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/functional/ReadRowsTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/functional/ReadRowsTest.java new file mode 100644 index 0000000000..bb7a1ca300 --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/functional/ReadRowsTest.java @@ -0,0 +1,99 @@ +package com.google.cloud.bigtable.data.v2.functional; + +import com.google.api.gax.rpc.InternalException; +import com.google.bigtable.v2.BigtableGrpc; +import com.google.bigtable.v2.ReadRowsRequest; +import com.google.bigtable.v2.ReadRowsResponse; +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.Query; +import com.google.cloud.bigtable.data.v2.models.Row; +import com.google.protobuf.ByteString; +import com.google.protobuf.BytesValue; +import com.google.protobuf.StringValue; +import io.grpc.Server; +import io.grpc.stub.StreamObserver; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +@RunWith(JUnit4.class) +public class ReadRowsTest { + private FakeService service; + private Server server; + + @Before + public void setUp() throws Exception { + service = new FakeService(); + server = FakeServiceBuilder.create(service) + .start(); + } + + @After + public void tearDown() throws Exception { + server.shutdown(); + } + + @Test + public void rowMergingErrorsUseInternalStatus() throws Exception { + BigtableDataSettings settings = BigtableDataSettings.newBuilderForEmulator(server.getPort()) + .setProjectId("fake-project") + .setInstanceId("fake-instance") + .build(); + + service.readRowsResponses.add( + ReadRowsResponse.newBuilder() + .addChunks( + ReadRowsResponse.CellChunk.newBuilder() + .setRowKey(ByteString.copyFromUtf8("z")) + .setFamilyName(StringValue.newBuilder().setValue("f")) + .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q")).build()) + .setTimestampMicros(1000) + .setValue(ByteString.copyFromUtf8("v")) + .setCommitRow(true) + ) + .addChunks( + ReadRowsResponse.CellChunk.newBuilder() + .setRowKey(ByteString.copyFromUtf8("a")) + .setFamilyName(StringValue.newBuilder().setValue("f")) + .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q")).build()) + .setTimestampMicros(1000) + .setValue(ByteString.copyFromUtf8("v")) + .setCommitRow(true) + ) + .build() + ); + + try (BigtableDataClient client = BigtableDataClient.create(settings)) { + Assert.assertThrows( + InternalException.class, + () -> { + for (Row ignored : client.readRows(Query.create("fake-table"))) { + + } + } + ); + } + } + + + static class FakeService extends BigtableGrpc.BigtableImplBase { + private List readRowsResponses = Collections.synchronizedList(new ArrayList<>()); + + @Override + public void readRows(ReadRowsRequest request, StreamObserver responseObserver) { + for (ReadRowsResponse r : readRowsResponses) { + responseObserver.onNext(r); + } + responseObserver.onCompleted(); + } + } +} From 2de12c4ad3115509100143b8e810554c9deabd16 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Tue, 9 Jan 2024 16:03:43 +0000 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-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 --- .../data/v2/stub/readrows/StateMachine.java | 2 - .../data/v2/functional/ReadRowsTest.java | 124 +++++++++--------- 2 files changed, 60 insertions(+), 66 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachine.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachine.java index 32b35f0637..e37f3f0dff 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachine.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachine.java @@ -16,7 +16,6 @@ package com.google.cloud.bigtable.data.v2.stub.readrows; import com.google.api.gax.grpc.GrpcStatusCode; -import com.google.api.gax.rpc.ApiException; import com.google.api.gax.rpc.InternalException; import com.google.bigtable.v2.ReadRowsResponse.CellChunk; import com.google.cloud.bigtable.data.v2.internal.ByteStringComparator; @@ -26,7 +25,6 @@ import com.google.common.collect.EvictingQueue; import com.google.protobuf.ByteString; import io.grpc.Status; - import java.util.List; /** diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/functional/ReadRowsTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/functional/ReadRowsTest.java index bb7a1ca300..86af4a62c2 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/functional/ReadRowsTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/functional/ReadRowsTest.java @@ -14,6 +14,9 @@ import com.google.protobuf.StringValue; import io.grpc.Server; import io.grpc.stub.StreamObserver; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -21,79 +24,72 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - @RunWith(JUnit4.class) public class ReadRowsTest { - private FakeService service; - private Server server; + private FakeService service; + private Server server; - @Before - public void setUp() throws Exception { - service = new FakeService(); - server = FakeServiceBuilder.create(service) - .start(); - } + @Before + public void setUp() throws Exception { + service = new FakeService(); + server = FakeServiceBuilder.create(service).start(); + } - @After - public void tearDown() throws Exception { - server.shutdown(); - } + @After + public void tearDown() throws Exception { + server.shutdown(); + } - @Test - public void rowMergingErrorsUseInternalStatus() throws Exception { - BigtableDataSettings settings = BigtableDataSettings.newBuilderForEmulator(server.getPort()) - .setProjectId("fake-project") - .setInstanceId("fake-instance") - .build(); + @Test + public void rowMergingErrorsUseInternalStatus() throws Exception { + BigtableDataSettings settings = + BigtableDataSettings.newBuilderForEmulator(server.getPort()) + .setProjectId("fake-project") + .setInstanceId("fake-instance") + .build(); - service.readRowsResponses.add( - ReadRowsResponse.newBuilder() - .addChunks( - ReadRowsResponse.CellChunk.newBuilder() - .setRowKey(ByteString.copyFromUtf8("z")) - .setFamilyName(StringValue.newBuilder().setValue("f")) - .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q")).build()) - .setTimestampMicros(1000) - .setValue(ByteString.copyFromUtf8("v")) - .setCommitRow(true) - ) - .addChunks( - ReadRowsResponse.CellChunk.newBuilder() - .setRowKey(ByteString.copyFromUtf8("a")) - .setFamilyName(StringValue.newBuilder().setValue("f")) - .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q")).build()) - .setTimestampMicros(1000) - .setValue(ByteString.copyFromUtf8("v")) - .setCommitRow(true) - ) - .build() - ); + service.readRowsResponses.add( + ReadRowsResponse.newBuilder() + .addChunks( + ReadRowsResponse.CellChunk.newBuilder() + .setRowKey(ByteString.copyFromUtf8("z")) + .setFamilyName(StringValue.newBuilder().setValue("f")) + .setQualifier( + BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q")).build()) + .setTimestampMicros(1000) + .setValue(ByteString.copyFromUtf8("v")) + .setCommitRow(true)) + .addChunks( + ReadRowsResponse.CellChunk.newBuilder() + .setRowKey(ByteString.copyFromUtf8("a")) + .setFamilyName(StringValue.newBuilder().setValue("f")) + .setQualifier( + BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q")).build()) + .setTimestampMicros(1000) + .setValue(ByteString.copyFromUtf8("v")) + .setCommitRow(true)) + .build()); - try (BigtableDataClient client = BigtableDataClient.create(settings)) { - Assert.assertThrows( - InternalException.class, - () -> { - for (Row ignored : client.readRows(Query.create("fake-table"))) { - - } - } - ); - } + try (BigtableDataClient client = BigtableDataClient.create(settings)) { + Assert.assertThrows( + InternalException.class, + () -> { + for (Row ignored : client.readRows(Query.create("fake-table"))) {} + }); } + } + static class FakeService extends BigtableGrpc.BigtableImplBase { + private List readRowsResponses = + Collections.synchronizedList(new ArrayList<>()); - static class FakeService extends BigtableGrpc.BigtableImplBase { - private List readRowsResponses = Collections.synchronizedList(new ArrayList<>()); - - @Override - public void readRows(ReadRowsRequest request, StreamObserver responseObserver) { - for (ReadRowsResponse r : readRowsResponses) { - responseObserver.onNext(r); - } - responseObserver.onCompleted(); - } + @Override + public void readRows( + ReadRowsRequest request, StreamObserver responseObserver) { + for (ReadRowsResponse r : readRowsResponses) { + responseObserver.onNext(r); + } + responseObserver.onCompleted(); } + } } From a46957c15a0e9481abf8d7fc42361ce6961eb5ad Mon Sep 17 00:00:00 2001 From: Igor Berntein Date: Tue, 9 Jan 2024 11:17:26 -0500 Subject: [PATCH 3/3] format Change-Id: Iccb2a38b78e5f6c420cb1656887beebaecfa02d2 --- .../data/v2/stub/readrows/StateMachine.java | 7 ++++++- .../bigtable/data/v2/functional/ReadRowsTest.java | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachine.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachine.java index e37f3f0dff..01d9ec6abb 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachine.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachine.java @@ -263,7 +263,12 @@ State handleLastScannedRow(ByteString rowKey) { direction = "decreasing"; } - validate(cmp < 0, "AWAITING_NEW_ROW: key must be strictly " + direction); + validate( + cmp < 0, + "AWAITING_NEW_ROW: last scanned key must be strictly " + + direction + + ". New last scanned key=" + + rowKey); } completeRow = adapter.createScanMarkerRow(rowKey); lastCompleteRowKey = rowKey; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/functional/ReadRowsTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/functional/ReadRowsTest.java index 86af4a62c2..1a74eb5aa8 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/functional/ReadRowsTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/functional/ReadRowsTest.java @@ -1,3 +1,18 @@ +/* + * Copyright 2024 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.functional; import com.google.api.gax.rpc.InternalException;