-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Push primary term to replication tracker #38044
Push primary term to replication tracker #38044
Conversation
This commit pushes the primary term into the replication tracker. This is a precursor to using the primary term to resolving ordering problems for retention leases. Namely, it can be that out-of-order retention lease sync requests arrive on a replica. To resolve this, we need a tuple of (primary term, version). For this to be, the primary term needs to be accessible in the replication tracker. As the primary term is part of the replication group anyway, this change conceptually makes sense.
Pinging @elastic/es-distributed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the primary term is part of the replication group anyway, this change conceptually makes sense.
This makes sense to me too. I thought we are going to provide a PrimaryTermSupplier to ReplicationTracker and I find that having pendingPrimaryTerm
and operationPrimaryTerm
at the same place is a bit easier to understand than two places.
This change looks good to me, but I prefer to wait for @ywelsch on this.
* | ||
* @return the primary term | ||
*/ | ||
public long getOperationPrimaryTerm() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please move getter and setter further down. I understand this is your preferred coding style. However, it conflicts with the coding style we have adapted for all these classes and don't think this is the right time and place to switch things up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed 72e2e7b.
operationPrimaryTerm = pendingPrimaryTerm; | ||
final long primaryTerm = indexSettings.getIndexMetaData().primaryTerm(shardId.id()); | ||
this.pendingPrimaryTerm = primaryTerm; | ||
replicationTracker.setOperationPrimaryTerm(primaryTerm); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we set this in the constructor of ReplicationTracker? Then we would always have a proper operation term in ReplicationTracker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed 72e2e7b.
* master: Remove types from watcher docs (elastic#38002) Add test coverage for Painless general casting of boolean and Boolean (elastic#37780) Fixed test bug, lastFollowTime is null if there are no follower indices. Add ECS schema for user-agent ingest processor (elastic#37727) (elastic#37984) Extract TransportRequestDeduplication from ShardStateAction (elastic#37870) Expose retention leases in shard stats (elastic#37991)
@elasticmachine run elasticsearch-ci/1 |
1 similar comment
@elasticmachine run elasticsearch-ci/1 |
…r-primary-term * elastic/master: Mute failing date index name processor test Reenable BWC testing after retention lease stats (elastic#38062) Move watcher to use seq# and primary term for concurrency control (elastic#37977) ILM setPriority corrections for a 0 value (elastic#38001) Temporarily disable BWC for retention lease stats (elastic#38049) Skip Shrink when numberOfShards not changed (elastic#37953) Add dispatching to `HandledTransportAction` (elastic#38050) Update httpclient for JDK 11 TLS engine (elastic#37994) Reduce flaxiness of ccr recovery timeouts test (elastic#38035) Fix ILM status to allow unknown fields (elastic#38043) Fix ILM Lifecycle Policy to allow unknown fields (elastic#38041) Update verify repository to allow unknown fields (elastic#37619) [ML] Datafeed deprecation checks (elastic#38026) Deprecate minimum_master_nodes (elastic#37868)
This commit pushes the primary term into the replication tracker. This is a precursor to using the primary term to resolving ordering problems for retention leases. Namely, it can be that out-of-order retention lease sync requests arrive on a replica. To resolve this, we need a tuple of (primary term, version). For this to be, the primary term needs to be accessible in the replication tracker. As the primary term is part of the replication group anyway, this change conceptually makes sense.
* master: (100 commits) Push primary term to replication tracker (elastic#38044) Introduce ability to minimize round-trips in CCS (elastic#37828) Don't Assert Ack on when Publish Timeout is 0 in Test (elastic#38077) Reduce object creation in Rounding class (elastic#38061) Treat put-mapping calls with `_doc` as a top-level key as typed calls. (elastic#38032) Fix test bug when testing the merging of mappings and templates. (elastic#38021) spelling: java script -- not JavaScript (elastic#37057) Enable SSL in reindex with security QA tests (elastic#37600) Disable BWC tests during backport (elastic#38074) SQL: Added SSL configuration options tests (elastic#37875) Minor fixes in the release notes script. (elastic#37967) Fix typo in docs. (elastic#38018) Update Lucene repo for 7.0.0-alpha2 (elastic#37985) Fix size of rolling-upgrade bootstrap config (elastic#38031) fix DateIndexNameProcessorTests offset pattern (elastic#38069) Speed up converting of temporal accessor to zoned date time (elastic#37915) Work around JDK8 timezone bug in tests (elastic#37968) Correct arg names when update mapping/settings from leader (elastic#38063) Introduce ssl settings to reindex from remote (elastic#37527) Mute testRetentionLeasesSyncOnExpiration ...
This commit pushes the primary term into the replication tracker. This is a precursor to using the primary term to resolving ordering problems for retention leases. Namely, it can be that out-of-order retention lease sync requests arrive on a replica. To resolve this, we need a tuple of (primary term, version). For this to be, the primary term needs to be accessible in the replication tracker. As the primary term is part of the replication group anyway, this change conceptually makes sense.
Relates #37951
Relates #38036