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.

Sign up for GitHub

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

Jump to bottom

[WIP] Sequence Numbers powered Compare and Set operations #36148

Closed
wants to merge 49 commits into from
Closed

[WIP] Sequence Numbers powered Compare and Set operations #36148

wants to merge 49 commits into from

Conversation

bleskes
Copy link
Contributor

@bleskes bleskes commented Dec 3, 2018

Elasticsearch has long supported compare and set (a.k.a optimistic concurrency control) operations using internal document versioning. Sadly that approach is flawed and can sometime do the wrong thing. Here's the relevant excerpt from the resiliency status page:

When a primary has been partitioned away from the cluster there is a short period of time until it detects this. During that time it will continue indexing writes locally, thereby updating document versions. When it tries to replicate the operation, however, it will discover that it is partitioned away. It won’t acknowledge the write and will wait until the partition is resolved to negotiate with the master on how to proceed. The master will decide to either fail any replicas which failed to index the operations on the primary or tell the primary that it has to step down because a new primary has been chosen in the meantime. Since the old primary has already written documents, clients may already have read from the old primary before it shuts itself down. The version numbers of these reads may not be unique if the new primary has already accepted writes for the same document

This PR introduces a new sequence number based approach that doesn't suffer from this dirty reads problem. The purpose for the PR is get initial feedback and serve as a basis for a BWC discussion.

Concretely I would like to get feedback:

  1. The Engine implementation
  2. Rest API
  3. I'm currently still using VersionConflictException, I'm thinking of renaming it to ElasticsearchConflictException
  4. BWC aspects

Once we clear those out, I'll break this up into smaller PRs and add documentation

