Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: handle retry info so client respect the delay server sets #2026

Merged
merged 14 commits into from
Dec 19, 2023
Prev Previous commit
Next Next commit
Merge branch 'main' into retry_info
  • Loading branch information
mutianf committed Dec 18, 2023
commit cb519de707e33fe55f35bb871d60833140f75ea6
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ public static EnhancedBigtableStubSettings finalizeSettings(
: null;

if (builder.getEnableRoutingCookie() && transportProvider != null) {
// TODO: this also need to be added to BigtableClientFactory
// patch cookies interceptor
transportProvider.setInterceptorProvider(() -> ImmutableList.of(new CookiesInterceptor()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ public void tearDown() throws Exception {

@Test
public void testReadRows() {
// Only server stream calls can return an initial metadata before the first response
client.readRows(Query.create("fake-table")).iterator().hasNext();

assertThat(fakeService.count.get()).isGreaterThan(1);
Expand Down Expand Up @@ -331,49 +330,6 @@ public void testGenerateInitialChangeStreamPartition() {
serverMetadata.clear();
}

@Test
public void testReadChangeStream() {
client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext();

assertThat(fakeService.count.get()).isGreaterThan(1);
assertThat(serverMetadata).hasSize(fakeService.count.get());

Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1);

// TODO: read change stream should have ROUTING_COOKIE_HEADER but the sequence is different from
// ReadRows.
// Debug the test failure.
assertThat(lastMetadata)
.containsAtLeast(
ROUTING_COOKIE_1.name(), "readChangeStream", ROUTING_COOKIE_2.name(), testCookie);
assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name());

serverMetadata.clear();
}

@Test
public void testGenerateInitialChangeStreamParition() {
client.generateInitialChangeStreamPartitions("table").iterator().hasNext();

assertThat(fakeService.count.get()).isGreaterThan(1);
assertThat(serverMetadata).hasSize(fakeService.count.get());

Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1);

// TODO: read change stream should have ROUTING_COOKIE_HEADER but the sequence is different from
// ReadRows.
// Debug the test failure.
assertThat(lastMetadata)
.containsAtLeast(
ROUTING_COOKIE_1.name(),
"generateInitialChangeStreamPartitions",
ROUTING_COOKIE_2.name(),
testCookie);
assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name());

serverMetadata.clear();
}

@Test
public void testNoCookieSucceedReadRows() {
fakeService.returnCookie = false;
Expand Down Expand Up @@ -499,48 +455,6 @@ public void testNoCookieSucceedGenerateInitialChangeStreamParition() {
serverMetadata.clear();
}

@Test
public void testNoCookieSucceedReadChangeStream() {
fakeService.returnCookie = false;

client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext();

assertThat(fakeService.count.get()).isGreaterThan(1);
assertThat(serverMetadata).hasSize(fakeService.count.get());

Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1);

assertThat(lastMetadata).doesNotContainKeys(ROUTING_COOKIE_2.name(), BAD_KEY.name());
// TODO: read change stream should have ROUTING_COOKIE_HEADER but the sequence is different from
// ReadRows.
// Debug the test failure.
// assertThat(lastMetadata).containsAtLeast(ROUTING_COOKIE_1.name(), routingCookie1Header);

serverMetadata.clear();

serverMetadata.clear();
}

@Test
public void testNoCookieSucceedGenerateInitialChangeStreamParition() {
fakeService.returnCookie = false;

client.generateInitialChangeStreamPartitions("table").iterator().hasNext();

assertThat(fakeService.count.get()).isGreaterThan(1);
assertThat(serverMetadata).hasSize(fakeService.count.get());

Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1);

assertThat(lastMetadata).doesNotContainKeys(ROUTING_COOKIE_2.name(), BAD_KEY.name());
// TODO: read change stream should have ROUTING_COOKIE_HEADER but the sequence is different from
// ReadRows.
// Debug the test failure.
// assertThat(lastMetadata).containsAtLeast(ROUTING_COOKIE_1.name(), routingCookie1Header);

serverMetadata.clear();
}

@Test
public void testCookiesInHeaders() throws Exception {
// Send 2 cookies in the headers, with routingCookieKey and ROUTING_COOKIE_2. ROUTING_COOKIE_2
Expand Down Expand Up @@ -682,7 +596,9 @@ public void testDisableRoutingCookie() throws IOException {
assertThat(fakeService.count.get()).isEqualTo(2);
fakeService.count.set(0);

client.readChangeStream(ReadChangeStreamQuery.create("fake-table")).iterator().hasNext();
for (ChangeStreamRecord record :
client.readChangeStream(ReadChangeStreamQuery.create("fake-table"))) {}

assertThat(fakeService.count.get()).isEqualTo(2);

assertThat(methods).isEmpty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,7 @@ public void routingCookieIsEnabled() throws IOException {
.setProjectId(dummyProjectId)
.setInstanceId(dummyInstanceId)
.setCredentialsProvider(credentialsProvider);

assertThat(builder.getEnableRoutingCookie()).isTrue();
assertThat(builder.build().getEnableRoutingCookie()).isTrue();
assertThat(builder.build().toBuilder().getEnableRoutingCookie()).isTrue();
Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.