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

CCR: Following primary should process NoOps once #34408

Merged
merged 5 commits into from Oct 20, 2018

Conversation

Projects
None yet
3 participants
@dnhatn
Contributor

dnhatn commented Oct 11, 2018

This is a follow-up for #34288 (comment).

Relates #34288

@elasticmachine

This comment has been minimized.

Show comment
Hide comment
@elasticmachine

elasticmachine commented Oct 11, 2018

protected Optional<Exception> preFlightCheckForNoOp(NoOp noOp) throws IOException {
if (noOp.origin() == Operation.Origin.PRIMARY && hasBeenProcessedBefore(noOp)) {
// See the comment in #indexingStrategyForOperation for the explanation why we can safely skip this operation.
final OptionalLong existingTerm = lookupPrimaryTerm(noOp.seqNo());

This comment has been minimized.

@bleskes

bleskes Oct 19, 2018

Member

random thought - I wonder if we should load the operation under assertion code and check it's the same (this goes for all duplicate ops).

@bleskes

bleskes Oct 19, 2018

Member

random thought - I wonder if we should load the operation under assertion code and check it's the same (this goes for all duplicate ops).

This comment has been minimized.

@dnhatn

dnhatn Oct 19, 2018

Contributor

Do you mean the existing operation should equal the processing operation except for the primary term?

@dnhatn

dnhatn Oct 19, 2018

Contributor

Do you mean the existing operation should equal the processing operation except for the primary term?

This comment has been minimized.

@bleskes

bleskes Oct 19, 2018

Member

yes

@bleskes

bleskes Oct 19, 2018

Member

yes

This comment has been minimized.

@dnhatn

dnhatn Oct 19, 2018

Contributor

++. I'll make it in a follow-up after this PR.

@dnhatn

dnhatn Oct 19, 2018

Contributor

++. I'll make it in a follow-up after this PR.

fix testHandleDocumentFailure
Clear the injected exception after the indexing
@dnhatn

This comment has been minimized.

Show comment
Hide comment
@dnhatn

dnhatn Oct 20, 2018

Contributor

Thanks @bleskes.

Contributor

dnhatn commented Oct 20, 2018

Thanks @bleskes.

@dnhatn dnhatn merged commit d90b673 into elastic:master Oct 20, 2018

4 checks passed

CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci Build finished.
Details
elasticsearch-ci/oss-distro-docs Build finished.
Details
elasticsearch-ci/packaging-sample Build finished.
Details

@dnhatn dnhatn deleted the dnhatn:noop-once branch Oct 20, 2018

dnhatn added a commit that referenced this pull request Oct 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment