Record Force Merges in Live Commit Data #52694
Conversation
Prerequesite of elastic#52182. Record force merges in the live commit data so two shard states with the same sequence number that differ only in whether or not they have been force merged can be distinguished when creating snapshots.
original-brownbear
added
>non-issue
:Distributed Indexing/Engine
|
Pinging @elastic/es-distributed (:Distributed/Engine) |
original-brownbear
changed the title
| @@ -53,20 +55,36 @@ | |||
| private boolean onlyExpungeDeletes = Defaults.ONLY_EXPUNGE_DELETES; | |||
| private boolean flush = Defaults.FLUSH; | |||
|
|
|||
| private static final Version FORCE_MERGE_UUID_VERSION = Version.V_8_0_0; | |||
|
|
|||
| public static final String FORCE_MERGE_UUID_NA_VALUE = "_na_"; | |||
There was a problem hiding this comment.
I figured it's a nice thing to just used _na_ instead of null to keep the serialization BwC simple.
|
Thanks @dnhatn for pointing out the fact that we need to tie the uuid to the force merge request. I implemented that now including an integration test that proves that it works. Let know what you think :) |
server/src/main/java/org/elasticsearch/index/engine/Engine.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/engine/Engine.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/engine/Engine.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java
Outdated
Show resolved
Hide resolved
|
Thanks @dnhatn ! all addressed I think :) |
|
@ywelsch are you good with this one as well? :) |
| commitData.put(Translog.TRANSLOG_UUID_KEY, translog.getTranslogUUID()); | ||
| commitData.put(SequenceNumbers.LOCAL_CHECKPOINT_KEY, Long.toString(localCheckpoint)); | ||
| commitData.put(SequenceNumbers.MAX_SEQ_NO, Long.toString(localCheckpointTracker.getMaxSeqNo())); | ||
| commitData.put(MAX_UNSAFE_AUTO_ID_TIMESTAMP_COMMIT_ID, Long.toString(maxUnsafeAutoIdTimestamp.get())); | ||
| commitData.put(HISTORY_UUID_KEY, historyUUID); | ||
| final String currentForceMergeUUID = forceMergeUUID; | ||
| if (currentForceMergeUUID != null) { |
There was a problem hiding this comment.
should we also check the NA value and not write it out in that case? Otherwise it will be up to all readers to do the right thing? I find the NA / null handling a bit confusing here, and wonder if we should just have one of these. My preference would be to have it Nullable everywhere, and not introduce another "null" value that is NA.
|
Jenkins run elasticsearch-ci/1 (some random dep. download issue) |
| @@ -158,7 +167,7 @@ public void writeTo(StreamOutput out) throws IOException { | |||
| out.writeBoolean(onlyExpungeDeletes); | |||
| out.writeBoolean(flush); | |||
| if (out.getVersion().onOrAfter(FORCE_MERGE_UUID_VERSION)) { | |||
| out.writeString(forceMergeUUID); | |||
| out.writeString(forceMergeUUID == null ? FORCE_MERGE_UUID_NA_VALUE : forceMergeUUID); | |||
There was a problem hiding this comment.
I would just use readOptionalString and writeOptionalString, no need for the extra FORCE_MERGE_UUID_NA_VALUE. It simplifies the code and no need for the extra value. Sending an extra byte on the wire does not hurt either.
| @@ -558,6 +566,11 @@ public String getHistoryUUID() { | |||
| return historyUUID; | |||
| } | |||
|
|
|||
| /** returns the force merge uuid for the engine */ | |||
|
Thanks Nhat + Yannick! |
* Record Force Merges in live commit data Prerequisite of elastic#52182. Record force merges in the live commit data so two shard states with the same sequence number that differ only in whether or not they have been force merged can be distinguished when creating snapshots.
* Record Force Merges in live commit data Prerequisite of #52182. Record force merges in the live commit data so two shard states with the same sequence number that differ only in whether or not they have been force merged can be distinguished when creating snapshots.
Prerequisite of #52182. Record force merges in the live commit data
so two shard states with the same sequence number that differ only in whether
or not they have been force merged can be distinguished when creating snapshots.