Skip to content

Remove sync flush logic in Engine#51450

Merged
dnhatn merged 6 commits intoelastic:masterfrom
dnhatn:synced-flush-engine
Jan 27, 2020
Merged

Remove sync flush logic in Engine#51450
dnhatn merged 6 commits intoelastic:masterfrom
dnhatn:synced-flush-engine

Conversation

@dnhatn
Copy link
Copy Markdown
Member

@dnhatn dnhatn commented Jan 24, 2020

This change removes the sync-flush logic in the InternalEngine as we no longer issue new syncIds in 8.0. Sadly, we need to keep the renewal logic as we should re-issue the syncId if users trigger a force-merge.

Relates #50776

@dnhatn dnhatn added >non-issue :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. v8.0.0 labels Jan 24, 2020
@dnhatn dnhatn requested a review from ywelsch January 24, 2020 20:56
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (:Distributed/Engine)

@dnhatn dnhatn mentioned this pull request Jan 24, 2020
7 tasks
@dnhatn
Copy link
Copy Markdown
Member Author

dnhatn commented Jan 26, 2020

I think it's fine not to renew sync_id after force merge in 8.0. If the replica is online, then sync_id is not needed as that replica should have established PRRL. And if the replica is offline, copying segment files is perhaps more beneficial than operation-based recovery. I've removed the renewal logic in cd97e6c.

Copy link
Copy Markdown
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
Copy Markdown
Member Author

dnhatn commented Jan 27, 2020

Thanks Yannick.

@dnhatn dnhatn merged commit 0c87892 into elastic:master Jan 27, 2020
@dnhatn dnhatn deleted the synced-flush-engine branch January 27, 2020 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Engine Anything around managing Lucene and the Translog in an open shard. >non-issue v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants