Skip to content

Conversation

@rajatbhatta
Copy link
Contributor

@rajatbhatta rajatbhatta commented Jul 6, 2023

No description provided.

@product-auto-label product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner API. labels Jul 6, 2023
@rajatbhatta rajatbhatta marked this pull request as ready for review July 6, 2023 13:13
@rajatbhatta rajatbhatta requested a review from a team as a code owner July 6, 2023 13:13
@rajatbhatta rajatbhatta requested review from arpan14 and olavloite July 6, 2023 13:13
*
* @return ServerStream\<BatchWriteResponse>
*/
ServerStream<BatchWriteResponse> batchWriteAtLeastOnce(Iterable<MutationGroup> mutationGroups)
Copy link
Collaborator

Choose a reason for hiding this comment

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

When should I create multiple MutationGroups instead of just one large mutation group? What's the tradeoff between a large number of small MutationGroups vs a few large MutationGroups? Is there any limits on the number of mutations that I may have in a MutationGroup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When should I create multiple MutationGroups instead of just one large mutation group?

Dependent mutations (to be committed together) should be part of a single MutationGroup.

For example, the following should be part of same MutationGroup:

  • Inserting rows into both parent and child tables. If they're part of different MutationGroups, it will result in a failure when a commit on the child table’s row is attempted first.
  • Inserting rows into tables T1, T2; T2 has a foreign key pointing to T1. If they're part of different MutationGroups, it will result in a failure when a commit on T2 is attempted first.

If there's a need to create just one large MutationGroup, it's better to use the existing Commit RPC for it.

What's the tradeoff between a large number of small MutationGroups vs a few large MutationGroups?

Depends on the use case. The bottomline is we should only put mutations meant to be committed together in a group, otherwise, should prefer a different group.

Is there any limits on the number of mutations that I may have in a MutationGroup?

Considering that in the base case, each mutation group could be committed into its own transaction, which has a limit of 40k mutations, the same limit would apply to the MutationGroup also.

@rajatbhatta rajatbhatta changed the title feat: add support for BatchWriteAtleastOnce feat: add support for BatchWriteAtLeastOnce Aug 23, 2023
@rajatbhatta rajatbhatta requested a review from olavloite August 23, 2023 07:29
Copy link
Collaborator

@olavloite olavloite left a comment

Choose a reason for hiding this comment

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

LGTM, with a couple of nits on the documentation. This is a non-trivial feature, so making sure that we make the documentation as clear as possible will help our users make the most out of this.

@arpan14 arpan14 added the owlbot:run Add this label to trigger the Owlbot post processor. label Sep 26, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Sep 26, 2023
@arpan14 arpan14 merged commit 8ea7bd1 into googleapis:main Sep 26, 2023
@arpan14 arpan14 deleted the batch-write-review branch September 26, 2023 05:08
surbhigarg92 pushed a commit to surbhigarg92/java-spanner that referenced this pull request Oct 5, 2023
* feat: add support for BatchWriteAtleastOnce * test: add batchwrite() support to MockSpannerServiceImpl * test: add commit timestamp to proto * test: add commit timestamp to proto * test: add commit timestamp to proto * consume the stream in tests * refactor tests * refactor tests * test if mutations are correctly applied * null check * skip for emulator * add method documentation * add method documentation * add method documentation * remove autogenerated code * remove autogenerated tests * batchWriteAtleastOnce -> batchWriteAtLeastOnce * batchWriteAtleastOnceWithOptions -> batchWriteAtLeastOnceWithOptions * changes based on updated batch write API * add copyright and doc * address review comments * address review comments * add more documentation --------- Co-authored-by: Arpan Mishra <akmish3@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the googleapis/java-spanner API. size: l Pull request size is large.

3 participants