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

fix(spanner): support transaction tags in partition DML #3473

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ If you are using Maven without the BOM, add this to your dependencies:
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-spanner</artifactId>
<version>6.81.0</version>
<version>6.81.1</version>
</dependency>

```
Expand Down Expand Up @@ -516,6 +516,7 @@ Samples are in the [`samples/`](https://github.com/googleapis/java-spanner/tree/
| Create Instance Partition Sample | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/CreateInstancePartitionSample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/CreateInstancePartitionSample.java) |
| Create Instance With Autoscaling Config Example | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/CreateInstanceWithAutoscalingConfigExample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/CreateInstanceWithAutoscalingConfigExample.java) |
| Create Instance With Processing Units Example | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/CreateInstanceWithProcessingUnitsExample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/CreateInstanceWithProcessingUnitsExample.java) |
| Create Instance Without Default Backup Schedules Example | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/CreateInstanceWithoutDefaultBackupSchedulesExample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/CreateInstanceWithoutDefaultBackupSchedulesExample.java) |
| Create Sequence Sample | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/CreateSequenceSample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/CreateSequenceSample.java) |
| Create Table With Foreign Key Delete Cascade Sample | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/CreateTableWithForeignKeyDeleteCascadeSample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/CreateTableWithForeignKeyDeleteCascadeSample.java) |
| Custom Timeout And Retry Settings Example | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/CustomTimeoutAndRetrySettingsExample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/CustomTimeoutAndRetrySettingsExample.java) |
Expand Down Expand Up @@ -571,6 +572,7 @@ Samples are in the [`samples/`](https://github.com/googleapis/java-spanner/tree/
| Update Database Sample | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/UpdateDatabaseSample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/UpdateDatabaseSample.java) |
| Update Database With Default Leader Sample | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/UpdateDatabaseWithDefaultLeaderSample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/UpdateDatabaseWithDefaultLeaderSample.java) |
| Update Instance Config Sample | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/UpdateInstanceConfigSample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/UpdateInstanceConfigSample.java) |
| Update Instance Default Backup Schedule Type Example | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/UpdateInstanceDefaultBackupScheduleTypeExample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/UpdateInstanceDefaultBackupScheduleTypeExample.java) |
| Update Instance Example | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/UpdateInstanceExample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/UpdateInstanceExample.java) |
| Update Json Data Sample | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/UpdateJsonDataSample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/UpdateJsonDataSample.java) |
| Update Jsonb Data Sample | [source code](https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/main/java/com/example/spanner/UpdateJsonbDataSample.java) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/java-spanner&page=editor&open_in_editor=samples/snippets/src/main/java/com/example/spanner/UpdateJsonbDataSample.java) |
Expand Down
2 changes: 1 addition & 1 deletion google-cloud-spanner/clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -790,5 +790,5 @@
<className>com/google/cloud/spanner/connection/Connection</className>
<method>boolean isAutoBatchDmlUpdateCountVerification()</method>
</difference>

</differences>
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ public static ReadQueryUpdateTransactionOption tag(String name) {
return new TagOption(name);
}

public static ReadQueryUpdateTransactionOption transactionTag(String name) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this would bring in confusion to customers, mainly because transaction tags are supported in read-write transactions by using Options.tag. Customers might try using this Options.transactionTag for read-write and will not work.
https://cloud.google.com/spanner/docs/introspection/troubleshooting-with-tags#java_1

Should we rename this to be more specific to PDML or is this somehow restricted to be used only for pdml transaction?

return new TransactionTagOption(name);
}

/**
* Specifying this will cause the list operations to fetch at most this many records in a page.
*/
Expand Down Expand Up @@ -394,6 +398,24 @@ void appendToOptions(Options options) {
}
}

static final class TransactionTagOption extends InternalOption
implements ReadQueryUpdateTransactionOption {
private final String transactionTag;

TransactionTagOption(String transactionTag) {
this.transactionTag = transactionTag;
}

String getTransactionTag() {
return transactionTag;
}

@Override
void appendToOptions(Options options) {
options.transactionTag = transactionTag;
}
}

static final class EtagOption extends InternalOption implements DeleteAdminApiOption {
private final String etag;

Expand Down Expand Up @@ -462,6 +484,7 @@ void appendToOptions(Options options) {
private RpcPriority priority;
private String tag;
private String etag;
private String transactionTag;
private Boolean validateOnly;
private Boolean withOptimisticLock;
private Boolean withExcludeTxnFromChangeStreams;
Expand Down Expand Up @@ -545,6 +568,14 @@ boolean hasTag() {
return tag != null;
}

boolean hasTransactionTag() {
return transactionTag != null;
}

String transactionTag() {
return transactionTag;
}

String tag() {
return tag;
}
Expand Down Expand Up @@ -661,6 +692,9 @@ public String toString() {
if (orderBy != null) {
b.append("orderBy: ").append(orderBy).append(' ');
}
if (transactionTag != null) {
b.append("transactionTag: ").append(transactionTag).append(' ');
}
return b.toString();
}

Expand Down Expand Up @@ -694,6 +728,7 @@ public boolean equals(Object o) {
&& Objects.equals(filter(), that.filter())
&& Objects.equals(priority(), that.priority())
&& Objects.equals(tag(), that.tag())
&& Objects.equals(transactionTag, that.transactionTag)
&& Objects.equals(etag(), that.etag())
&& Objects.equals(validateOnly(), that.validateOnly())
&& Objects.equals(withOptimisticLock(), that.withOptimisticLock())
Expand Down Expand Up @@ -760,6 +795,9 @@ public int hashCode() {
if (orderBy != null) {
result = 31 * result + orderBy.hashCode();
}
if (transactionTag != null) {
result = 31 * result + transactionTag.hashCode();
}
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,22 +194,29 @@ ExecuteSqlRequest newTransactionRequestFrom(final Statement statement, final Opt
if (options.hasTag()) {
requestOptionsBuilder.setRequestTag(options.tag());
}
if (options.hasTransactionTag()) {
requestOptionsBuilder.setTransactionTag(options.transactionTag());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The official documentation states that transaction tags not supported for read-only transactions and are supported for read-write, but there’s no mention of partitioned DML transactions.
can you please verify if transaction tags are indeed supported for pdml txn?

}
builder.setRequestOptions(requestOptionsBuilder.build());
}
return builder.build();
}

private ByteString initTransaction(final Options options) {
final BeginTransactionRequest request =
BeginTransactionRequest.Builder builder =
BeginTransactionRequest.newBuilder()
.setSession(session.getName())
.setOptions(
TransactionOptions.newBuilder()
.setPartitionedDml(TransactionOptions.PartitionedDml.getDefaultInstance())
.setExcludeTxnFromChangeStreams(
options.withExcludeTxnFromChangeStreams() == Boolean.TRUE))
.build();
Transaction tx = rpc.beginTransaction(request, session.getOptions(), true);
options.withExcludeTxnFromChangeStreams() == Boolean.TRUE));

if (options.hasTransactionTag()) {
builder.setRequestOptions(
RequestOptions.newBuilder().setTransactionTag(options.transactionTag()).build());
}
Transaction tx = rpc.beginTransaction(builder.build(), session.getOptions(), true);
if (tx.getId().isEmpty()) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.INTERNAL,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1944,6 +1944,32 @@ public void testPartitionedDMLWithTag() {
assertThat(request.getRequestOptions().getTransactionTag()).isEmpty();
}

@Test
public void testPartitionedDMLWithTransactionTag() {
DatabaseClient client =
spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE));
client.executePartitionedUpdate(
UPDATE_STATEMENT,
Options.transactionTag("testTransactionTag"),
Options.tag("app=spanner,env=test,action=dml"));

List<BeginTransactionRequest> beginTransactions =
mockSpanner.getRequestsOfType(BeginTransactionRequest.class);
assertThat(beginTransactions).hasSize(1);
BeginTransactionRequest beginTransaction = beginTransactions.get(0);
assertNotNull(beginTransaction.getOptions());
assertTrue(beginTransaction.getOptions().hasPartitionedDml());
assertFalse(beginTransaction.getOptions().getExcludeTxnFromChangeStreams());

List<ExecuteSqlRequest> requests = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class);
assertThat(requests).hasSize(1);
ExecuteSqlRequest request = requests.get(0);
assertNotNull(request.getRequestOptions());
assertThat(request.getRequestOptions().getTransactionTag()).isEqualTo("testTransactionTag");
assertThat(request.getRequestOptions().getRequestTag())
.isEqualTo("app=spanner,env=test,action=dml");
}

@Test
public void testCommitWithTag() {
DatabaseClient client =
Expand Down
Loading