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.

Sign up for GitHub

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

Bigtable: improve list tables spooler #3639

Merged
merged 4 commits into from
Sep 6, 2018
Merged

Bigtable: improve list tables spooler #3639

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
827cdfa
Bigtable: improve list tables spooler
igorbernstein2 Sep 4, 2018
c481000
update async test as well
igorbernstein2 Sep 4, 2018
3535bdc
reformat
igorbernstein2 Sep 6, 2018
96e7e6d
Merge remote-tracking branch 'origin/master' into table-admin-list
igorbernstein2 Sep 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 61 additions & 9 deletions ...able-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
*/
package com.google.cloud.bigtable.admin.v2;

import com.google.api.core.ApiAsyncFunction;
import com.google.cloud.bigtable.admin.v2.BaseBigtableTableAdminClient.ListTablesPage;
import com.google.cloud.bigtable.admin.v2.BaseBigtableTableAdminClient.ListTablesPagedResponse;
import com.google.common.collect.Lists;
import com.google.common.util.concurrent.MoreExecutors;
import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -32,7 +35,6 @@
import com.google.bigtable.admin.v2.GetTableRequest;
import com.google.bigtable.admin.v2.InstanceName;
import com.google.bigtable.admin.v2.ListTablesRequest;
import com.google.bigtable.admin.v2.ListTablesResponse;
import com.google.bigtable.admin.v2.TableName;
import com.google.cloud.bigtable.admin.v2.models.CreateTableRequest;
import com.google.cloud.bigtable.admin.v2.models.ModifyColumnFamiliesRequest;
Expand Down Expand Up @@ -327,18 +329,68 @@ public List<TableName> listTables() {
* }</pre>
*/
public ApiFuture<List<TableName>> listTablesAsync() {
ApiFuture<ListTablesPagedResponse> listResp =
this.stub.listTablesPagedCallable().futureCall(composeListTableRequest());
ListTablesRequest request = ListTablesRequest.newBuilder().setParent(instanceName.toString())
.build();

return ApiFutures.transform(
listResp,
new ApiFunction<ListTablesPagedResponse, List<TableName>>() {
// TODO(igorbernstein2): try to upstream pagination spooling or figure out a way to expose the
// paginated responses while maintaining the wrapper facade.

// Fetch the first page.
ApiFuture<ListTablesPage> firstPageFuture = ApiFutures.transform(
stub.listTablesPagedCallable().futureCall(request),
new ApiFunction<ListTablesPagedResponse, ListTablesPage>() {
@Override
public List<TableName> apply(ListTablesPagedResponse input) {
return convertToTableNames(input.iterateAll());
public ListTablesPage apply(ListTablesPagedResponse response) {
return response.getPage();
}
},
MoreExecutors.directExecutor());
MoreExecutors.directExecutor()
);

// Fetch the rest of the pages by chaining the futures.
ApiFuture<List<com.google.bigtable.admin.v2.Table>> allProtos = ApiFutures

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

.transformAsync(
firstPageFuture,
new ApiAsyncFunction<ListTablesPage, List<com.google.bigtable.admin.v2.Table>>() {
List<com.google.bigtable.admin.v2.Table> responseAccumulator = Lists

This comment was marked as spam.

Sign in to view

.newArrayList();

@Override
public ApiFuture<List<com.google.bigtable.admin.v2.Table>> apply(
ListTablesPage page) {
// Add all entries from the page
responseAccumulator.addAll(Lists.newArrayList(page.getValues()));

// If this is the last page, just return the accumulated responses.
if (!page.hasNextPage()) {
return ApiFutures.immediateFuture(responseAccumulator);
}

// Otherwise fetch the next page.
return ApiFutures.transformAsync(
page.getNextPageAsync(),
this,
MoreExecutors.directExecutor()
);
}
},
MoreExecutors.directExecutor()
);

// Wrap all of the accumulated protos.
return ApiFutures.transform(allProtos,
new ApiFunction<List<com.google.bigtable.admin.v2.Table>, List<TableName>>() {
@Override
public List<TableName> apply(List<com.google.bigtable.admin.v2.Table> protos) {
List<TableName> results = Lists.newArrayListWithCapacity(protos.size());

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

for (com.google.bigtable.admin.v2.Table proto : protos) {
results.add(TableName.parse(proto.getName()));
}
return results;
}
},
MoreExecutors.directExecutor()
);
}

/**
Expand Down
117 changes: 77 additions & 40 deletions ...-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.google.bigtable.admin.v2.ListTablesRequest;
import com.google.bigtable.admin.v2.ModifyColumnFamiliesRequest.Modification;
import com.google.bigtable.admin.v2.TableName;
import com.google.cloud.bigtable.admin.v2.BaseBigtableTableAdminClient.ListTablesPage;
import com.google.cloud.bigtable.admin.v2.BaseBigtableTableAdminClient.ListTablesPagedResponse;
import com.google.cloud.bigtable.admin.v2.models.ConsistencyToken;
import com.google.cloud.bigtable.admin.v2.models.CreateTableRequest;
Expand Down Expand Up @@ -327,67 +328,103 @@ public void testGetTableAsync() throws Exception {
@Test
public void testListTables() {
// Setup
ListTablesRequest expectedRequest = ListTablesRequest.newBuilder()
.setParent(INSTANCE_NAME.toString())
.build();

ListTablesPagedResponse expectedResponseWrapper = Mockito.mock(ListTablesPagedResponse.class);
com.google.bigtable.admin.v2.ListTablesRequest expectedRequest =

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

com.google.bigtable.admin.v2.ListTablesRequest.newBuilder()
.setParent(INSTANCE_NAME.toString())
.build();

Iterable<com.google.bigtable.admin.v2.Table> expectedResults = Lists.newArrayList(
com.google.bigtable.admin.v2.Table.newBuilder()
.setName(TABLE_NAME.toString() + "1")
.build(),
com.google.bigtable.admin.v2.Table.newBuilder()
.setName(TABLE_NAME.toString() + "2")
.build());
// 3 Tables spread across 2 pages
List<com.google.bigtable.admin.v2.Table> expectedProtos = Lists.newArrayList();
for (int i = 0; i < 3; i++) {
expectedProtos.add(
com.google.bigtable.admin.v2.Table.newBuilder()
.setName(TABLE_NAME.toString() + i)
.build());
}
// 2 on the first page
ListTablesPage page0 = Mockito.mock(ListTablesPage.class);
Mockito.when(page0.getValues()).thenReturn(expectedProtos.subList(0, 2));
Mockito.when(page0.getNextPageToken()).thenReturn("next-page");
Mockito.when(page0.hasNextPage()).thenReturn(true);

// 1 on the last page
ListTablesPage page1 = Mockito.mock(ListTablesPage.class);
Mockito.when(page1.getValues()).thenReturn(expectedProtos.subList(2, 3));

// Link page0 to page1
Mockito.when(page0.getNextPageAsync()).thenReturn(
ApiFutures.immediateFuture(page1)
);

Mockito.when(mockListTableCallable.futureCall(expectedRequest))
.thenReturn(ApiFutures.immediateFuture(expectedResponseWrapper));
// Link page to the response
ListTablesPagedResponse response0 = Mockito.mock(ListTablesPagedResponse.class);
Mockito.when(response0.getPage()).thenReturn(page0);

Mockito.when(expectedResponseWrapper.iterateAll())
.thenReturn(expectedResults);
Mockito.when(mockListTableCallable.futureCall(expectedRequest)).thenReturn(
ApiFutures.immediateFuture(response0)
);

// Execute
List<TableName> actualResults = adminClient.listTables();

// Verify
assertThat(actualResults).containsExactly(
TableName.parse(TABLE_NAME.toString() + "1"),
TableName.parse(TABLE_NAME.toString() + "2")
);
List<TableName> expectedResults = Lists.newArrayList();
for (com.google.bigtable.admin.v2.Table expectedProto : expectedProtos) {
expectedResults.add(TableName.parse(expectedProto.getName()));
}

assertThat(actualResults).containsExactlyElementsIn(expectedResults);
}

@Test
public void testListTablesAsync() throws Exception {
// Setup
ListTablesRequest expectedRequest = ListTablesRequest.newBuilder()
.setParent(INSTANCE_NAME.toString())
.build();

ListTablesPagedResponse expectedResponseWrapper = Mockito.mock(ListTablesPagedResponse.class);
com.google.bigtable.admin.v2.ListTablesRequest expectedRequest =
com.google.bigtable.admin.v2.ListTablesRequest.newBuilder()
.setParent(INSTANCE_NAME.toString())
.build();

Iterable<com.google.bigtable.admin.v2.Table> expectedResults = Lists.newArrayList(
com.google.bigtable.admin.v2.Table.newBuilder()
.setName(TABLE_NAME.toString() + "1")
.build(),
com.google.bigtable.admin.v2.Table.newBuilder()
.setName(TABLE_NAME.toString() + "2")
.build());
// 3 Tables spread across 2 pages
List<com.google.bigtable.admin.v2.Table> expectedProtos = Lists.newArrayList();
for (int i = 0; i < 3; i++) {
expectedProtos.add(
com.google.bigtable.admin.v2.Table.newBuilder()
.setName(TABLE_NAME.toString() + i)
.build());
}
// 2 on the first page
ListTablesPage page0 = Mockito.mock(ListTablesPage.class);
Mockito.when(page0.getValues()).thenReturn(expectedProtos.subList(0, 2));
Mockito.when(page0.getNextPageToken()).thenReturn("next-page");
Mockito.when(page0.hasNextPage()).thenReturn(true);

// 1 on the last page
ListTablesPage page1 = Mockito.mock(ListTablesPage.class);
Mockito.when(page1.getValues()).thenReturn(expectedProtos.subList(2, 3));

// Link page0 to page1
Mockito.when(page0.getNextPageAsync()).thenReturn(
ApiFutures.immediateFuture(page1)
);

Mockito.when(mockListTableCallable.futureCall(expectedRequest))
.thenReturn(ApiFutures.immediateFuture(expectedResponseWrapper));
// Link page to the response
ListTablesPagedResponse response0 = Mockito.mock(ListTablesPagedResponse.class);
Mockito.when(response0.getPage()).thenReturn(page0);

Mockito.when(expectedResponseWrapper.iterateAll())
.thenReturn(expectedResults);
Mockito.when(mockListTableCallable.futureCall(expectedRequest)).thenReturn(
ApiFutures.immediateFuture(response0)
);

// Execute
ApiFuture<List<TableName>> actualResults = adminClient.listTablesAsync();

// Verify
assertThat(actualResults.get()).containsExactly(
TableName.parse(TABLE_NAME.toString() + "1"),
TableName.parse(TABLE_NAME.toString() + "2")
);
List<TableName> expectedResults = Lists.newArrayList();
for (com.google.bigtable.admin.v2.Table expectedProto : expectedProtos) {
expectedResults.add(TableName.parse(expectedProto.getName()));
}

assertThat(actualResults.get()).containsExactlyElementsIn(expectedResults);
}

@Test
Expand Down