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

couchbase: add CAS support #86

Merged
merged 6 commits into from
Aug 12, 2024

Conversation

sapk
Copy link
Contributor

@sapk sapk commented Aug 1, 2024

Set and use Couchbase CAS in metadata couchbase_cas to prevent write conflict.
If an entry is modified by an other process between read and write it will return an error if CAS is mismatched.

This change should be transparent and prevent data loss by default.
This check can be disabled by clearing the CAS value if needed.

Copy link
Collaborator

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

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

Thanks for this great contribution @sapk! I've left a couple of comments and suggestions. Mostly some minor grammar and spelling fixes. Also, feel free to discard any nit comments.

I'm curious whether we should be allowing for these CAS checks to be toggled via the config instead of making a user clear out metadata fields with bloblang. Not super familiar with Couchbase though so maybe it's more desirable to attach these values by default (like what your PR is currently doing). Let me know your thoughts on this.

internal/impl/couchbase/processor.go Outdated Show resolved Hide resolved
internal/impl/couchbase/processor.go Outdated Show resolved Hide resolved
internal/impl/couchbase/processor_test.go Outdated Show resolved Hide resolved
internal/impl/couchbase/processor_test.go Outdated Show resolved Hide resolved
internal/impl/couchbase/processor_test.go Outdated Show resolved Hide resolved
internal/impl/couchbase/processor_test.go Outdated Show resolved Hide resolved
internal/impl/couchbase/processor.go Outdated Show resolved Hide resolved
internal/impl/couchbase/processor.go Outdated Show resolved Hide resolved
sapk and others added 2 commits August 3, 2024 14:17
Co-authored-by: Greg Furman <31275503+gregfurman@users.noreply.github.com>
@sapk
Copy link
Contributor Author

sapk commented Aug 9, 2024

It should be good now.

@gregfurman
Copy link
Collaborator

@sapk Thanks for the follow-up! I added one small suggestion for the docs to no longer include that deleted() approach of disabling things. If you're happy with that, just regen the docs and we can merge this in.

sapk and others added 2 commits August 12, 2024 17:15
Co-authored-by: Greg Furman <31275503+gregfurman@users.noreply.github.com>
Copy link
Collaborator

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@gregfurman gregfurman merged commit 60879af into warpstreamlabs:main Aug 12, 2024
3 checks passed
@sapk sapk deleted the couchbase-add-cas branch August 13, 2024 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants