-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Restoring from snapshot should force generation of a new history uuid #26694
Changes from 5 commits
cd9ffba
ab13a76
da5f848
171aa45
efdf3f6
f5cfbdc
3c363f0
5c330b4
431ceef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2080,10 +2080,24 @@ private DocumentMapperForType docMapper(String type) { | |
|
||
private EngineConfig newEngineConfig(EngineConfig.OpenMode openMode) { | ||
Sort indexSort = indexSortSupplier.get(); | ||
final boolean forceNewHistoryUUID; | ||
switch (shardRouting.recoverySource().getType()) { | ||
case EMPTY_STORE: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. EMPTY_STORE should force a new history uuid (see also my comment here).
Note that this leaves the case of allocating a stale primary, which should also force a fresh history. Here, we have no way at the moment though to detect that based on the recovery source object. For this, if the recovery source is EXISTING_STORE, we can compare the on-disk allocation id ( I'm ok if you do this in a follow-up, but I still think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Re forcing a stale allocations - good catch. As you say, that's a more complicated one indeed. I think we should do it as a follow up and discuss possible approaches. I'm thinking about a new recovery source. I think this is an edge case we should be explicit about. |
||
case EXISTING_STORE: | ||
case PEER: | ||
forceNewHistoryUUID = false; | ||
break; | ||
case SNAPSHOT: | ||
case LOCAL_SHARDS: | ||
forceNewHistoryUUID = true; | ||
break; | ||
default: | ||
throw new AssertionError("unknown recovery type: [" + shardRouting.recoverySource().getType() + "]"); | ||
} | ||
return new EngineConfig(openMode, shardId, shardRouting.allocationId().getId(), | ||
threadPool, indexSettings, warmer, store, indexSettings.getMergePolicy(), | ||
mapperService.indexAnalyzer(), similarityService.similarity(mapperService), codecService, shardEventListener, | ||
indexCache.query(), cachingPolicy, translogConfig, | ||
indexCache.query(), cachingPolicy, forceNewHistoryUUID, translogConfig, | ||
IndexingMemoryController.SHARD_INACTIVE_TIME_SETTING.get(indexSettings.getSettings()), | ||
Arrays.asList(refreshListeners, new RefreshMetricUpdater(refreshMetric)), indexSort, | ||
this::runTranslogRecovery); | ||
|
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.
commitIndexWriter still has a check
if (historyUUID != null)
. Is that one obsolete now?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.
good catch. fixing.