Add primary term to doc write response#24171
Merged
jasontedor merged 19 commits intoelastic:masterfrom Apr 19, 2017
Merged
Conversation
This commit adds the primary term to the doc write response.
64 tasks
nik9000
reviewed
Apr 19, 2017
docs/reference/docs/bulk.asciidoc
Outdated
| } | ||
| -------------------------------------------------- | ||
| // TESTRESPONSE[s/"took": 30/"took": $body.took/ s/"index_uuid": .../"index_uuid": $body.items.3.update.error.index_uuid/ s/"_seq_no" : 0/"_seq_no" : $body.items.0.index._seq_no/ s/"_seq_no" : 1/"_seq_no" : $body.items.1.delete._seq_no/ s/"_seq_no" : 2/"_seq_no" : $body.items.2.create._seq_no/ s/"_seq_no" : 3/"_seq_no" : $body.items.3.update._seq_no/] | ||
| // TESTRESPONSE[s/"took": 30/"took": $body.took/ s/"index_uuid": .../"index_uuid": $body.items.3.update.error.index_uuid/ s/"_seq_no" : 0/"_seq_no" : $body.items.0.index._seq_no/ s/"_primary_term" : 1/"_primary_term" : $body.items.0.index._primary_term/ s/"_seq_no" : 1/"_seq_no" : $body.items.1.delete._seq_no/ s/"_primary_term" : 2/"_primary_term" : $body.items.1.delete._primary_term/ s/"_seq_no" : 2/"_seq_no" : $body.items.2.create._seq_no/ s/"_primary_term" : 3/"_primary_term" : $body.items.2.create._primary_term/ s/"_seq_no" : 3/"_seq_no" : $body.items.3.update._seq_no/ s/"_primary_term" : 4/"_primary_term" : $body.items.3.update._primary_term/] |
Member
There was a problem hiding this comment.
You can stick these onto separate lines if you like.
bleskes
suggested changes
Apr 19, 2017
Contributor
bleskes
left a comment
There was a problem hiding this comment.
Looks good - I left one comment about the Engine related classes.
| protected Result(Operation.TYPE operationType, long version, long seqNo) { | ||
| this(operationType, null, version, seqNo); | ||
| protected Result(Operation.TYPE operationType, long version, long seqNo, long primaryTerm) { | ||
| this(operationType, null, version, seqNo, primaryTerm); |
Contributor
There was a problem hiding this comment.
I'm on sure about this one - the semantics we currently have is that Result represents everything that the engine decides. The primary term is part of the operation - I'm not sure we need this here?
| } | ||
| -------------------------------------------------- | ||
| // TESTRESPONSE[s/"took": 30/"took": $body.took/ s/"index_uuid": .../"index_uuid": $body.items.3.update.error.index_uuid/ s/"_seq_no" : 0/"_seq_no" : $body.items.0.index._seq_no/ s/"_seq_no" : 1/"_seq_no" : $body.items.1.delete._seq_no/ s/"_seq_no" : 2/"_seq_no" : $body.items.2.create._seq_no/ s/"_seq_no" : 3/"_seq_no" : $body.items.3.update._seq_no/] | ||
| // TESTRESPONSE[s/"took": 30/"took": $body.took/] |
Contributor
There was a problem hiding this comment.
good to know this is possible
Member
Author
There was a problem hiding this comment.
Yes, I didn't know it was until @nik9000 made the suggestion and so I went and read the source to understand the syntax behind it.
* master: Add BucketMetricValue interface (elastic#24188) Enable index-time sorting (elastic#24055) Clarify elasticsearch user uid:gid mapping in Docker docs Update field-names-field.asciidoc (elastic#24178) ElectMasterService.hasEnoughMasterNodes should return false if no masters were found Remove Ubuntu 12.04 (elastic#24161) [Test] Add unit tests for InternalHDRPercentilesTests (elastic#24157) Replicate write failures (elastic#23314) Rename variable in translog simple commit test Strengthen translog commit with open view test Stronger check in translog prepare and commit test Fix translog prepare commit and commit test ingest-node.asciidoc - Clarify json processor (elastic#21876) Painless: more testing for script_stack (elastic#24168)
bleskes
approved these changes
Apr 19, 2017
This reverts commit 64772ae.
This reverts commit d4bbfcd.
…-term"" This reverts commit 6c17c2c.
* master: Wait for cluster to become quiescent between REST tests (elastic#24148) Plugins: Remove leniency for missing plugins dir (elastic#24173) Changing some grammar in docs (elastic#24164)
jasontedor
commented
Apr 19, 2017
| return new DeleteResult(ex, plan.versionOfDeletion, plan.versionOfDeletion, | ||
| plan.currentlyDeleted == false); | ||
| return new DeleteResult( | ||
| ex, plan.versionOfDeletion, plan.seqNoOfDeletion, plan.currentlyDeleted == false); |
Member
Author
There was a problem hiding this comment.
@bleskes By the way, I think this was a bug, maybe there's a test case missing here?
Contributor
Contributor
There was a problem hiding this comment.
@bleskes thanks for the ping, I am working on adding tests for the delete case
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit adds the primary term to the doc write response.
Relates #10708