Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Support ES7.0, use primary_term and seq_no for job doc versioning #5

Merged

Conversation

zengyan-amazon
Copy link
Member

*Issue #4 *

Description of changes: This change updates the JobScheduler plugin to build with Elasticsearch 7.0, and make the change to use document primary_term and seq_no for job versioning, which is a breaking change introduced in ES7.0.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@zengyan-amazon zengyan-amazon added the version compatibility A change to add support for new versions of Elasticsearch from upstream label May 15, 2019
@zengyan-amazon zengyan-amazon self-assigned this May 15, 2019
@@ -188,16 +188,16 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re

ShardNodes shardNodes = new ShardNodes(localNodeId, shardNodeIds);
if (shardNodes.isOwningNode(index.id())) {
this.sweep(shardId, index.id(), result.getVersion(), index.source());
this.sweep(shardId, index.id(), result.getVersion(), index.source(), result.getTerm(), result.getSeqNo());
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we pass in the JobDocVersion instead of all 3 properties separately to reduce parameters

Copy link
Member Author

Choose a reason for hiding this comment

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

refactored to pass JobDocVersion

@@ -218,7 +219,7 @@ void sweep(ShardId shardId, String docId, Long version, BytesReference jobSource
this.sweptJobs.put(shardId, jobVersionMap);
}
jobVersionMap.compute(docId, (id, currentVersion) -> {
JobDocVersion newJobVersion = new JobDocVersion(primaryTerm, seqNo, version);
JobDocVersion newJobVersion = new JobDocVersion(version.getPrimaryTerm(), version.getSeqNo(), version.getVersion());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to do this? Just use the given version from parameter for comparison?

Copy link
Member Author

Choose a reason for hiding this comment

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

updated, fixed this silly code

Copy link
Contributor

@dbbaughe dbbaughe left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@zengyan-amazon zengyan-amazon merged commit 066bc63 into opendistro-for-elasticsearch:master May 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
version compatibility A change to add support for new versions of Elasticsearch from upstream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants