Skip to content

Commit

Permalink
Bigtable: non-functional improvements relating to error handling (#3809)
Browse files Browse the repository at this point in the history 
* Use gax's async error handling now that it has been upstreamed

* Bigtable: non-functional improvements

* Add error handling examples to the javadocs
* Use gax for error handling now that the code has been upstreamed
* Fix outdated bigtable-admin readme

* spacing

* more docs

* more docs

* address feedback
  • Loading branch information
@igorbernstein2
igorbernstein2 authored Oct 23, 2018
1 parent 64c0ee6 commit 827230e
Show file tree
Hide file tree
Showing 4 changed files with 249 additions and 110 deletions.
11 changes: 6 additions & 5 deletions google-cloud-clients/google-cloud-bigtable-admin/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,23 @@ at the top of your file:


```java
import com.google.bigtable.admin.v2.ColumnFamily;
import static com.google.cloud.bigtable.admin.v2.models.GCRules.GCRULES;
import com.google.bigtable.admin.v2.InstanceName;
import com.google.cloud.bigtable.admin.v2.BigtableTableAdminClient;
import com.google.cloud.bigtable.admin.v2.models.CreateTableRequest;
import com.google.cloud.bigtable.admin.v2.models.Table;
```

Then, to create a table, use the following code:
```java
InstanceName instanceName = InstanceName.of("my-project", "my-instance");

TableAdminClient tableAdminClient = TableAdminClient.create(instanceName);
BigtableTableAdminClient tableAdminClient = BigtableTableAdminClient.create(instanceName);

try {
CreateTable createTableReq = TableAdminRequests.createTable("tableId")
Table createdTable = tableAdminClient.createTable(
CreateTableRequest.of("my-table")
.addFamily("cf2", GCRULES.maxVersions(10));
client.createTable(createTableReq);

} finally {
tableAdminClient.close();
}
Expand Down
66 changes: 21 additions & 45 deletions ...e-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.api.core.ApiFunction;
import com.google.api.core.ApiFuture;
import com.google.api.core.ApiFutures;
import com.google.api.gax.rpc.ApiExceptions;
import com.google.api.resourcenames.ResourceName;
import com.google.bigtable.admin.v2.AppProfileName;
import com.google.bigtable.admin.v2.ClusterName;
Expand Down Expand Up @@ -46,9 +47,7 @@
import com.google.common.base.Verify;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.UncheckedExecutionException;
import com.google.iam.v1.GetIamPolicyRequest;
import com.google.iam.v1.SetIamPolicyRequest;
import com.google.iam.v1.TestIamPermissionsRequest;
Expand Down Expand Up @@ -160,7 +159,7 @@ public void close() {
*/
@SuppressWarnings("WeakerAccess")
public Instance createInstance(CreateInstanceRequest request) {
return awaitFuture(createInstanceAsync(request));
return ApiExceptions.callAndTranslateApiException(createInstanceAsync(request));
}

/**
Expand Down Expand Up @@ -208,7 +207,7 @@ public Instance apply(com.google.bigtable.admin.v2.Instance proto) {
*/
@SuppressWarnings("WeakerAccess")
public Instance updateInstance(UpdateInstanceRequest request) {
return awaitFuture(updateInstanceAsync(request));
return ApiExceptions.callAndTranslateApiException(updateInstanceAsync(request));
}

/**
Expand Down Expand Up @@ -251,7 +250,7 @@ public Instance apply(com.google.bigtable.admin.v2.Instance proto) {
*/
@SuppressWarnings("WeakerAccess")
public Instance getInstance(String id) {
return awaitFuture(getInstanceAsync(id));
return ApiExceptions.callAndTranslateApiException(getInstanceAsync(id));
}

/**
Expand Down Expand Up @@ -302,7 +301,7 @@ public Instance apply(com.google.bigtable.admin.v2.Instance proto) {
*/
@SuppressWarnings("WeakerAccess")
public List<Instance> listInstances() {
return awaitFuture(listInstancesAsync());
return ApiExceptions.callAndTranslateApiException(listInstancesAsync());
}

/**
Expand Down Expand Up @@ -385,7 +384,7 @@ public List<Instance> apply(com.google.bigtable.admin.v2.ListInstancesResponse p
*/
@SuppressWarnings("WeakerAccess")
public void deleteInstance(String instanceId) {
awaitFuture(deleteInstanceAsync(instanceId));
ApiExceptions.callAndTranslateApiException(deleteInstanceAsync(instanceId));
}

/**
Expand Down Expand Up @@ -434,7 +433,7 @@ public Void apply(Empty input) {
*/
@SuppressWarnings("WeakerAccess")
public Cluster createCluster(CreateClusterRequest request) {
return awaitFuture(createClusterAsync(request));
return ApiExceptions.callAndTranslateApiException(createClusterAsync(request));
}

/**
Expand Down Expand Up @@ -478,7 +477,7 @@ public Cluster apply(com.google.bigtable.admin.v2.Cluster proto) {
*/
@SuppressWarnings("WeakerAccess")
public Cluster getCluster(String instanceId, String clusterId) {
return awaitFuture(getClusterAsync(instanceId, clusterId));
return ApiExceptions.callAndTranslateApiException(getClusterAsync(instanceId, clusterId));
}

/**
Expand Down Expand Up @@ -531,7 +530,7 @@ public Cluster apply(com.google.bigtable.admin.v2.Cluster proto) {
*/
@SuppressWarnings("WeakerAccess")
public List<Cluster> listClusters(String instanceId) {
return awaitFuture(listClustersAsync(instanceId));
return ApiExceptions.callAndTranslateApiException(listClustersAsync(instanceId));
}

/**
Expand Down Expand Up @@ -615,7 +614,8 @@ public List<Cluster> apply(com.google.bigtable.admin.v2.ListClustersResponse pro
*/
@SuppressWarnings("WeakerAccess")
public Cluster resizeCluster(String instanceId, String clusterId, int numServeNodes) {
return awaitFuture(resizeClusterAsync(instanceId, clusterId, numServeNodes));
return ApiExceptions.callAndTranslateApiException(
resizeClusterAsync(instanceId, clusterId, numServeNodes));
}

/**
Expand Down Expand Up @@ -662,7 +662,7 @@ public Cluster apply(com.google.bigtable.admin.v2.Cluster proto) {
*/
@SuppressWarnings("WeakerAccess")
public void deleteCluster(String instanceId, String clusterId) {
awaitFuture(deleteClusterAsync(instanceId, clusterId));
ApiExceptions.callAndTranslateApiException(deleteClusterAsync(instanceId, clusterId));
}

/**
Expand Down Expand Up @@ -713,7 +713,7 @@ public Void apply(Empty input) {
*/
@SuppressWarnings("WeakerAccess")
public AppProfile createAppProfile(CreateAppProfileRequest request) {
return awaitFuture(createAppProfileAsync(request));
return ApiExceptions.callAndTranslateApiException(createAppProfileAsync(request));
}

/**
Expand Down Expand Up @@ -759,7 +759,7 @@ public AppProfile apply(com.google.bigtable.admin.v2.AppProfile proto) {
*/
@SuppressWarnings("WeakerAccess")
public AppProfile getAppProfile(String instanceId, String appProfileId) {
return awaitFuture(getAppProfileAsync(instanceId, appProfileId));
return ApiExceptions.callAndTranslateApiException(getAppProfileAsync(instanceId, appProfileId));
}

/**
Expand Down Expand Up @@ -808,7 +808,7 @@ public AppProfile apply(com.google.bigtable.admin.v2.AppProfile proto) {
*/
@SuppressWarnings("WeakerAccess")
public List<AppProfile> listAppProfiles(String instanceId) {
return awaitFuture(listAppProfilesAsync(instanceId));
return ApiExceptions.callAndTranslateApiException(listAppProfilesAsync(instanceId));
}

/**
Expand Down Expand Up @@ -911,7 +911,7 @@ public List<AppProfile> apply(List<com.google.bigtable.admin.v2.AppProfile> inpu
*/
@SuppressWarnings("WeakerAccess")
public AppProfile updateAppProfile(UpdateAppProfileRequest request) {
return awaitFuture(updateAppProfileAsync(request));
return ApiExceptions.callAndTranslateApiException(updateAppProfileAsync(request));
}

/**
Expand Down Expand Up @@ -966,7 +966,7 @@ public AppProfile apply(com.google.bigtable.admin.v2.AppProfile proto) {
*/
@SuppressWarnings("WeakerAccess")
public void deleteAppProfile(String instanceId, String appProfileId) {
awaitFuture(deleteAppProfileAsync(instanceId, appProfileId));
ApiExceptions.callAndTranslateApiException(deleteAppProfileAsync(instanceId, appProfileId));
}

/**
Expand Down Expand Up @@ -1016,7 +1016,7 @@ public Void apply(Empty input) {
*/
@SuppressWarnings("WeakerAccess")
public Policy getIamPolicy(String instanceId) {
return awaitFuture(getIamPolicyAsync(instanceId));
return ApiExceptions.callAndTranslateApiException(getIamPolicyAsync(instanceId));
}

/**
Expand Down Expand Up @@ -1083,7 +1083,7 @@ public Policy apply(com.google.iam.v1.Policy proto) {
*/
@SuppressWarnings("WeakerAccess")
public Policy setIamPolicy(String instanceId, Policy policy) {
return awaitFuture(setIamPolicyAsync(instanceId, policy));
return ApiExceptions.callAndTranslateApiException(setIamPolicyAsync(instanceId, policy));
}

/**
Expand Down Expand Up @@ -1210,7 +1210,8 @@ public ApiFuture<List<String>> testIamPermissionAsync(String instanceId, String.
*/
@SuppressWarnings("WeakerAccess")
public List<String> testIamPermission(ResourceName resourceName, String... permissions) {
return awaitFuture(testIamPermissionAsync(resourceName, permissions));
return ApiExceptions.callAndTranslateApiException(
testIamPermissionAsync(resourceName, permissions));
}


Expand Down Expand Up @@ -1276,29 +1277,4 @@ public com.google.iam.v1.Policy toPb(Policy policy) {
return super.toPb(policy);
}
}

/**
* Awaits the result of a future, taking care to propagate errors while maintaining the call site
* in a suppressed exception. This allows semantic errors to be caught across threads, while
* preserving the call site in the error. The caller's stacktrace will be made available as a
* suppressed exception.
*/
// TODO(igorbernstein2): try to move this into gax
private <T> T awaitFuture(ApiFuture<T> future) {
RuntimeException error;
try {
return Futures.getUnchecked(future);
} catch (UncheckedExecutionException e) {
if (e.getCause() instanceof RuntimeException) {
error = (RuntimeException) e.getCause();
} else {
error = e;
}
} catch (RuntimeException e) {
error = e;
}
// Add the caller's stack as a suppressed exception
error.addSuppressed(new RuntimeException("Encountered error while awaiting future"));
throw error;
}
}
47 changes: 11 additions & 36 deletions ...able-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.api.core.ApiFunction;
import com.google.api.core.ApiFuture;
import com.google.api.core.ApiFutures;
import com.google.api.gax.rpc.ApiExceptions;
import com.google.bigtable.admin.v2.DeleteTableRequest;
import com.google.bigtable.admin.v2.DropRowRangeRequest;
import com.google.bigtable.admin.v2.GetTableRequest;
Expand All @@ -33,9 +34,7 @@
import com.google.cloud.bigtable.admin.v2.stub.EnhancedBigtableTableAdminStub;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.UncheckedExecutionException;
import com.google.protobuf.ByteString;
import com.google.protobuf.Empty;
import java.io.IOException;
Expand Down Expand Up @@ -149,7 +148,7 @@ public void close() {
*/
@SuppressWarnings("WeakerAccess")
public Table createTable(CreateTableRequest request) {
return awaitFuture(createTableAsync(request));
return ApiExceptions.callAndTranslateApiException(createTableAsync(request));
}

/**
Expand Down Expand Up @@ -222,7 +221,7 @@ public ApiFuture<Table> createTableAsync(CreateTableRequest request) {
*/
@SuppressWarnings("WeakerAccess")
public Table modifyFamilies(ModifyColumnFamiliesRequest request) {
return awaitFuture(modifyFamiliesAsync(request));
return ApiExceptions.callAndTranslateApiException(modifyFamiliesAsync(request));
}

/**
Expand Down Expand Up @@ -289,7 +288,7 @@ public ApiFuture<Table> modifyFamiliesAsync(ModifyColumnFamiliesRequest request)
*/
@SuppressWarnings("WeakerAccess")
public void deleteTable(String tableId) {
awaitFuture(deleteTableAsync(tableId));
ApiExceptions.callAndTranslateApiException(deleteTableAsync(tableId));
}

/**
Expand Down Expand Up @@ -342,7 +341,7 @@ public ApiFuture<Void> deleteTableAsync(String tableId) {
*/
@SuppressWarnings("WeakerAccess")
public Table getTable(String tableId) {
return awaitFuture(getTableAsync(tableId));
return ApiExceptions.callAndTranslateApiException(getTableAsync(tableId));
}

/**
Expand Down Expand Up @@ -397,7 +396,7 @@ public ApiFuture<Table> getTableAsync(String tableId) {
// TODO(igorbernstein2): consider changing this method to use relative table ids.
@SuppressWarnings("WeakerAccess")
public List<TableName> listTables() {
return awaitFuture(listTablesAsync());
return ApiExceptions.callAndTranslateApiException(listTablesAsync());
}

/**
Expand Down Expand Up @@ -506,7 +505,7 @@ public List<TableName> apply(List<com.google.bigtable.admin.v2.Table> protos) {
*/
@SuppressWarnings("WeakerAccess")
public void dropRowRange(String tableId, String rowKeyPrefix) {
awaitFuture(dropRowRangeAsync(tableId, rowKeyPrefix));
ApiExceptions.callAndTranslateApiException(dropRowRangeAsync(tableId, rowKeyPrefix));
}

/**
Expand Down Expand Up @@ -552,7 +551,7 @@ public ApiFuture<Void> dropRowRangeAsync(String tableId, String rowKeyPrefix) {
*/
@SuppressWarnings("WeakerAccess")
public void dropRowRange(String tableId, ByteString rowKeyPrefix) {
awaitFuture(dropRowRangeAsync(tableId, rowKeyPrefix));
ApiExceptions.callAndTranslateApiException(dropRowRangeAsync(tableId, rowKeyPrefix));
}

/**
Expand Down Expand Up @@ -604,7 +603,7 @@ public ApiFuture<Void> dropRowRangeAsync(String tableId, ByteString rowKeyPrefix
*/
@SuppressWarnings("WeakerAccess")
public void dropAllRows(String tableId) {
awaitFuture(dropAllRowsAsync(tableId));
ApiExceptions.callAndTranslateApiException(dropAllRowsAsync(tableId));
}

/**
Expand Down Expand Up @@ -659,7 +658,8 @@ public ApiFuture<Void> dropAllRowsAsync(String tableId) {
public void awaitReplication(String tableId) {
TableName tableName = TableName
.of(instanceName.getProject(), instanceName.getInstance(), tableId);
awaitFuture(stub.awaitReplicationCallable().futureCall(tableName));
ApiExceptions
.callAndTranslateApiException(stub.awaitReplicationCallable().futureCall(tableName));
}

/**
Expand Down Expand Up @@ -734,29 +734,4 @@ public Void apply(Empty empty) {
},
MoreExecutors.directExecutor());
}

/**
* Awaits the result of a future, taking care to propagate errors while maintaining the call site
* in a suppressed exception. This allows semantic errors to be caught across threads, while
* preserving the call site in the error. The caller's stacktrace will be made available as a
* suppressed exception.
*/
// TODO(igorbernstein2): try to move this into gax
private <T> T awaitFuture(ApiFuture<T> future) {
RuntimeException error;
try {
return Futures.getUnchecked(future);
} catch (UncheckedExecutionException e) {
if (e.getCause() instanceof RuntimeException) {
error = (RuntimeException) e.getCause();
} else {
error = e;
}
} catch (RuntimeException e) {
error = e;
}
// Add the caller's stack as a suppressed exception
error.addSuppressed(new RuntimeException("Encountered error while awaiting future"));
throw error;
}
}
Loading

0 comments on commit 827230e

Please sign in to comment.