Bootstrap a new history_uuid when force allocating a stale primary #33432
Conversation
This commit ensures that we bootstrap a new history_uuid when force allocating a stale primary. A stale primary should never be the source of an operation-based recovery to another shard which exists before the forced-allocation.
dnhatn
added
>enhancement
:Distributed/Recovery
|
Pinging @elastic/es-distributed |
| @@ -107,25 +112,68 @@ public int hashCode() { | |||
| } | |||
|
|
|||
| /** | |||
| * recovery from an existing on-disk store or a fresh copy | |||
| * Recovery from a refresh copy | |||
|
|
||
| Set<String> newHistoryUUIds = Arrays.stream(client().admin().indices().prepareStats("test").clear().get().getShards()) | ||
| .map(shard -> shard.getCommitStats().getUserData().get(Engine.HISTORY_UUID_KEY)).collect(Collectors.toSet()); | ||
| assertThat(newHistoryUUIds, everyItem(not(isIn(historyUUIDs)))); |
There was a problem hiding this comment.
mabye assert that all new history uuids are the same, just for fun?
|
Thanks @bleskes for reviewing. |
* master: CORE: Make Pattern Exclusion Work with Aliases (elastic#33518) Reverse logic for CCR license checks (elastic#33549) Add latch countdown on failure in CCR license tests (elastic#33548) HLRC: Add put stored script support to high-level rest client (elastic#31323) Create temporary directory if needed in CCR test Add license checks for auto-follow implementation (elastic#33496) Bootstrap a new history_uuid when force allocating a stale primary (elastic#33432) INGEST: Remove Outdated TODOs (elastic#33458) Logging: Clean up skipping test Logging: Skip test if it'd fail CRUD: AwaitsFix entire wait_for_refresh close test Painless: Add Imported Static Method (elastic#33440)
|
Note that if the node crashes between shard initialization and shard recovery, the allocation id (both in the in-sync set as well as on the node's local shard copy) will be adjusted but the history UUID will not, which is unsafe to do. I would have preferred to put the history UUID into the index metadata (on a per-shard level, similar to in-sync-allocation ids), and then realigned the shards history uuid with the index metadata one, avoiding the above problem. |
|
@ywelsch that's a good insight. If we put the history uuid in the index meta data, I think we need to work out the exact semantics of it and how it correlates to the lucene index version (or may we want to move to store it in the state file together with the history uuid). I think we can also consider another alternative - only add the allocation id of the stale copy to the insync set once it has been recovered (potentially clearing the in sync set when the command is run). I also wonder if we should do the same for recovery from snapshot (if I read the code correctly, I think we have the same problem) |
I would be ok to activate it only after it has been recovered, but I would also like for the allocate stale primary command to immediately take effect (in terms of resetting the in-sync set). An empty in-sync set has currently the semantics of "allocate a fresh shard copy" (i.e. not an existing one), so that's not an option. We could just make it a singleton set with a random allocation id or keep the set as is as long as the newly allocated primary has not been activated. |
Agreed.
I first thought this was a hack, but then I thought that maybe we should set it to |
works for me. |
… after recovery is done Relates to elastic#33432
removes fake allocation id after recovery is done Relates to #33432
…4140) removes fake allocation id after recovery is done Relates to elastic#33432
This commit ensures that we bootstrap a new history_uuid when force
allocating a stale primary. A stale primary should never be the source
of an operation-based recovery to another shard which exists before the
forced-allocation.
Closes #26712