-
Notifications
You must be signed in to change notification settings - Fork 106
refactor: decouple ConsistencyRequest from TableAdminRequestContext #2887
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
base: main
Are you sure you want to change the base?
Changes from all commits
b68d628
6e39723
59addc0
35e797a
7bed5b9
bb2a0a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,9 +43,11 @@ public abstract class ConsistencyRequest { | |
| @Nullable | ||
| public abstract String getConsistencyToken(); | ||
|
|
||
| protected abstract boolean isFullyQualified(); | ||
|
|
||
| public static ConsistencyRequest forReplication(String tableId) { | ||
| return new AutoValue_ConsistencyRequest( | ||
| tableId, CheckConsistencyRequest.ModeCase.STANDARD_READ_REMOTE_WRITES, null); | ||
| tableId, CheckConsistencyRequest.ModeCase.STANDARD_READ_REMOTE_WRITES, null, false); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -59,37 +61,106 @@ public static ConsistencyRequest forReplication(String tableId, String consisten | |
| Preconditions.checkNotNull(consistencyToken, "consistencyToken must not be null"); | ||
|
|
||
| return new AutoValue_ConsistencyRequest( | ||
| tableId, CheckConsistencyRequest.ModeCase.STANDARD_READ_REMOTE_WRITES, consistencyToken); | ||
| tableId, | ||
| CheckConsistencyRequest.ModeCase.STANDARD_READ_REMOTE_WRITES, | ||
| consistencyToken, | ||
| false); | ||
| } | ||
|
|
||
| public static ConsistencyRequest forDataBoost(String tableId) { | ||
| return new AutoValue_ConsistencyRequest( | ||
| tableId, CheckConsistencyRequest.ModeCase.DATA_BOOST_READ_LOCAL_WRITES, null); | ||
| tableId, CheckConsistencyRequest.ModeCase.DATA_BOOST_READ_LOCAL_WRITES, null, false); | ||
| } | ||
|
|
||
| @InternalApi | ||
| public CheckConsistencyRequest toCheckConsistencyProto( | ||
| TableAdminRequestContext requestContext, String token) { | ||
| public static ConsistencyRequest forReplicationFromTableName(String tableName) { | ||
| return new AutoValue_ConsistencyRequest( | ||
| tableName, CheckConsistencyRequest.ModeCase.STANDARD_READ_REMOTE_WRITES, null, true); | ||
| } | ||
|
|
||
| @InternalApi | ||
| public static ConsistencyRequest forReplicationFromTableName( | ||
| String tableName, String consistencyToken) { | ||
| Preconditions.checkNotNull(consistencyToken, "consistencyToken must not be null"); | ||
|
|
||
| return new AutoValue_ConsistencyRequest( | ||
| tableName, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (just a general question): If this uses the fully qualified name (i assume by the true param), do we need to do any validation to ensure that the input is fully qualified? What happens if isn't? |
||
| CheckConsistencyRequest.ModeCase.STANDARD_READ_REMOTE_WRITES, | ||
| consistencyToken, | ||
| true); | ||
| } | ||
|
|
||
| private CheckConsistencyRequest.Builder buildBaseRequest(String name, String token) { | ||
| CheckConsistencyRequest.Builder builder = CheckConsistencyRequest.newBuilder(); | ||
| TableName tableName = | ||
| TableName.of(requestContext.getProjectId(), requestContext.getInstanceId(), getTableId()); | ||
|
|
||
| if (getMode().equals(CheckConsistencyRequest.ModeCase.STANDARD_READ_REMOTE_WRITES)) { | ||
| builder.setStandardReadRemoteWrites(StandardReadRemoteWrites.newBuilder().build()); | ||
| } else { | ||
| builder.setDataBoostReadLocalWrites(DataBoostReadLocalWrites.newBuilder().build()); | ||
| } | ||
|
|
||
| return builder.setName(tableName.toString()).setConsistencyToken(token).build(); | ||
| return builder.setName(name).setConsistencyToken(token); | ||
| } | ||
|
|
||
| /** | ||
| * Creates a CheckConsistencyRequest proto. This variant is used when the ConsistencyRequest was | ||
| * initialized with a short table ID, relying on the TableAdminRequestContext to construct the | ||
| * fully qualified table name. | ||
| */ | ||
| @InternalApi | ||
| public CheckConsistencyRequest toCheckConsistencyProto( | ||
| TableAdminRequestContext requestContext, String token) { | ||
| Preconditions.checkState( | ||
| !isFullyQualified(), | ||
| "Use toCheckConsistencyProto(String token) for fully qualified table names."); | ||
| TableName tableName = | ||
| TableName.of(requestContext.getProjectId(), requestContext.getInstanceId(), getTableId()); | ||
|
|
||
| return buildBaseRequest(tableName.toString(), token).build(); | ||
| } | ||
|
|
||
| /** | ||
| * Creates a CheckConsistencyRequest proto. This variant is used when the ConsistencyRequest was | ||
| * initialized with a fully qualified table name, eliminating the need for a request context. | ||
| */ | ||
| @InternalApi | ||
| public CheckConsistencyRequest toCheckConsistencyProto(String token) { | ||
| Preconditions.checkState( | ||
| isFullyQualified(), | ||
| "Use toCheckConsistencyProto(TableAdminRequestContext, String) for non-qualified table" | ||
| + " names."); | ||
|
|
||
| return buildBaseRequest(getTableId(), token).build(); | ||
| } | ||
|
|
||
| /** | ||
| * Creates a GenerateConsistencyTokenRequest proto. This variant is used when the | ||
| * ConsistencyRequest was initialized with a short table ID, relying on the | ||
| * TableAdminRequestContext to construct the fully qualified table name. | ||
| */ | ||
| @InternalApi | ||
| public GenerateConsistencyTokenRequest toGenerateTokenProto( | ||
| TableAdminRequestContext requestContext) { | ||
| Preconditions.checkState( | ||
| !isFullyQualified(), "Use toGenerateTokenProto() for fully qualified table names."); | ||
| GenerateConsistencyTokenRequest.Builder builder = GenerateConsistencyTokenRequest.newBuilder(); | ||
| TableName tableName = | ||
| TableName.of(requestContext.getProjectId(), requestContext.getInstanceId(), getTableId()); | ||
|
|
||
| return builder.setName(tableName.toString()).build(); | ||
| } | ||
|
|
||
| /** | ||
| * Creates a GenerateConsistencyTokenRequest proto. This variant is used when the | ||
| * ConsistencyRequest was initialized with a fully qualified table name, eliminating the need for | ||
| * a request context. | ||
| */ | ||
| @InternalApi | ||
| public GenerateConsistencyTokenRequest toGenerateTokenProto() { | ||
| Preconditions.checkState( | ||
| isFullyQualified(), | ||
| "Use toGenerateTokenProto(TableAdminRequestContext) for non-qualified table names."); | ||
| GenerateConsistencyTokenRequest.Builder builder = GenerateConsistencyTokenRequest.newBuilder(); | ||
| return builder.setName(getTableId()).build(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,14 +54,28 @@ public class EnhancedBigtableTableAdminStub extends GrpcBigtableTableAdminStub { | |
| private final BigtableTableAdminStubSettings settings; | ||
| private final ClientContext clientContext; | ||
|
|
||
| private final TableAdminRequestContext requestContext; | ||
| @javax.annotation.Nullable private final TableAdminRequestContext requestContext; | ||
|
|
||
| @Deprecated private final AwaitReplicationCallable awaitReplicationCallable; | ||
|
|
||
| private final AwaitConsistencyCallable awaitConsistencyCallable; | ||
| private final OperationCallable<Void, Empty, OptimizeRestoredTableMetadata> | ||
| optimizeRestoredTableOperationBaseCallable; | ||
|
|
||
| /** | ||
| * Creates an instance of {@link EnhancedBigtableTableAdminStub} using the provided settings. This | ||
| * variant is used by the V2 client stack which relies on fully qualified table names and | ||
| * therefore does not require a {@link TableAdminRequestContext}. | ||
| * | ||
| * @param settings The settings used to configure the stub. | ||
| * @return A new instance of {@code EnhancedBigtableTableAdminStub}. | ||
| * @throws IOException If there are errors creating the underlying client context. | ||
| */ | ||
| public static EnhancedBigtableTableAdminStub createEnhanced( | ||
| BigtableTableAdminStubSettings settings) throws IOException { | ||
| return new EnhancedBigtableTableAdminStub(settings, ClientContext.create(settings), null); | ||
| } | ||
|
jinseopkim0 marked this conversation as resolved.
|
||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Not something that needs to be done in this PR. Thoughts on adding a similar one for the e.g. are there certain cases where they don't know the fully qualified |
||
| public static EnhancedBigtableTableAdminStub createEnhanced( | ||
| BigtableTableAdminStubSettings settings, TableAdminRequestContext requestContext) | ||
| throws IOException { | ||
|
|
@@ -72,7 +86,7 @@ public static EnhancedBigtableTableAdminStub createEnhanced( | |
| private EnhancedBigtableTableAdminStub( | ||
| BigtableTableAdminStubSettings settings, | ||
| ClientContext clientContext, | ||
| TableAdminRequestContext requestContext) | ||
| @javax.annotation.Nullable TableAdminRequestContext requestContext) | ||
| throws IOException { | ||
| super(settings, clientContext); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.