From 483656ecef77d9b7edea58c831fba7bd799c8e53 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Tue, 5 May 2020 18:11:45 -0400 Subject: [PATCH 1/3] chore: add more context to row merging errors --- .../data/v2/stub/readrows/StateMachine.java | 25 +++++- .../v2/stub/readrows/StateMachineTest.java | 77 +++++++++++++++++++ 2 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachineTest.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 5703dc9a7..0e8d1ad01 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 @@ -77,6 +77,13 @@ final class StateMachine { private State currentState; private ByteString lastCompleteRowKey; + // debug stats + private int numScannedNotifications = 0; + private int numRowsCommitted = 0; + private int numChunksProcessed = 0; + private int numCellsInRow = 0; + private int numCellsInLastRow = 0; + // Track current cell attributes: protocol omits them when they are repeated private ByteString rowKey; private String familyName; @@ -120,6 +127,7 @@ final class StateMachine { */ void handleLastScannedRow(ByteString key) { try { + numScannedNotifications++; currentState = currentState.handleLastScannedRow(key); } catch (RuntimeException e) { currentState = null; @@ -148,6 +156,7 @@ void handleLastScannedRow(ByteString key) { */ void handleChunk(CellChunk chunk) { try { + numChunksProcessed++; currentState = currentState.handleChunk(chunk); } catch (RuntimeException e) { currentState = null; @@ -191,6 +200,7 @@ private void reset() { expectedCellSize = 0; remainingCellBytes = 0; completeRow = null; + numCellsInRow = 0; adapter.reset(); } @@ -326,6 +336,7 @@ State handleChunk(CellChunk chunk) { return AWAITING_CELL_VALUE; } adapter.finishCell(); + numCellsInRow++; if (!chunk.getCommitRow()) { return AWAITING_NEW_CELL; @@ -374,6 +385,7 @@ State handleChunk(CellChunk chunk) { return AWAITING_CELL_VALUE; } adapter.finishCell(); + numCellsInRow++; if (!chunk.getCommitRow()) { return AWAITING_NEW_CELL; @@ -416,12 +428,21 @@ private State handleCommit() { validate(remainingCellBytes == 0, "Can't commit with remaining bytes"); completeRow = adapter.finishRow(); lastCompleteRowKey = rowKey; + numRowsCommitted++; + numCellsInLastRow = numCellsInRow; return AWAITING_ROW_CONSUME; } - private static void validate(boolean condition, String message) { + private void validate(boolean condition, String message) { if (!condition) { - throw new InvalidInputException(message); + throw new InvalidInputException(message + + ". numScannedNotifications: " + numScannedNotifications + + ", numRowsCommitted: " + numRowsCommitted + + ", numChunksProcessed: " + numChunksProcessed + + ", numCellsInRow: " + numCellsInRow + + ", numCellsInLastRow: " + numCellsInLastRow + + ", rowKey: " + rowKey + + ", lastCompleteRowKey: " + lastCompleteRowKey); } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachineTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachineTest.java new file mode 100644 index 000000000..8de616431 --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachineTest.java @@ -0,0 +1,77 @@ +/* + * Copyright 2020 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.readrows; + +import com.google.bigtable.v2.ReadRowsResponse; +import com.google.cloud.bigtable.data.v2.models.DefaultRowAdapter; +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 org.junit.Before; +import org.junit.Test; + +import static com.google.common.truth.Truth.assertThat; + +public class StateMachineTest { + StateMachine stateMachine; + + @Before + public void setUp() throws Exception { + stateMachine = new StateMachine<>(new DefaultRowAdapter().createRowBuilder()); + } + + @Test + public void testErrorHandlingStats() { + StateMachine.InvalidInputException actualError = null; + + + ReadRowsResponse.CellChunk chunk = ReadRowsResponse.CellChunk.newBuilder() + .setRowKey(ByteString.copyFromUtf8("my-key1")) + .setFamilyName(StringValue.newBuilder().setValue("my-family")) + .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) + .setTimestampMicros(1_000) + .setValue(ByteString.copyFromUtf8("my-value")) + .setCommitRow(true) + .build(); + try { + stateMachine.handleChunk(chunk); + stateMachine.consumeRow(); + stateMachine.handleChunk( + chunk.toBuilder() + .setRowKey(ByteString.copyFromUtf8("my-key2")) + .setValueSize(123) // invalid value size + .build() + ); + } catch (StateMachine.InvalidInputException e) { + actualError = e; + } + + assertThat(actualError).hasMessageThat() + .contains("my-key1"); + assertThat(actualError).hasMessageThat() + .contains("my-key2"); + assertThat(actualError).hasMessageThat() + .contains("numScannedNotifications: 0"); + assertThat(actualError).hasMessageThat() + .contains("numChunksProcessed: 2"); + assertThat(actualError).hasMessageThat() + .contains("numCellsInRow: 0"); + assertThat(actualError).hasMessageThat() + .contains("numCellsInLastRow: 1"); + + } +} \ No newline at end of file From f9fa5819711e834b97a513acc7611b0cfbdbd5ed Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 6 May 2020 10:51:03 -0400 Subject: [PATCH 2/3] reformat --- .../data/v2/stub/readrows/StateMachine.java | 24 +++-- .../v2/stub/readrows/StateMachineTest.java | 88 +++++++++---------- 2 files changed, 58 insertions(+), 54 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 0e8d1ad01..9aedb507f 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 @@ -435,14 +435,22 @@ private State handleCommit() { private void validate(boolean condition, String message) { if (!condition) { - throw new InvalidInputException(message - + ". numScannedNotifications: " + numScannedNotifications - + ", numRowsCommitted: " + numRowsCommitted - + ", numChunksProcessed: " + numChunksProcessed - + ", numCellsInRow: " + numCellsInRow - + ", numCellsInLastRow: " + numCellsInLastRow - + ", rowKey: " + rowKey - + ", lastCompleteRowKey: " + lastCompleteRowKey); + throw new InvalidInputException( + message + + ". numScannedNotifications: " + + numScannedNotifications + + ", numRowsCommitted: " + + numRowsCommitted + + ", numChunksProcessed: " + + numChunksProcessed + + ", numCellsInRow: " + + numCellsInRow + + ", numCellsInLastRow: " + + numCellsInLastRow + + ", rowKey: " + + rowKey + + ", lastCompleteRowKey: " + + lastCompleteRowKey); } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachineTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachineTest.java index 8de616431..eb510358e 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachineTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachineTest.java @@ -15,6 +15,8 @@ */ package com.google.cloud.bigtable.data.v2.stub.readrows; +import static com.google.common.truth.Truth.assertThat; + import com.google.bigtable.v2.ReadRowsResponse; import com.google.cloud.bigtable.data.v2.models.DefaultRowAdapter; import com.google.cloud.bigtable.data.v2.models.Row; @@ -23,55 +25,49 @@ import com.google.protobuf.StringValue; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; -import static com.google.common.truth.Truth.assertThat; - +@RunWith(JUnit4.class) public class StateMachineTest { - StateMachine stateMachine; + StateMachine stateMachine; - @Before - public void setUp() throws Exception { - stateMachine = new StateMachine<>(new DefaultRowAdapter().createRowBuilder()); - } + @Before + public void setUp() throws Exception { + stateMachine = new StateMachine<>(new DefaultRowAdapter().createRowBuilder()); + } - @Test - public void testErrorHandlingStats() { - StateMachine.InvalidInputException actualError = null; - - - ReadRowsResponse.CellChunk chunk = ReadRowsResponse.CellChunk.newBuilder() - .setRowKey(ByteString.copyFromUtf8("my-key1")) - .setFamilyName(StringValue.newBuilder().setValue("my-family")) - .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) - .setTimestampMicros(1_000) - .setValue(ByteString.copyFromUtf8("my-value")) - .setCommitRow(true) - .build(); - try { - stateMachine.handleChunk(chunk); - stateMachine.consumeRow(); - stateMachine.handleChunk( - chunk.toBuilder() - .setRowKey(ByteString.copyFromUtf8("my-key2")) - .setValueSize(123) // invalid value size - .build() - ); - } catch (StateMachine.InvalidInputException e) { - actualError = e; - } - - assertThat(actualError).hasMessageThat() - .contains("my-key1"); - assertThat(actualError).hasMessageThat() - .contains("my-key2"); - assertThat(actualError).hasMessageThat() - .contains("numScannedNotifications: 0"); - assertThat(actualError).hasMessageThat() - .contains("numChunksProcessed: 2"); - assertThat(actualError).hasMessageThat() - .contains("numCellsInRow: 0"); - assertThat(actualError).hasMessageThat() - .contains("numCellsInLastRow: 1"); + @Test + public void testErrorHandlingStats() { + StateMachine.InvalidInputException actualError = null; + ReadRowsResponse.CellChunk chunk = + ReadRowsResponse.CellChunk.newBuilder() + .setRowKey(ByteString.copyFromUtf8("my-key1")) + .setFamilyName(StringValue.newBuilder().setValue("my-family")) + .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) + .setTimestampMicros(1_000) + .setValue(ByteString.copyFromUtf8("my-value")) + .setCommitRow(true) + .build(); + try { + stateMachine.handleChunk(chunk); + stateMachine.consumeRow(); + stateMachine.handleChunk( + chunk + .toBuilder() + .setRowKey(ByteString.copyFromUtf8("my-key2")) + .setValueSize(123) // invalid value size + .build()); + } catch (StateMachine.InvalidInputException e) { + actualError = e; } -} \ No newline at end of file + + assertThat(actualError).hasMessageThat().contains("my-key1"); + assertThat(actualError).hasMessageThat().contains("my-key2"); + assertThat(actualError).hasMessageThat().contains("numScannedNotifications: 0"); + assertThat(actualError).hasMessageThat().contains("numChunksProcessed: 2"); + assertThat(actualError).hasMessageThat().contains("numCellsInRow: 0"); + assertThat(actualError).hasMessageThat().contains("numCellsInLastRow: 1"); + } +} From cf693a2e80e4c089e266834b5aef1305e7e4b57e Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 6 May 2020 11:04:16 -0400 Subject: [PATCH 3/3] add last 5 keys to exception --- .../bigtable/data/v2/stub/readrows/StateMachine.java | 9 +++++++-- .../data/v2/stub/readrows/StateMachineTest.java | 12 ++++++++---- 2 files changed, 15 insertions(+), 6 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 9aedb507f..b6b6db678 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 @@ -18,7 +18,9 @@ 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; +import com.google.common.base.Joiner; import com.google.common.base.Preconditions; +import com.google.common.collect.EvictingQueue; import com.google.protobuf.ByteString; import java.util.List; @@ -83,6 +85,7 @@ final class StateMachine { private int numChunksProcessed = 0; private int numCellsInRow = 0; private int numCellsInLastRow = 0; + private EvictingQueue lastSeenKeys = EvictingQueue.create(5); // Track current cell attributes: protocol omits them when they are repeated private ByteString rowKey; @@ -428,6 +431,8 @@ private State handleCommit() { validate(remainingCellBytes == 0, "Can't commit with remaining bytes"); completeRow = adapter.finishRow(); lastCompleteRowKey = rowKey; + + lastSeenKeys.add(rowKey); numRowsCommitted++; numCellsInLastRow = numCellsInRow; return AWAITING_ROW_CONSUME; @@ -449,8 +454,8 @@ private void validate(boolean condition, String message) { + numCellsInLastRow + ", rowKey: " + rowKey - + ", lastCompleteRowKey: " - + lastCompleteRowKey); + + ", last5Keys: " + + Joiner.on(",").join(lastSeenKeys)); } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachineTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachineTest.java index eb510358e..cbb5e7d80 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachineTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/StateMachineTest.java @@ -53,20 +53,24 @@ public void testErrorHandlingStats() { try { stateMachine.handleChunk(chunk); stateMachine.consumeRow(); + + stateMachine.handleChunk( + chunk.toBuilder().setRowKey(ByteString.copyFromUtf8("my-key2")).build()); + stateMachine.consumeRow(); + stateMachine.handleChunk( chunk .toBuilder() - .setRowKey(ByteString.copyFromUtf8("my-key2")) + .setRowKey(ByteString.copyFromUtf8("my-key3")) .setValueSize(123) // invalid value size .build()); } catch (StateMachine.InvalidInputException e) { actualError = e; } - assertThat(actualError).hasMessageThat().contains("my-key1"); - assertThat(actualError).hasMessageThat().contains("my-key2"); + assertThat(actualError).hasMessageThat().containsMatch("last5Keys: .*my-key1.*my-key2"); assertThat(actualError).hasMessageThat().contains("numScannedNotifications: 0"); - assertThat(actualError).hasMessageThat().contains("numChunksProcessed: 2"); + assertThat(actualError).hasMessageThat().contains("numChunksProcessed: 3"); assertThat(actualError).hasMessageThat().contains("numCellsInRow: 0"); assertThat(actualError).hasMessageThat().contains("numCellsInLastRow: 1"); }