Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Put a fake allocation id on allocate stale primary command #34140

Merged
merged 31 commits into from Nov 7, 2018
Merged

Put a fake allocation id on allocate stale primary command #34140

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
62ce29b
Put a fake allocation id on allocate stale primary command; remove it…
Sep 27, 2018
3185235
added a test case to spot the problem if allocation id is adjusted be…
Sep 29, 2018
2023a04
Merge remote-tracking branch 'remotes/origin/master' into forced_allo…
Sep 30, 2018
02e5e05
simplify test case to spot the problem if allocation id is adjusted b…
Oct 1, 2018
8b697ef
extended testAllocateCommand: added AllocateStalePrimaryAllocationCom…
Oct 2, 2018
e331b1f
enforced check that there is only one allocation id on adding/removin…
Oct 2, 2018
d3df30c
fix BalanceConfigurationTests: using TestGatewayAllocator instead of …
Oct 3, 2018
fbdf6d7
enforce check for fake_allocation: for existing store it could be onl…
Oct 3, 2018
f0d71c5
Merge remote-tracking branch 'remotes/origin/master' into forced_allo…
Oct 3, 2018
6b3cfaa
Merge remote-tracking branch 'remotes/origin/master' into forced_allo…
Oct 5, 2018
8f909af
inline addAllocationId; add assert on initializingShard.allocationId(…
Oct 25, 2018
3554615
java doc for FORCED_ALLOCATION_ID; use front and back underscore for …
Oct 25, 2018
70a8379
S/R deserves its own allocation id
Oct 26, 2018
9ac69bd
fix assert on initializingShard.allocationId() equal to startedShard.…
Oct 29, 2018
46183b1
fixed index routing table validation message for fake allocation id case
Oct 29, 2018
e14d094
extract AllocateStalePrimaryCommand to its own test method
Oct 29, 2018
72a24a4
simplify AllocationIdIT
Oct 29, 2018
e06ba85
simplify AllocationIdIT; don't restart master
Oct 30, 2018
a1440e3
Merge remote-tracking branch 'remotes/origin/master' into forced_allo…
Oct 30, 2018
e741bf5
after merge compilation fix
Oct 30, 2018
30dffa6
Merge branch 'origin/master' into forced_allocation_allocation_id
Nov 5, 2018
99dc666
move updates earlier and reuse it
Nov 5, 2018
e8e3925
drop getNodeIdByName as AllocateStalePrimaryAllocationCommand can use…
Nov 5, 2018
3f02dce
dropped unnecessary settings
Nov 5, 2018
dd2fe3b
handle single historyUUID
Nov 5, 2018
1d8899e
reuse ESIntegTestCase.createIndex
Nov 5, 2018
d141d9e
drop redundant assertBusy
Nov 5, 2018
e4e45c7
comment on the reason behind the test
Nov 5, 2018
2b6d535
S&R leftover
Nov 5, 2018
07a795b
drop useless open index (index is still opened)
Nov 5, 2018
78dfaa3
Merge remote-tracking branch 'remotes/origin/master' into forced_allo…
Nov 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion server/src/main/java/org/elasticsearch/cluster/routing/IndexRoutingTable.java
View file
Open in desktop
Expand Up @@ -141,7 +141,9 @@ boolean validate(MetaData metaData) {

if (shardRouting.primary() && shardRouting.initializing() &&
shardRouting.recoverySource().getType() == RecoverySource.Type.EXISTING_STORE &&
inSyncAllocationIds.contains(shardRouting.allocationId().getId()) == false)
inSyncAllocationIds.contains(shardRouting.allocationId().getId()) == false &&
(inSyncAllocationIds.contains(RecoverySource.ExistingStoreRecoverySource.FORCED_ALLOCATION_ID) == false
vladimirdolzhenko marked this conversation as resolved.
Show resolved Hide resolved
|| inSyncAllocationIds.size() != 1))
throw new IllegalStateException("a primary shard routing " + shardRouting + " is a primary that is recovering from " +
"a known allocation id but has no corresponding entry in the in-sync " +
"allocation set " + inSyncAllocationIds);
Expand Down
1 change: 1 addition & 0 deletions server/src/main/java/org/elasticsearch/cluster/routing/RecoverySource.java
View file
Open in desktop
Expand Up @@ -132,6 +132,7 @@ public String toString() {
* Recovery from an existing on-disk store
*/
public static final class ExistingStoreRecoverySource extends RecoverySource {
public static final String FORCED_ALLOCATION_ID = "_forced_allocation";
vladimirdolzhenko marked this conversation as resolved.
Show resolved Hide resolved
public static final ExistingStoreRecoverySource INSTANCE = new ExistingStoreRecoverySource(false);
public static final ExistingStoreRecoverySource FORCE_STALE_PRIMARY_INSTANCE = new ExistingStoreRecoverySource(true);

Expand Down
24 changes: 21 additions & 3 deletions server/src/main/java/org/elasticsearch/cluster/routing/allocation/IndexMetaDataUpdater.java
View file
Open in desktop
Expand Up @@ -69,6 +69,13 @@ public void shardInitialized(ShardRouting unassignedShard, ShardRouting initiali
@Override
public void shardStarted(ShardRouting initializingShard, ShardRouting startedShard) {
addAllocationId(startedShard);
vladimirdolzhenko marked this conversation as resolved.
Show resolved Hide resolved
if (startedShard.primary()
// started shard has to have null recoverySource; have to pick up recoverySource from its initializing state
&& (initializingShard.recoverySource() == RecoverySource.ExistingStoreRecoverySource.FORCE_STALE_PRIMARY_INSTANCE
|| initializingShard.recoverySource() instanceof RecoverySource.SnapshotRecoverySource)) {
vladimirdolzhenko marked this conversation as resolved.
Show resolved Hide resolved
Updates updates = changes(startedShard.shardId());
vladimirdolzhenko marked this conversation as resolved.
Show resolved Hide resolved
vladimirdolzhenko marked this conversation as resolved.
Show resolved Hide resolved
updates.removedAllocationIds.add(RecoverySource.ExistingStoreRecoverySource.FORCED_ALLOCATION_ID);
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 make sure this is the only one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in e331b1f I added assert check in another place where we have old and new allocation ids

}
}

@Override
Expand Down Expand Up @@ -140,11 +147,13 @@ private IndexMetaData.Builder updateInSyncAllocations(RoutingTable newRoutingTab
Set<String> oldInSyncAllocationIds = oldIndexMetaData.inSyncAllocationIds(shardId.id());

// check if we have been force-initializing an empty primary or a stale primary
// the same is applicable for snapshot/restore as it can be considered as allocating a stale primary.
vladimirdolzhenko marked this conversation as resolved.
Show resolved Hide resolved
if (updates.initializedPrimary != null && oldInSyncAllocationIds.isEmpty() == false &&
oldInSyncAllocationIds.contains(updates.initializedPrimary.allocationId().getId()) == false) {
// we're not reusing an existing in-sync allocation id to initialize a primary, which means that we're either force-allocating
// an empty or a stale primary (see AllocateEmptyPrimaryAllocationCommand or AllocateStalePrimaryAllocationCommand).
RecoverySource.Type recoverySourceType = updates.initializedPrimary.recoverySource().getType();
RecoverySource recoverySource = updates.initializedPrimary.recoverySource();
RecoverySource.Type recoverySourceType = recoverySource.getType();
boolean emptyPrimary = recoverySourceType == RecoverySource.Type.EMPTY_STORE;
assert updates.addedAllocationIds.isEmpty() : (emptyPrimary ? "empty" : "stale") +
" primary is not force-initialized in same allocation round where shards are started";
Expand All @@ -156,16 +165,24 @@ private IndexMetaData.Builder updateInSyncAllocations(RoutingTable newRoutingTab
// forcing an empty primary resets the in-sync allocations to the empty set (ShardRouting.allocatedPostIndexCreate)
indexMetaDataBuilder.putInSyncAllocationIds(shardId.id(), Collections.emptySet());
} else {
assert recoverySource == RecoverySource.ExistingStoreRecoverySource.FORCE_STALE_PRIMARY_INSTANCE
|| recoverySource instanceof RecoverySource.SnapshotRecoverySource
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

snapshot/restore behaves like allocating a stale primary ( confirmed by @ywelsch )

Enforced check and extended comment at fbdf6d7

RecoverySource.ExistingStoreRecoverySource was introduced due to misleading by BalanceConfigurationTests test failure (fixed at d3df30c):

it used oversimplified allocator and it leads to inconsistent behaviour allocation decision (and after that inconsistent cluster state). On removing nodes some shards could be completely unassigned, in such cases PrimaryShardAllocator makes decision to to allocate that shard as NO_VALID_SHARD_COPY, while used in test NoopGatewayAllocator ignores that.

: recoverySource;
// forcing a stale primary resets the in-sync allocations to the singleton set with the stale id
indexMetaDataBuilder.putInSyncAllocationIds(shardId.id(),
Collections.singleton(updates.initializedPrimary.allocationId().getId()));
Collections.singleton(RecoverySource.ExistingStoreRecoverySource.FORCED_ALLOCATION_ID));
}
} else {
// standard path for updating in-sync ids
Set<String> inSyncAllocationIds = new HashSet<>(oldInSyncAllocationIds);
inSyncAllocationIds.addAll(updates.addedAllocationIds);
inSyncAllocationIds.removeAll(updates.removedAllocationIds);

assert oldInSyncAllocationIds.contains(RecoverySource.ExistingStoreRecoverySource.FORCED_ALLOCATION_ID) == false
|| inSyncAllocationIds.size() == 1
vladimirdolzhenko marked this conversation as resolved.
Show resolved Hide resolved
&& inSyncAllocationIds.contains(RecoverySource.ExistingStoreRecoverySource.FORCED_ALLOCATION_ID) == false :
"fake allocation id has to be removed, inSyncAllocationIds:" + inSyncAllocationIds;

// Prevent set of inSyncAllocationIds to grow unboundedly. This can happen for example if we don't write to a primary
// but repeatedly shut down nodes that have active replicas.
// We use number_of_replicas + 1 (= possible active shard copies) to bound the inSyncAllocationIds set
Expand Down Expand Up @@ -291,7 +308,8 @@ void removeAllocationId(ShardRouting shardRouting) {
* Add allocation id of this shard to the set of in-sync shard copies
*/
private void addAllocationId(ShardRouting shardRouting) {
changes(shardRouting.shardId()).addedAllocationIds.add(shardRouting.allocationId().getId());
final Updates changes = changes(shardRouting.shardId());
changes.addedAllocationIds.add(shardRouting.allocationId().getId());
}

/**
Expand Down