bleskes added 30 commits October 5, 2018 14:26
@bleskes
initial version
fed3cb9
@bleskes
lint
c6fca4c
@bleskes
Merge remote-tracking branch 'upstream/master' into opt_seq_no
97d407b
@bleskes
adapted test
a91d18f
@bleskes
lint
60680ae
@bleskes
initial rest parsing
c177f27
@bleskes
fix version test
f8645ae
@bleskes
fix validation
17d4c5a
@bleskes
magic numbers are tricky
24fbaeb
@bleskes
Merge remote-tracking branch 'upstream/master' into opt_seq_no
b57e9a0
@bleskes
seq# on delete fix
d7ffbe2
@bleskes
Merge remote-tracking branch 'upstream/master' into opt_seq_no
903cef5
@bleskes
fix rest params
d3badd7
@bleskes
Merge remote-tracking branch 'upstream/master' into opt_seq_no
9a837db
@bleskes
rest tests
c7917c1
@bleskes
compile
93bb437
@bleskes
fix rest
96fb99e
@bleskes
rest doc
12316f0
@bleskes
fix rest
005c477
@bleskes
Merge remote-tracking branch 'upstream/master' into opt_seq_no
d7c5451
@bleskes
Merge remote-tracking branch 'upstream/master' into opt_seq_no
11ad5fa
@bleskes
Merge remote-tracking branch 'upstream/master' into opt_seq_no
0bcb1dd 
# Conflicts:
#	server/src/main/java/org/elasticsearch/rest/action/document/RestDeleteAction.java
#	x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/index/engine/FollowingEngineTests.java
@bleskes
fix compilation issues
d59712d
@bleskes
iter
c139f7d
@bleskes
Merge remote-tracking branch 'upstream/master' into opt_seq_no
0c62332
@bleskes
fix get response tests
d2b6b6d
@bleskes
updates should use the right seq no info
1cb3faa
@bleskes
line length
c87530d
@bleskes
Merge remote-tracking branch 'upstream/master' into opt_seq_no
3ae2168
@bleskes
compile
4e20e2b
bleskes added a commit that referenced this pull request Dec 18, 2018
@bleskes
Expose Sequence Number based Optimistic Concurrency Control in the re…
a8a7e6e 
…st layer (#36721)

Relates #36148
Relates #10708
bleskes added a commit that referenced this pull request Dec 18, 2018
@bleskes
Rename seq# powered optimistic concurrency control parameters to ifSe…
1db1565 
…qNo/ifPrimaryTerm (#36757)

This PR renames the parameters previously introduce to the following:

### URL Parameters
```
PUT twitter/_doc/1?if_seq_no=501&if_primary_term=1
{
    "user" : "kimchy",
    "post_date" : "2009-11-15T14:12:12",
    "message" : "trying out Elasticsearch"
}

DELETE twitter/_doc/1?if_seq_no=501&if_primary_term=1
```

### Bulk API
```
POST _bulk
{ "index" : { "_index" : "test", "_type" : "_doc", "_id" : "1", "if_seq_no": 501, "if_primary_term": 1 } }
{ "field1" : "value1" }
{ "delete" : { "_index" : "test", "_type" : "_doc", "_id" : "2", "if_seq_no": 501, "if_primary_term": 1 } }
```

### Java API
```
IndexRequest.ifSeqNo(long seqNo)
IndexRequest.ifPrimaryTerm(long primaryTerm)
DeleteRequest.ifSeqNo(long seqNo)
DeleteRequest.ifPrimaryTerm(long primaryTerm)
```

Relates #36148
Relates #10708
bleskes added a commit that referenced this pull request Dec 18, 2018
@bleskes
enable bwc tests and switch transport serialization version to 6.6.0 …
10f6408 
…for CAS features

Relates to #36148
This was referenced Jan 10, 2019
Merged
Merged
bleskes added a commit that referenced this pull request Jan 11, 2019
@bleskes
Use Sequence number powered OCC for processing updates (#37308)
d21df2a 
Updates perform realtime get, perform the requested update and then index the document again
using optimistic concurrency control. This PR changes the logic to use sequence numbers instead
of versioning. 

Note that the current versioning logic isn't suffering from the same problem as external OCC
requests because the get and indexing is always done on the same primary.

Relates #36148 
Relates #10708
bleskes added a commit that referenced this pull request Jan 11, 2019
@bleskes
Use Sequence number powered OCC for processing updates (#37308)
204d20a 
Updates perform realtime get, perform the requested update and then index the document again
using optimistic concurrency control. This PR changes the logic to use sequence numbers instead
of versioning.

Note that the current versioning logic isn't suffering from the same problem as external OCC
requests because the get and indexing is always done on the same primary.

Relates #36148
Relates #10708
jaymode pushed a commit that referenced this pull request Jan 11, 2019
@bleskes
Document Seq No powered optimistic concurrency control (#37284)
cae71cd 
Add documentation to describe the new sequence number powered optimistic concurrency control

Relates #36148
Relates #10708
bleskes added a commit that referenced this pull request Jan 11, 2019
@bleskes
Document Seq No powered optimistic concurrency control (#37284)
2ee2c29 
Add documentation to describe the new sequence number powered optimistic concurrency control

Relates #36148
Relates #10708
bleskes added a commit that referenced this pull request Jan 11, 2019
@bleskes
Document Seq No powered optimistic concurrency control (#37284)
28069db 
Add documentation to describe the new sequence number powered optimistic concurrency control

Relates #36148
Relates #10708
@bleskes
Copy link
Contributor Author

bleskes commented Jan 11, 2019

This has now been fully implemented. Deprecation and removal of OCC via internal versioning will be tracked separately. Thanks all for the feedback.

@bleskes bleskes closed this Jan 11, 2019
@bleskes bleskes deleted the cas_seq_no branch January 11, 2019 16:36
This was referenced Jan 25, 2019
Merged
Merged
bleskes added a commit that referenced this pull request Jan 29, 2019
@bleskes
Add Seq# based optimistic concurrency control to UpdateRequest (#37872)
65a9b61 
The update request has a lesser known support for a one off update of a known document version. This PR adds an a seq# based alternative to power these operations.

Relates #36148 
Relates #10708
bleskes added a commit that referenced this pull request Jan 29, 2019
@bleskes
Move update and delete by query to use seq# for optimistic concurrenc…
218df30 
…y control (#37857)

The delete and update by query APIs both offer protection against overriding concurrent user changes to the documents they touch. They currently are using internal versioning. This PR changes that to rely on sequences numbers and primary terms.

Relates #37639 
Relates #36148 
Relates #10708
bleskes added a commit to bleskes/elasticsearch that referenced this pull request Feb 1, 2019
@bleskes
Move update and delete by query to use seq# for optimistic concurrenc…
8270021 
…y control (elastic#37857)

The delete and update by query APIs both offer protection against overriding concurrent user changes to the documents they touch. They currently are using internal versioning. This PR changes that to rely on sequences numbers and primary terms.

Relates elastic#37639
Relates elastic#36148
Relates elastic#10708
bleskes added a commit to bleskes/elasticsearch that referenced this pull request Feb 1, 2019
@bleskes
Add Seq# based optimistic concurrency control to UpdateRequest (elast…
85d7d50 
…ic#37872)

The update request has a lesser known support for a one off update of a known document version. This PR adds an a seq# based alternative to power these operations.

Relates elastic#36148
Relates elastic#10708
bleskes added a commit that referenced this pull request Feb 1, 2019
@bleskes
6.x Backport of sequence number powered CAS PRs: #37977, #37857 and #…
c7ef318 
…37872 (#38155)

* Move update and delete by query to use seq# for optimistic concurrency control (#37857)

The delete and update by query APIs both offer protection against overriding concurrent user changes to the documents they touch. They currently are using internal versioning. This PR changes that to rely on sequences numbers and primary terms.

Relates #37639
Relates #36148
Relates #10708

* Add Seq# based optimistic concurrency control to UpdateRequest (#37872)

The update request has a lesser known support for a one off update of a known document version. This PR adds an a seq# based alternative to power these operations.

Relates #36148
Relates #10708

* Move watcher to use seq# and primary term for concurrency control (#37977)

* Adapt minimum versions for seq# power operations

After backporting #37977, #37857 and #37872
@bleskes bleskes mentioned this pull request Feb 3, 2019
Merged
bleskes added a commit that referenced this pull request Feb 4, 2019
@bleskes
Move ML Optimistic Concurrency Control to Seq No (#38278)
ff13a43 
This commit moves the usage of internal versioning for CAS operations to use sequence numbers and primary terms

Relates to #36148
Relates to #10708
bleskes added a commit that referenced this pull request Feb 4, 2019
@bleskes
6.x Backport of #38278: Move ML Optimistic Concurrency Control to Seq No
bfcb11b 
This commit moves the usage of internal versioning for CAS operations to use sequence numbers and primary terms

Relates to #36148
Relates to #10708
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasontedor jasontedor jasontedor left review comments

@s1monw s1monw s1monw left review comments

@ywelsch ywelsch Awaiting requested review from ywelsch

Assignees
No one assigned
Labels
:Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. WIP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bleskes @elasticmachine @s1monw @ywelsch @jasontedor