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

Call afterWriteOperation after trim translog in peer recovery #45182

Merged
merged 4 commits into from
Aug 5, 2019

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Aug 5, 2019

testShouldFlushAfterPeerRecovery was added #28350 to make sure the flushing loop triggered by afterWriteOperation eventually terminates. This test relies on the fact that we call afterWriteOperation after making changes in translog. In #44756, we roll a new generation in RecoveryTarget#finalizeRecovery but do not call afterWriteOperation.

Relates #28350
Relates #45073

CI: https://scans.gradle.com/s/434yf54fgevvm

@dnhatn dnhatn added >test Issues or PRs that are addressing/adding tests :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. v8.0.0 v7.4.0 labels Aug 5, 2019
@dnhatn dnhatn requested a review from ywelsch August 5, 2019 01:37
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@dnhatn dnhatn changed the title Call afterWriteOperation in testShouldFlushAfterPeerRecovery Call afterWriteOperation after trim translog in peer recovery Aug 5, 2019
@dnhatn dnhatn requested a review from ywelsch August 5, 2019 13:08
Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

LGTM

@dnhatn
Copy link
Member Author

dnhatn commented Aug 5, 2019

Thanks @ywelsch.

@dnhatn dnhatn merged commit c26f850 into elastic:master Aug 5, 2019
@dnhatn dnhatn deleted the should-flush branch August 5, 2019 17:55
dnhatn added a commit that referenced this pull request Aug 11, 2019
testShouldFlushAfterPeerRecovery was added #28350 to make sure the
flushing loop triggered by afterWriteOperation eventually terminates.
This test relies on the fact that we call afterWriteOperation after
making changes in translog. In #44756, we roll a new generation in
RecoveryTarget#finalizeRecovery but do not call afterWriteOperation.

Relates #28350
Relates #45073
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. >test Issues or PRs that are addressing/adding tests v7.4.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants