Support atomic writes in the docstore #3500

sandeepvinayak · 2024-10-11T21:26:28Z

docstore/all

Details in the issue : #3501

Please reference any Issue related to this Pull Request. Example: Fixes #1.

See
here
for tips on good Pull Request description.

google-cla · 2024-10-11T21:26:32Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

sandeepvinayak · 2024-10-11T21:32:47Z

@eliben @vangent We are trying to add the transactions support/atomic writes in the docstore and this is the high level idea of how can we support that in go-cloud. https://docs.google.com/document/d/1UVj1kmwDfrs5qm8r7X1p4fAFmsdPEeBHcvWJ8zWF1dY/edit?tab=t.0

The PR is just at a high level to support the idea.
Let me know what you think about it and we can help to drive it further.

All modules are on a Go version after 1.18, so we can rename interface{} to any to modernize the code. docstore is omitted, because there are PRs in flight that this might conflict with (#3500, #3508).

docstore/awsdynamodb/dynamo.go

jba · 2025-01-16T17:05:08Z

docstore/awsdynamodb/dynamo.go

+func (c *collection) transactWrite(ctx context.Context, actions []*driver.Action, errs []error, opts *driver.RunActionsOptions) {
+	if len(actions) == 0 {
+		return
+	}
 	setErr := func(err error) {


The idea here is that if anything in this function fails, everything fails (since it's atomic). Document that here.

docstore/awsdynamodb/dynamo.go

jba · 2025-01-16T17:08:35Z

docstore/docstore.go

@@ -534,6 +537,12 @@ func (l *ActionList) String() string {
 	return "[" + strings.Join(as, ", ") + "]"
 }

+// AtomicWrites causes all following writes in the list to execute atomically.


"to execute as a single atomic operation"

(to avoid confusion with the fact that each write already happens atomically by itself)

docstore/driver/util.go

jba · 2025-01-16T17:14:52Z

docstore/driver/util_test.go

@@ -79,7 +79,7 @@ func TestGroupActions(t *testing.T) {
 	}{
 		{


Please add various cases involving atomic writes, mixed with other writes and gets before, after, and concurrently.

docstore/drivertest/drivertest.go

docstore/memdocstore/mem.go

sandeepvinayak · 2025-01-22T01:27:08Z

@jba thanks for your feedback, I addressed all of the comments, please take another look

docstore/awsdynamodb/dynamo.go

docstore/drivertest/drivertest.go

sandeepvinayak · 2025-01-22T21:17:44Z

@jba, I addressed your latest 2 comments

codecov · 2025-01-23T17:39:40Z

Codecov Report

Attention: Patch coverage is 71.79487% with 11 lines in your changes missing coverage. Please review.

Project coverage is 73.28%. Comparing base (61f69d3) to head (8d0616d).
Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
docstore/awsdynamodb/dynamo.go	46.66%	8 Missing ⚠️
docstore/docstore.go	75.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3500      +/-   ##
==========================================
+ Coverage   73.26%   73.28%   +0.01%     
==========================================
  Files         113      113              
  Lines       15065    15080      +15     
==========================================
+ Hits        11038    11051      +13     
- Misses       3261     3263       +2     
  Partials      766      766

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sandeepvinayak · 2025-01-23T17:50:32Z

@jba the codecov report is under-reporting because the conformance tests are disabled for now but I verified the tests we added are good for dynamo. I will enable the 2 conformance tests added along with the other providers in the follow up PR.

jba · 2025-01-23T17:53:33Z

Thanks for the PR! Looking forward to the follow-ups.

sandeepvinayak force-pushed the spal-transactions branch from e452b85 to a3911cb Compare October 11, 2024 21:33

sandeepvinayak mentioned this pull request Oct 11, 2024

Transactions/Atomic writes in the docstore #3501

Open

sandeepvinayak force-pushed the spal-transactions branch from a3911cb to 541d19a Compare October 11, 2024 21:38

sandeepvinayak force-pushed the spal-transactions branch 2 times, most recently from bcf60ed to 04a45a7 Compare December 7, 2024 09:15

support atomic writes

c95f790

sandeepvinayak force-pushed the spal-transactions branch from 04a45a7 to c95f790 Compare December 7, 2024 09:17

sandeepvinayak changed the title ~~WIP: Support transaction in the docstore~~ Support atomic writes in the docstore Dec 7, 2024

jba mentioned this pull request Dec 14, 2024

all: rename interface{} to any #3515

Merged

jba requested changes Jan 16, 2025

View reviewed changes

sandeepvinayak force-pushed the spal-transactions branch 2 times, most recently from d87f80e to aa357b9 Compare January 22, 2025 01:23

addressing comments

c8a11bd

sandeepvinayak force-pushed the spal-transactions branch from aa357b9 to c8a11bd Compare January 22, 2025 01:26

sandeepvinayak requested a review from jba January 22, 2025 01:26

jba requested changes Jan 22, 2025

View reviewed changes

docstore/awsdynamodb/dynamo.go Outdated Show resolved Hide resolved

docstore/drivertest/drivertest.go Outdated Show resolved Hide resolved

docstore/drivertest/drivertest.go Outdated Show resolved Hide resolved

sandeepvinayak force-pushed the spal-transactions branch from e46f0b9 to 9c979be Compare January 22, 2025 21:16

sandeepvinayak requested a review from jba January 22, 2025 21:18

sandeepvinayak force-pushed the spal-transactions branch from 9c979be to 6a57b2f Compare January 22, 2025 22:32

addressing comments

8d0616d

sandeepvinayak force-pushed the spal-transactions branch from 6a57b2f to 8d0616d Compare January 23, 2025 04:10

jba approved these changes Jan 23, 2025

View reviewed changes

jba merged commit 4d1f585 into google:master Jan 23, 2025
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support atomic writes in the docstore #3500

Support atomic writes in the docstore #3500

sandeepvinayak commented Oct 11, 2024 •

edited

Loading

google-cla bot commented Oct 11, 2024

sandeepvinayak commented Oct 11, 2024

jba Jan 16, 2025

jba Jan 16, 2025

jba Jan 16, 2025

sandeepvinayak commented Jan 22, 2025

sandeepvinayak commented Jan 22, 2025

codecov bot commented Jan 23, 2025 •

edited

Loading

sandeepvinayak commented Jan 23, 2025 •

edited

Loading

jba commented Jan 23, 2025

Support atomic writes in the docstore #3500

Support atomic writes in the docstore #3500

Conversation

sandeepvinayak commented Oct 11, 2024 • edited Loading

google-cla bot commented Oct 11, 2024

sandeepvinayak commented Oct 11, 2024

jba Jan 16, 2025

Choose a reason for hiding this comment

jba Jan 16, 2025

Choose a reason for hiding this comment

jba Jan 16, 2025

Choose a reason for hiding this comment

sandeepvinayak commented Jan 22, 2025

sandeepvinayak commented Jan 22, 2025

codecov bot commented Jan 23, 2025 • edited Loading

Codecov Report

sandeepvinayak commented Jan 23, 2025 • edited Loading

jba commented Jan 23, 2025

sandeepvinayak commented Oct 11, 2024 •

edited

Loading

codecov bot commented Jan 23, 2025 •

edited

Loading

sandeepvinayak commented Jan 23, 2025 •

edited

Loading