From c56c6f9d39e6df2f0ebdd86d0f50668e77850723 Mon Sep 17 00:00:00 2001 From: Igor Berntein Date: Thu, 13 Apr 2023 10:36:23 -0400 Subject: [PATCH 1/7] feat: add experimental support for reverse scans public preview Change-Id: I092f236658a93a8ebd0bbc3bf1ef83c5c4978432 --- .../clirr-ignored-differences.xml | 11 ++ .../data/v2/internal/RowMergerUtil.java | 2 +- .../bigtable/data/v2/internal/RowSetUtil.java | 115 +++++++++--------- .../cloud/bigtable/data/v2/models/Query.java | 27 +++- .../v2/stub/EnhancedBigtableStubSettings.java | 3 +- .../readrows/ReadRowsResumptionStrategy.java | 3 +- .../data/v2/stub/readrows/RowMerger.java | 4 +- .../v2/stub/readrows/RowMergingCallable.java | 2 +- .../data/v2/stub/readrows/StateMachine.java | 18 ++- .../v2/BigtableDataClientFactoryTest.java | 38 ++++++ .../data/v2/internal/RowSetUtilTest.java | 32 ++--- .../cloud/bigtable/data/v2/it/ReadIT.java | 84 +++++++++++++ .../bigtable/data/v2/models/QueryTest.java | 7 ++ .../v2/stub/EnhancedBigtableStubTest.java | 15 +++ .../v2/stub/readrows/StateMachineTest.java | 2 +- 15 files changed, 278 insertions(+), 85 deletions(-) diff --git a/google-cloud-bigtable/clirr-ignored-differences.xml b/google-cloud-bigtable/clirr-ignored-differences.xml index da5feada67..1ca5867295 100644 --- a/google-cloud-bigtable/clirr-ignored-differences.xml +++ b/google-cloud-bigtable/clirr-ignored-differences.xml @@ -134,4 +134,15 @@ * * + + + 7002 + com/google/cloud/bigtable/data/v2/internal/RowSetUtil + * + + + 7004 + com/google/cloud/bigtable/data/v2/stub/readrows/RowMerger + * + diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RowMergerUtil.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RowMergerUtil.java index 9fbc356d53..184dfff623 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RowMergerUtil.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RowMergerUtil.java @@ -30,7 +30,7 @@ public class RowMergerUtil implements AutoCloseable { public RowMergerUtil() { RowBuilder rowBuilder = new DefaultRowAdapter().createRowBuilder(); - merger = new RowMerger<>(rowBuilder); + merger = new RowMerger<>(rowBuilder, false); } @Override diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtil.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtil.java index fbc19ad4bc..68f81cc56f 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtil.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtil.java @@ -50,80 +50,79 @@ public final class RowSetUtil { private RowSetUtil() {} /** - * Splits the provided {@link RowSet} along the provided splitPoint into 2 segments. The right - * segment will contain all keys that are strictly greater than the splitPoint and all {@link - * RowRange}s truncated to start right after the splitPoint. The primary usecase is to resume a - * broken ReadRows stream. + * Removes all the keys and range parts that fall on or before the splitPoint. + * + *

The direction of before is determined by fromStart: for forward scans fromStart is true and + * will remove all the keys and range segments that would've been read prior to the splitPoint + * (ie. all of the keys sort lexiographically at or before the split point. For reverse scans, + * fromStart is false and all segments that sort lexiographically at or after the split point are + * removed. The primary usecase is to resume a broken ReadRows stream. */ - @Nonnull - public static Split split(@Nonnull RowSet rowSet, @Nonnull ByteString splitPoint) { - // Edgecase: splitPoint is the leftmost key ("") - if (splitPoint.isEmpty()) { - return Split.of(null, rowSet); - } + public static RowSet erase(RowSet rowSet, ByteString splitPoint, boolean fromStart) { + RowSet.Builder newRowSet = RowSet.newBuilder(); - // An empty RowSet represents a full table scan. Make that explicit so that there is RowRange to - // split. if (rowSet.getRowKeysList().isEmpty() && rowSet.getRowRangesList().isEmpty()) { rowSet = RowSet.newBuilder().addRowRanges(RowRange.getDefaultInstance()).build(); } - RowSet.Builder leftBuilder = RowSet.newBuilder(); - boolean leftIsEmpty = true; - RowSet.Builder rightBuilder = RowSet.newBuilder(); - boolean rightIsEmpty = true; - + // Handle point lookups for (ByteString key : rowSet.getRowKeysList()) { - if (ByteStringComparator.INSTANCE.compare(key, splitPoint) <= 0) { - leftBuilder.addRowKeys(key); - leftIsEmpty = false; + if (fromStart) { + // key is right of the split + if (ByteStringComparator.INSTANCE.compare(key, splitPoint) > 0) { + newRowSet.addRowKeys(key); + } } else { - rightBuilder.addRowKeys(key); - rightIsEmpty = false; + // key is left of the split + if (ByteStringComparator.INSTANCE.compare(key, splitPoint) < 0) { + newRowSet.addRowKeys(key); + } } } - for (RowRange range : rowSet.getRowRangesList()) { - StartPoint startPoint = StartPoint.extract(range); - int startCmp = - ComparisonChain.start() - .compare(startPoint.value, splitPoint, ByteStringComparator.INSTANCE) - // when value lies on the split point, only closed start points are on the left - .compareTrueFirst(startPoint.isClosed, true) - .result(); - - // Range is fully on the right side - if (startCmp > 0) { - rightBuilder.addRowRanges(range); - rightIsEmpty = false; - continue; + // Handle ranges + for (RowRange rowRange : rowSet.getRowRangesList()) { + RowRange newRange = truncateRange(rowRange, splitPoint, fromStart); + if (newRange != null) { + newRowSet.addRowRanges(newRange); } + } - EndPoint endPoint = EndPoint.extract(range); - int endCmp = - ComparisonChain.start() - // empty (true) end key means rightmost regardless of the split point - .compareFalseFirst(endPoint.value.isEmpty(), false) - .compare(endPoint.value, splitPoint, ByteStringComparator.INSTANCE) - // don't care if the endpoint is open/closed: both will be on the left if the value is - // <= - .result(); - - if (endCmp <= 0) { - // Range is fully on the left - leftBuilder.addRowRanges(range); - leftIsEmpty = false; - } else { - // Range is split - leftBuilder.addRowRanges(range.toBuilder().setEndKeyClosed(splitPoint)); - leftIsEmpty = false; - rightBuilder.addRowRanges(range.toBuilder().setStartKeyOpen(splitPoint)); - rightIsEmpty = false; + // Return the new rowset if there is anything left to read + RowSet result = newRowSet.build(); + if (result.getRowKeysList().isEmpty() && result.getRowRangesList().isEmpty()) { + return null; + } + return result; + } + + private static RowRange truncateRange(RowRange range, ByteString split, boolean fromStart) { + if (fromStart) { + // range end is on or left of the split: skip + if (EndPoint.extract(range).compareTo(new EndPoint(split, true)) <= 0) { + return null; + } + } else { + // range is on or right of the split + if (StartPoint.extract(range).compareTo(new StartPoint(split, true)) >= 0) { + return null; + } + } + RowRange.Builder newRange = range.toBuilder(); + + if (fromStart) { + // range start is on or left of the split + if (StartPoint.extract(range).compareTo(new StartPoint(split, true)) <= 0) { + newRange.setStartKeyOpen(split); + } + } else { + // range end is on or right of the split + if (EndPoint.extract(range).compareTo(new EndPoint(split, true)) >= 0) { + newRange.setEndKeyOpen(split); } } - return Split.of( - leftIsEmpty ? null : leftBuilder.build(), rightIsEmpty ? null : rightBuilder.build()); + return newRange.build(); } /** diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java index 271ffe3adf..7de167dd52 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java @@ -184,6 +184,26 @@ public Query limit(long limit) { return this; } + /** + * Return rows in reverse order. + * + *

The row will be streamed in reverse lexiographic order of the keys. The row key ranges are + * still expected to be oriented the same way as forwards. ie [a,c] where a <= c. The row content + * will remain unchanged from the ordering forward scans. This is particularly useful to get the + * last N records before a key: + * + * 

{@code
+   * query
+   *   .range(ByteStringRange.unbounded().endOpen("key"))
+   *   .limit(10)
+   *   .reversed(true)
+   * }
+ */ + public Query reversed(boolean enable) { + builder.setReversed(enable); + return this; + } + /** * Split this query into multiple queries that can be evenly distributed across Bigtable nodes and * be run in parallel. This method takes the results from {@link @@ -379,11 +399,12 @@ public boolean advance(@Nonnull ByteString lastSeenRowKey) { // Split the row ranges / row keys. Return false if there's nothing // left on the right of the split point. - RowSetUtil.Split split = RowSetUtil.split(query.builder.getRows(), lastSeenRowKey); - if (split.getRight() == null) { + RowSet remaining = + RowSetUtil.erase(query.builder.getRows(), lastSeenRowKey, !query.builder.getReversed()); + if (remaining == null) { return false; } - query.builder.setRows(split.getRight()); + query.builder.setRows(remaining); return true; } } 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 9e1ba64222..b6a07fe345 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 @@ -732,7 +732,8 @@ private Builder() { .setTotalTimeout(PRIME_REQUEST_TIMEOUT) .build()); - featureFlags = FeatureFlags.newBuilder(); + featureFlags = FeatureFlags.newBuilder() + .setReverseScans(true); } private Builder(EnhancedBigtableStubSettings settings) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsResumptionStrategy.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsResumptionStrategy.java index ab312ec41c..2db46c0c29 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsResumptionStrategy.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsResumptionStrategy.java @@ -85,7 +85,8 @@ public ReadRowsRequest getResumeRequest(ReadRowsRequest originalRequest) { return originalRequest; } - RowSet remaining = RowSetUtil.split(originalRequest.getRows(), lastKey).getRight(); + RowSet remaining = + RowSetUtil.erase(originalRequest.getRows(), lastKey, !originalRequest.getReversed()); // Edge case: retrying a fulfilled request. // A fulfilled request is one that has had all of its row keys and ranges fulfilled, or if it diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/RowMerger.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/RowMerger.java index 0b8ebfd90d..54edf57a31 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/RowMerger.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/RowMerger.java @@ -61,8 +61,8 @@ public class RowMerger implements Reframer { private final StateMachine stateMachine; private Queue mergedRows; - public RowMerger(RowBuilder rowBuilder) { - stateMachine = new StateMachine<>(rowBuilder); + public RowMerger(RowBuilder rowBuilder, boolean reversed) { + stateMachine = new StateMachine<>(rowBuilder, reversed); mergedRows = new ArrayDeque<>(); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/RowMergingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/RowMergingCallable.java index 04814dd781..6f48166200 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/RowMergingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/RowMergingCallable.java @@ -49,7 +49,7 @@ public RowMergingCallable( public void call( ReadRowsRequest request, ResponseObserver responseObserver, ApiCallContext context) { RowBuilder rowBuilder = rowAdapter.createRowBuilder(); - RowMerger merger = new RowMerger<>(rowBuilder); + RowMerger merger = new RowMerger<>(rowBuilder, request.getReversed()); ReframingResponseObserver innerObserver = new ReframingResponseObserver<>(responseObserver, merger); inner.call(request, innerObserver, context); 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 b6b6db678f..1d188fcff6 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 @@ -76,6 +76,7 @@ */ final class StateMachine { private final RowBuilder adapter; + private boolean reversed; private State currentState; private ByteString lastCompleteRowKey; @@ -102,9 +103,11 @@ final class StateMachine { * Initialize a new state machine that's ready for a new row. * * @param adapter The adapter that will build the final row. + * @param reversed */ - StateMachine(RowBuilder adapter) { + StateMachine(RowBuilder adapter, boolean reversed) { this.adapter = adapter; + this.reversed = reversed; reset(); } @@ -261,9 +264,18 @@ State handleChunk(CellChunk chunk) { validate(chunk.hasFamilyName(), "AWAITING_NEW_ROW: family missing"); validate(chunk.hasQualifier(), "AWAITING_NEW_ROW: qualifier missing"); if (lastCompleteRowKey != null) { + + int cmp = ByteStringComparator.INSTANCE.compare(lastCompleteRowKey, + chunk.getRowKey()); + String direction = "increasing"; + if (reversed) { + cmp *= -1; + direction = "decreasing"; + } + validate( - ByteStringComparator.INSTANCE.compare(lastCompleteRowKey, chunk.getRowKey()) < 0, - "AWAITING_NEW_ROW: key must be strictly increasing"); + cmp < 0, + "AWAITING_NEW_ROW: key must be strictly " + direction); } rowKey = chunk.getRowKey(); diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java index ebda860851..edcda45938 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java @@ -26,6 +26,7 @@ import com.google.api.gax.rpc.TransportChannelProvider; import com.google.api.gax.rpc.WatchdogProvider; import com.google.bigtable.v2.BigtableGrpc; +import com.google.bigtable.v2.FeatureFlags; import com.google.bigtable.v2.InstanceName; import com.google.bigtable.v2.MutateRowRequest; import com.google.bigtable.v2.MutateRowResponse; @@ -36,8 +37,14 @@ import com.google.cloud.bigtable.data.v2.internal.NameUtil; import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.common.base.Preconditions; +import com.google.common.io.BaseEncoding; import io.grpc.Attributes; +import io.grpc.Metadata; import io.grpc.Server; +import io.grpc.ServerCall; +import io.grpc.ServerCall.Listener; +import io.grpc.ServerCallHandler; +import io.grpc.ServerInterceptor; import io.grpc.ServerTransportFilter; import io.grpc.stub.StreamObserver; import java.io.IOException; @@ -78,12 +85,24 @@ public class BigtableDataClientFactoryTest { private final BlockingQueue setUpAttributes = new LinkedBlockingDeque<>(); private final BlockingQueue terminateAttributes = new LinkedBlockingDeque<>(); + private final BlockingQueue requestMetadata = new LinkedBlockingDeque<>(); @Before public void setUp() throws IOException { service = new FakeBigtableService(); server = FakeServiceBuilder.create(service) + .intercept( + new ServerInterceptor() { + @Override + public Listener interceptCall( + ServerCall call, + Metadata headers, + ServerCallHandler next) { + requestMetadata.add(headers); + return next.startCall(call, headers); + } + }) .addTransportFilter( new ServerTransportFilter() { @Override @@ -276,6 +295,24 @@ public void testCreateWithRefreshingChannel() throws Exception { assertThat(terminateAttributes).hasSize(poolSize); } + @Test + public void testFeatureFlags() throws Exception { + try (BigtableDataClientFactory factory = BigtableDataClientFactory.create(defaultSettings); + BigtableDataClient client = factory.createDefault()) { + + requestMetadata.clear(); + client.mutateRow(RowMutation.create("some-table", "some-key").deleteRow()); + } + + Metadata metadata = requestMetadata.take(); + String encodedValue = + metadata.get(Metadata.Key.of("bigtable-features", Metadata.ASCII_STRING_MARSHALLER)); + FeatureFlags featureFlags = + FeatureFlags.parseFrom(BaseEncoding.base64Url().decode(encodedValue)); + + assertThat(featureFlags.getReverseScans()).isTrue(); + } + @Test public void testBulkMutationFlowControllerConfigured() throws Exception { BigtableDataSettings settings = @@ -306,6 +343,7 @@ private static class FakeBigtableService extends BigtableGrpc.BigtableImplBase { volatile MutateRowRequest lastRequest; BlockingQueue readRowsRequests = new LinkedBlockingDeque<>(); BlockingQueue pingAndWarmRequests = new LinkedBlockingDeque<>(); + private ApiFunction readRowsCallback = new ApiFunction() { @Override diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtilTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtilTest.java index 37ec606103..39d3c62c22 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtilTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtilTest.java @@ -36,37 +36,41 @@ public class RowSetUtilTest { @Test public void testSplitFullScan() { RowSet input = RowSet.getDefaultInstance(); - RowSetUtil.Split split = RowSetUtil.split(input, ByteString.copyFromUtf8("g")); - assertThat(split.getLeft()).isEqualTo(parse("-g]")); - assertThat(split.getRight()).isEqualTo(parse("(g-")); + RowSet right = RowSetUtil.erase(input, ByteString.copyFromUtf8("g"), true); + assertThat(right).isEqualTo(parse("(g-")); + + RowSet left = RowSetUtil.erase(input, ByteString.copyFromUtf8("g"), false); + assertThat(left).isEqualTo(parse("-g)")); } @Test public void testSplitAllLeft() { - RowSet input = parse("a,c,(a1-c],[a2-c],(a3-c),[a4-c)"); - RowSetUtil.Split split = RowSetUtil.split(input, ByteString.copyFromUtf8("c")); + RowSet input = parse("a,(a1-c),[a2-c),(a3-c),[a4-c)"); + RowSet left = RowSetUtil.erase(input, ByteString.copyFromUtf8("c"), false); + RowSet right = RowSetUtil.erase(input, ByteString.copyFromUtf8("c"), true); - assertThat(split.getLeft()).isEqualTo(input); - assertThat(split.getRight()).isNull(); + assertThat(left).isEqualTo(input); + assertThat(right).isNull(); } @Test public void testSplitAllRight() { RowSet input = parse("a1,c,(a-c],[a2-c],(a3-c),[a4-c)"); - RowSetUtil.Split split = RowSetUtil.split(input, ByteString.copyFromUtf8("a")); - assertThat(split.getLeft()).isNull(); - assertThat(split.getRight()).isEqualTo(input); + assertThat(RowSetUtil.erase(input, ByteString.copyFromUtf8("a"), true)).isEqualTo(input); + assertThat(RowSetUtil.erase(input, ByteString.copyFromUtf8("a"), false)).isNull(); } @Test public void testSplit() { - RowSet input = parse("a1,c,(a1-c],[a2-c],(a3-c),[a4-c)"); - RowSetUtil.Split split = RowSetUtil.split(input, ByteString.copyFromUtf8("b")); + RowSet input = parse("a1,c,(a1-c],[a2-c],(a3-c),[a4-c),[b-z],(b-y]"); + + RowSet before = RowSetUtil.erase(input, ByteString.copyFromUtf8("b"), false); + RowSet after = RowSetUtil.erase(input, ByteString.copyFromUtf8("b"), true); - assertThat(split.getLeft()).isEqualTo(parse("a1,(a1-b],[a2-b],(a3-b],[a4-b]")); - assertThat(split.getRight()).isEqualTo(parse("c,(b-c],(b-c],(b-c),(b-c)")); + assertThat(before).isEqualTo(parse("a1,(a1-b),[a2-b),(a3-b),[a4-b)")); + assertThat(after).isEqualTo(parse("c,(b-c],(b-c],(b-c),(b-c),(b-z],(b-y]")); } @Test diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/ReadIT.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/ReadIT.java index d8626059fa..533ee3c581 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/ReadIT.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/ReadIT.java @@ -16,6 +16,7 @@ package com.google.cloud.bigtable.data.v2.it; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.TruthJUnit.assume; import com.google.api.core.ApiFuture; import com.google.api.core.ApiFutureCallback; @@ -31,9 +32,11 @@ import com.google.cloud.bigtable.data.v2.models.RowCell; import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; +import com.google.cloud.bigtable.test_helpers.env.EmulatorEnv; import com.google.cloud.bigtable.test_helpers.env.TestEnvRule; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; +import com.google.common.truth.TruthJUnit; import com.google.common.util.concurrent.MoreExecutors; import com.google.protobuf.ByteString; import java.util.ArrayList; @@ -224,6 +227,87 @@ public void rangeQueries() { .isEmpty(); } + @Test + public void reversed() { + assume() + .withMessage("reverse scans are not supported in the emulator") + .that(testEnvRule.env()) + .isNotInstanceOf(EmulatorEnv.class); + BigtableDataClient client = testEnvRule.env().getDataClient(); + String tableId = testEnvRule.env().getTableId(); + String familyId = testEnvRule.env().getFamilyId(); + String uniqueKey = prefix + "-rev-queries"; + String keyA = uniqueKey + "-" + "a"; + String keyB = uniqueKey + "-" + "b"; + String keyC = uniqueKey + "-" + "c"; + + long timestampMicros = System.currentTimeMillis() * 1_000; + + client.bulkMutateRows( + BulkMutation.create(tableId) + .add(RowMutationEntry.create(keyA).setCell(familyId, "", timestampMicros, "A")) + .add(RowMutationEntry.create(keyB).setCell(familyId, "", timestampMicros, "A")) + .add(RowMutationEntry.create(keyC).setCell(familyId, "", timestampMicros, "Z"))); + + Row expectedRowA = + Row.create( + ByteString.copyFromUtf8(keyA), + ImmutableList.of( + RowCell.create( + testEnvRule.env().getFamilyId(), + ByteString.copyFromUtf8(""), + timestampMicros, + ImmutableList.of(), + ByteString.copyFromUtf8("A")))); + + Row expectedRowB = + Row.create( + ByteString.copyFromUtf8(keyB), + ImmutableList.of( + RowCell.create( + testEnvRule.env().getFamilyId(), + ByteString.copyFromUtf8(""), + timestampMicros, + ImmutableList.of(), + ByteString.copyFromUtf8("B")))); + Row expectedRowC = + Row.create( + ByteString.copyFromUtf8(keyB), + ImmutableList.of( + RowCell.create( + testEnvRule.env().getFamilyId(), + ByteString.copyFromUtf8(""), + timestampMicros, + ImmutableList.of(), + ByteString.copyFromUtf8("C")))); + + assertThat( + ImmutableList.copyOf( + client.readRows( + Query.create(tableId) + .reversed(true) + .range(ByteStringRange.prefix(uniqueKey)))) + ).containsExactly(expectedRowC, expectedRowB, expectedRowA).inOrder(); + + assertThat( + ImmutableList.copyOf( + client.readRows( + Query.create(tableId) + .reversed(true) + .range(ByteStringRange.prefix(uniqueKey)) + .limit(2))) + ).containsExactly(expectedRowC, expectedRowB).inOrder(); + + assertThat( + ImmutableList.copyOf( + client.readRows( + Query.create(tableId) + .reversed(true) + .range(ByteStringRange.unbounded().endClosed(keyC)) + .limit(2))) + ).containsExactly(expectedRowC, expectedRowB).inOrder(); + } + @Test public void readSingleNonexistentAsyncCallback() throws Exception { ApiFuture future = diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java index 655aeda688..93e5b1c92f 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java @@ -505,4 +505,11 @@ public void testQueryPaginatorEmptyTable() { assertThat(queryPaginator.advance(ByteString.EMPTY)).isFalse(); } + + @Test + public void testQueryReversed() { + Query query = Query.create(TABLE_ID).reversed(true); + assertThat(query.toProto(requestContext)) + .isEqualTo(expectedProtoBuilder().setReversed(true).build()); + } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java index c147c112e5..5e6e6fbe5d 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java @@ -53,6 +53,7 @@ import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Queues; +import com.google.common.io.BaseEncoding; import com.google.protobuf.ByteString; import com.google.protobuf.BytesValue; import com.google.protobuf.StringValue; @@ -230,6 +231,20 @@ public void testBatchJwtAudience() assertThat(parsed.getPayload().getAudience()).isEqualTo("https://bigtable.googleapis.com/"); } + @Test + public void testFeatureFlags() throws InterruptedException, IOException, ExecutionException { + + enhancedBigtableStub.readRowCallable().futureCall(Query.create("fake-table")).get(); + Metadata metadata = metadataInterceptor.headers.take(); + + String encodedFeatureFlags = + metadata.get(Key.of("bigtable-features", Metadata.ASCII_STRING_MARSHALLER)); + FeatureFlags featureFlags = + FeatureFlags.parseFrom(BaseEncoding.base64Url().decode(encodedFeatureFlags)); + + assertThat(featureFlags.getReverseScans()).isTrue(); + } + @Test public void testCreateReadRowsCallable() throws InterruptedException { ServerStreamingCallable streamingCallable = 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 cbb5e7d80f..c98506eb41 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 @@ -34,7 +34,7 @@ public class StateMachineTest { @Before public void setUp() throws Exception { - stateMachine = new StateMachine<>(new DefaultRowAdapter().createRowBuilder()); + stateMachine = new StateMachine<>(new DefaultRowAdapter().createRowBuilder(), false); } @Test From e6179fc4ef3800fb8b509902e7f13dcaa285e711 Mon Sep 17 00:00:00 2001 From: Igor Berntein Date: Thu, 13 Apr 2023 11:22:32 -0400 Subject: [PATCH 2/7] Add sample Change-Id: I1f965e0f43fd3713a7abfff68466be7c861d08a7 --- .../main/java/com/example/bigtable/Reads.java | 34 +++++++++++++++++++ .../java/com/example/bigtable/ReadsTest.java | 31 +++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/samples/snippets/src/main/java/com/example/bigtable/Reads.java b/samples/snippets/src/main/java/com/example/bigtable/Reads.java index d68997c649..3d8871c0b1 100644 --- a/samples/snippets/src/main/java/com/example/bigtable/Reads.java +++ b/samples/snippets/src/main/java/com/example/bigtable/Reads.java @@ -24,6 +24,7 @@ import com.google.cloud.bigtable.data.v2.BigtableDataClient; import com.google.cloud.bigtable.data.v2.models.Filters; import com.google.cloud.bigtable.data.v2.models.Query; +import com.google.cloud.bigtable.data.v2.models.Range.ByteStringRange; import com.google.cloud.bigtable.data.v2.models.Row; import com.google.cloud.bigtable.data.v2.models.RowCell; import java.io.IOException; @@ -200,6 +201,39 @@ public static void readPrefix(String projectId, String instanceId, String tableI } // [END bigtable_reads_prefix] + // [START bigtable_reverse_scan] + public static void readRowsReversed() { + // TODO(developer): Replace these variables before running the sample. + String projectId = "my-project-id"; + String instanceId = "my-instance-id"; + String tableId = "mobile-time-series"; + readPrefix(projectId, instanceId, tableId); + } + + public static void readRowsReversed(String projectId, String instanceId, String tableId) { + // Initialize client that will be used to send requests. This client only needs to be created + // once, and can be reused for multiple requests. After completing all of your requests, call + // the "close" method on the client to safely clean up any remaining background resources. + try (BigtableDataClient dataClient = BigtableDataClient.create(projectId, instanceId)) { + Query query = Query.create(tableId) + .reversed(true) + .limit(2) + .prefix("phone#4c410523") + .range(ByteStringRange.unbounded() + .startClosed("phone#5c10102") + .endClosed("phone#5c10102") + ); + ServerStream rows = dataClient.readRows(query); + for (Row row : rows) { + printRow(row); + } + } catch (IOException e) { + System.out.println( + "Unable to initialize service client, as a network error occurred: \n" + e.toString()); + } + } + // [END bigtable_reverse_scan] + // [START bigtable_reads_filter] public static void readFilter() { // TODO(developer): Replace these variables before running the sample. diff --git a/samples/snippets/src/test/java/com/example/bigtable/ReadsTest.java b/samples/snippets/src/test/java/com/example/bigtable/ReadsTest.java index dc3d56eed6..1af117d638 100644 --- a/samples/snippets/src/test/java/com/example/bigtable/ReadsTest.java +++ b/samples/snippets/src/test/java/com/example/bigtable/ReadsTest.java @@ -186,6 +186,37 @@ public void testReadPrefix() { TIMESTAMP)); } + @Test + public void testReadRowsReversed() { + Reads.readRowsReversed(projectId, instanceId, TABLE_ID); + String output = bout.toString(); + + assertThat(output) + .contains( + String.format( + "Reading data for phone#5c10102#20190502\n" + + "Column Family stats_summary\n" + + "\tconnected_cell: \u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0001 @%1$s\n" + + "\tconnected_wifi: \u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000 @%1$s\n" + + "\tos_build: PQ2A.190406.000 @%1$s" + + "Reading data for phone#5c10102#20190501\n" + + "Column Family stats_summary\n" + + "\tconnected_cell: \u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0001 @%1$s\n" + + "\tconnected_wifi: \u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0001 @%1$s\n" + + "\tos_build: PQ2A.190401.002 @%1$s\n\n" + + "Reading data for phone#4c410523#20190505\n" + + "Column Family stats_summary\n" + + "\tconnected_cell: \u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000 @%1$s\n" + + "\tconnected_wifi: \u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0001 @%1$s\n" + + "\tos_build: PQ2A.190406.000 @%1$s\n\n" + + "Reading data for phone#4c410523#20190502\n" + + "Column Family stats_summary\n" + + "\tconnected_cell: \u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0001 @%1$s\n" + + "\tconnected_wifi: \u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0001 @%1$s\n" + + "\tos_build: PQ2A.190405.004 @%1$s\n\n", + TIMESTAMP)); + } + @Test public void testReadFilter() { Reads.readFilter(projectId, instanceId, TABLE_ID); From 6eae36ed622a95f233cae9522c95c8255c0e0465 Mon Sep 17 00:00:00 2001 From: Igor Berntein Date: Mon, 17 Apr 2023 09:53:52 +0200 Subject: [PATCH 3/7] cleanup sample Change-Id: I1b24aed62198cf28c395f4152a548a555146ffb9 --- .../snippets/src/main/java/com/example/bigtable/Reads.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/samples/snippets/src/main/java/com/example/bigtable/Reads.java b/samples/snippets/src/main/java/com/example/bigtable/Reads.java index 3d8871c0b1..14000084e4 100644 --- a/samples/snippets/src/main/java/com/example/bigtable/Reads.java +++ b/samples/snippets/src/main/java/com/example/bigtable/Reads.java @@ -219,10 +219,7 @@ public static void readRowsReversed(String projectId, String instanceId, String .reversed(true) .limit(2) .prefix("phone#4c410523") - .range(ByteStringRange.unbounded() - .startClosed("phone#5c10102") - .endClosed("phone#5c10102") - ); + .range("phone#5c10102", "phone#5c10103"); ServerStream rows = dataClient.readRows(query); for (Row row : rows) { printRow(row); From 50ebc01616656dae2d85c6048ccf7cc8e8ae837c Mon Sep 17 00:00:00 2001 From: Igor Berntein Date: Tue, 30 May 2023 14:30:32 -0400 Subject: [PATCH 4/7] typo in sample Change-Id: Ib0ec1c4d01f9cb6153c07dfb3637dc8092050f3f --- samples/snippets/src/main/java/com/example/bigtable/Reads.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/snippets/src/main/java/com/example/bigtable/Reads.java b/samples/snippets/src/main/java/com/example/bigtable/Reads.java index 14000084e4..e9bc3439ba 100644 --- a/samples/snippets/src/main/java/com/example/bigtable/Reads.java +++ b/samples/snippets/src/main/java/com/example/bigtable/Reads.java @@ -207,7 +207,7 @@ public static void readRowsReversed() { String projectId = "my-project-id"; String instanceId = "my-instance-id"; String tableId = "mobile-time-series"; - readPrefix(projectId, instanceId, tableId); + readRowsReversed(projectId, instanceId, tableId); } public static void readRowsReversed(String projectId, String instanceId, String tableId) { From 659f7c6196fa124b15ce886c840bad68353f0e8e Mon Sep 17 00:00:00 2001 From: Igor Berntein Date: Tue, 27 Jun 2023 10:42:41 -0400 Subject: [PATCH 5/7] format Change-Id: I96d336959ea618de13d902dd663f3555bec3a6a9 --- .../v2/stub/EnhancedBigtableStubSettings.java | 3 +- .../data/v2/stub/readrows/StateMachine.java | 9 ++-- .../cloud/bigtable/data/v2/it/ReadIT.java | 42 +++++++++---------- 3 files changed, 25 insertions(+), 29 deletions(-) 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 b6a07fe345..eba09a7464 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 @@ -732,8 +732,7 @@ private Builder() { .setTotalTimeout(PRIME_REQUEST_TIMEOUT) .build()); - featureFlags = FeatureFlags.newBuilder() - .setReverseScans(true); + featureFlags = FeatureFlags.newBuilder().setReverseScans(true); } private Builder(EnhancedBigtableStubSettings settings) { 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 1d188fcff6..6791679829 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 @@ -265,17 +265,14 @@ State handleChunk(CellChunk chunk) { validate(chunk.hasQualifier(), "AWAITING_NEW_ROW: qualifier missing"); if (lastCompleteRowKey != null) { - int cmp = ByteStringComparator.INSTANCE.compare(lastCompleteRowKey, - chunk.getRowKey()); - String direction = "increasing"; + int cmp = ByteStringComparator.INSTANCE.compare(lastCompleteRowKey, chunk.getRowKey()); + String direction = "increasing"; if (reversed) { cmp *= -1; direction = "decreasing"; } - validate( - cmp < 0, - "AWAITING_NEW_ROW: key must be strictly " + direction); + validate(cmp < 0, "AWAITING_NEW_ROW: key must be strictly " + direction); } rowKey = chunk.getRowKey(); diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/ReadIT.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/ReadIT.java index 533ee3c581..1dbd33eed7 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/ReadIT.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/ReadIT.java @@ -36,7 +36,6 @@ import com.google.cloud.bigtable.test_helpers.env.TestEnvRule; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; -import com.google.common.truth.TruthJUnit; import com.google.common.util.concurrent.MoreExecutors; import com.google.protobuf.ByteString; import java.util.ArrayList; @@ -282,30 +281,31 @@ public void reversed() { ByteString.copyFromUtf8("C")))); assertThat( - ImmutableList.copyOf( - client.readRows( - Query.create(tableId) - .reversed(true) - .range(ByteStringRange.prefix(uniqueKey)))) - ).containsExactly(expectedRowC, expectedRowB, expectedRowA).inOrder(); + ImmutableList.copyOf( + client.readRows( + Query.create(tableId).reversed(true).range(ByteStringRange.prefix(uniqueKey))))) + .containsExactly(expectedRowC, expectedRowB, expectedRowA) + .inOrder(); assertThat( - ImmutableList.copyOf( - client.readRows( - Query.create(tableId) - .reversed(true) - .range(ByteStringRange.prefix(uniqueKey)) - .limit(2))) - ).containsExactly(expectedRowC, expectedRowB).inOrder(); + ImmutableList.copyOf( + client.readRows( + Query.create(tableId) + .reversed(true) + .range(ByteStringRange.prefix(uniqueKey)) + .limit(2)))) + .containsExactly(expectedRowC, expectedRowB) + .inOrder(); assertThat( - ImmutableList.copyOf( - client.readRows( - Query.create(tableId) - .reversed(true) - .range(ByteStringRange.unbounded().endClosed(keyC)) - .limit(2))) - ).containsExactly(expectedRowC, expectedRowB).inOrder(); + ImmutableList.copyOf( + client.readRows( + Query.create(tableId) + .reversed(true) + .range(ByteStringRange.unbounded().endClosed(keyC)) + .limit(2)))) + .containsExactly(expectedRowC, expectedRowB) + .inOrder(); } @Test From 95be137c392dcd3b380b27f0a9b74c8e3a51c0d1 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Tue, 27 Jun 2023 14:45:35 +0000 Subject: [PATCH 6/7] =?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 --- .../src/main/java/com/example/bigtable/Reads.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/samples/snippets/src/main/java/com/example/bigtable/Reads.java b/samples/snippets/src/main/java/com/example/bigtable/Reads.java index e9bc3439ba..1bd5609f96 100644 --- a/samples/snippets/src/main/java/com/example/bigtable/Reads.java +++ b/samples/snippets/src/main/java/com/example/bigtable/Reads.java @@ -24,7 +24,6 @@ import com.google.cloud.bigtable.data.v2.BigtableDataClient; import com.google.cloud.bigtable.data.v2.models.Filters; import com.google.cloud.bigtable.data.v2.models.Query; -import com.google.cloud.bigtable.data.v2.models.Range.ByteStringRange; import com.google.cloud.bigtable.data.v2.models.Row; import com.google.cloud.bigtable.data.v2.models.RowCell; import java.io.IOException; @@ -215,11 +214,12 @@ public static void readRowsReversed(String projectId, String instanceId, String // once, and can be reused for multiple requests. After completing all of your requests, call // the "close" method on the client to safely clean up any remaining background resources. try (BigtableDataClient dataClient = BigtableDataClient.create(projectId, instanceId)) { - Query query = Query.create(tableId) - .reversed(true) - .limit(2) - .prefix("phone#4c410523") - .range("phone#5c10102", "phone#5c10103"); + Query query = + Query.create(tableId) + .reversed(true) + .limit(2) + .prefix("phone#4c410523") + .range("phone#5c10102", "phone#5c10103"); ServerStream rows = dataClient.readRows(query); for (Row row : rows) { printRow(row); From 6208dc2887e3f4190753a82733f033143df66fcc Mon Sep 17 00:00:00 2001 From: Igor Berntein Date: Tue, 27 Jun 2023 11:19:19 -0400 Subject: [PATCH 7/7] fix integration test Change-Id: I12faf9498706c87eab2cab43ad20d2b056935270 --- .../com/google/cloud/bigtable/data/v2/it/ReadIT.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/ReadIT.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/ReadIT.java index 1dbd33eed7..7b58e14f7c 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/ReadIT.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/ReadIT.java @@ -245,8 +245,8 @@ public void reversed() { client.bulkMutateRows( BulkMutation.create(tableId) .add(RowMutationEntry.create(keyA).setCell(familyId, "", timestampMicros, "A")) - .add(RowMutationEntry.create(keyB).setCell(familyId, "", timestampMicros, "A")) - .add(RowMutationEntry.create(keyC).setCell(familyId, "", timestampMicros, "Z"))); + .add(RowMutationEntry.create(keyB).setCell(familyId, "", timestampMicros, "B")) + .add(RowMutationEntry.create(keyC).setCell(familyId, "", timestampMicros, "C"))); Row expectedRowA = Row.create( @@ -271,7 +271,7 @@ public void reversed() { ByteString.copyFromUtf8("B")))); Row expectedRowC = Row.create( - ByteString.copyFromUtf8(keyB), + ByteString.copyFromUtf8(keyC), ImmutableList.of( RowCell.create( testEnvRule.env().getFamilyId(), @@ -302,9 +302,9 @@ public void reversed() { client.readRows( Query.create(tableId) .reversed(true) - .range(ByteStringRange.unbounded().endClosed(keyC)) + .range(ByteStringRange.unbounded().endOpen(keyC)) .limit(2)))) - .containsExactly(expectedRowC, expectedRowB) + .containsExactly(expectedRowB, expectedRowA) .inOrder(); }