TEST: Capture replication targets when replication group ready (#34407)

Today, WriteReplicationAction uses a set of replication targets directly
from the primary shard of ReplicationGroup. It should be fine except
when we add/remove or promote a shard while a write action is executing.
We have encountered these two issues:

1. Replicas are not found in the replication targets. This happens
because we remove replicas but the WriteReplicationAction still uses the
old replication targets which include the removed replicas.

2. Access ReplicationGroup from a primary shard which hasn't activated
the primary-mode yet. This is because we won't activate the primary-mode
for a promoting shard after bumping the primary term which is executed
asynchronously.

This commit captures the replication targets when the replication group
is ready and continue using those targets until we re-compute the new
targets after the group is changed.

Closes #33457
This commit is contained in:
Nhat Nguyen 2018-10-17 17:37:52 -04:00 committed by GitHub
parent a45626deb5
commit eb36f10394
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 42 additions and 16 deletions

View File

@ -164,6 +164,8 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
private final AtomicInteger replicaId = new AtomicInteger(); private final AtomicInteger replicaId = new AtomicInteger();
private final AtomicInteger docId = new AtomicInteger(); private final AtomicInteger docId = new AtomicInteger();
boolean closed = false; boolean closed = false;
private ReplicationTargets replicationTargets;
private final PrimaryReplicaSyncer primaryReplicaSyncer = new PrimaryReplicaSyncer(Settings.EMPTY, private final PrimaryReplicaSyncer primaryReplicaSyncer = new PrimaryReplicaSyncer(Settings.EMPTY,
new TaskManager(Settings.EMPTY, threadPool, Collections.emptySet()), new TaskManager(Settings.EMPTY, threadPool, Collections.emptySet()),
(request, parentTask, primaryAllocationId, primaryTerm, listener) -> { (request, parentTask, primaryAllocationId, primaryTerm, listener) -> {
@ -277,6 +279,7 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
for (final IndexShard replica : replicas) { for (final IndexShard replica : replicas) {
recoverReplica(replica); recoverReplica(replica);
} }
computeReplicationTargets();
} }
public IndexShard addReplica() throws IOException { public IndexShard addReplica() throws IOException {
@ -320,8 +323,9 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
*/ */
public Future<PrimaryReplicaSyncer.ResyncTask> promoteReplicaToPrimary(IndexShard replica) throws IOException { public Future<PrimaryReplicaSyncer.ResyncTask> promoteReplicaToPrimary(IndexShard replica) throws IOException {
PlainActionFuture<PrimaryReplicaSyncer.ResyncTask> fut = new PlainActionFuture<>(); PlainActionFuture<PrimaryReplicaSyncer.ResyncTask> fut = new PlainActionFuture<>();
promoteReplicaToPrimary(replica, promoteReplicaToPrimary(replica, (shard, listener) -> {
(shard, listener) -> primaryReplicaSyncer.resync(shard, computeReplicationTargets();
primaryReplicaSyncer.resync(shard,
new ActionListener<PrimaryReplicaSyncer.ResyncTask>() { new ActionListener<PrimaryReplicaSyncer.ResyncTask>() {
@Override @Override
public void onResponse(PrimaryReplicaSyncer.ResyncTask resyncTask) { public void onResponse(PrimaryReplicaSyncer.ResyncTask resyncTask) {
@ -334,7 +338,8 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
listener.onFailure(e); listener.onFailure(e);
fut.onFailure(e); fut.onFailure(e);
} }
})); });
});
return fut; return fut;
} }
@ -370,6 +375,7 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
final boolean removed = replicas.remove(replica); final boolean removed = replicas.remove(replica);
if (removed) { if (removed) {
updateAllocationIDsOnPrimary(); updateAllocationIDsOnPrimary();
computeReplicationTargets();
} }
return removed; return removed;
} }
@ -392,6 +398,7 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
ESIndexLevelReplicationTestCase.this.recoverUnstartedReplica(replica, primary, targetSupplier, markAsRecovering, inSyncIds, ESIndexLevelReplicationTestCase.this.recoverUnstartedReplica(replica, primary, targetSupplier, markAsRecovering, inSyncIds,
routingTable); routingTable);
ESIndexLevelReplicationTestCase.this.startReplicaAfterRecovery(replica, primary, inSyncIds, routingTable); ESIndexLevelReplicationTestCase.this.startReplicaAfterRecovery(replica, primary, inSyncIds, routingTable);
computeReplicationTargets();
} }
public synchronized DiscoveryNode getPrimaryNode() { public synchronized DiscoveryNode getPrimaryNode() {
@ -468,6 +475,7 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
public synchronized void reinitPrimaryShard() throws IOException { public synchronized void reinitPrimaryShard() throws IOException {
primary = reinitShard(primary); primary = reinitShard(primary);
computeReplicationTargets();
} }
public void syncGlobalCheckpoint() { public void syncGlobalCheckpoint() {
@ -486,6 +494,24 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
currentClusterStateVersion.incrementAndGet(), currentClusterStateVersion.incrementAndGet(),
activeIds(), routingTable(Function.identity()), Collections.emptySet()); activeIds(), routingTable(Function.identity()), Collections.emptySet());
} }
private synchronized void computeReplicationTargets() {
this.replicationTargets = new ReplicationTargets(primary, replicas);
}
private synchronized ReplicationTargets getReplicationTargets() {
return replicationTargets;
}
}
static final class ReplicationTargets {
final IndexShard primary;
final List<IndexShard> replicas;
ReplicationTargets(IndexShard primary, List<IndexShard> replicas) {
this.primary = primary;
this.replicas = Collections.unmodifiableList(replicas);
}
} }
protected abstract class ReplicationAction<Request extends ReplicationRequest<Request>, protected abstract class ReplicationAction<Request extends ReplicationRequest<Request>,
@ -493,13 +519,13 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
Response extends ReplicationResponse> { Response extends ReplicationResponse> {
private final Request request; private final Request request;
private ActionListener<Response> listener; private ActionListener<Response> listener;
private final ReplicationGroup replicationGroup; private final ReplicationTargets replicationTargets;
private final String opType; private final String opType;
protected ReplicationAction(Request request, ActionListener<Response> listener, ReplicationGroup group, String opType) { protected ReplicationAction(Request request, ActionListener<Response> listener, ReplicationGroup group, String opType) {
this.request = request; this.request = request;
this.listener = listener; this.listener = listener;
this.replicationGroup = group; this.replicationTargets = group.getReplicationTargets();
this.opType = opType; this.opType = opType;
} }
@ -523,7 +549,7 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
} }
IndexShard getPrimaryShard() { IndexShard getPrimaryShard() {
return replicationGroup.primary; return replicationTargets.primary;
} }
protected abstract PrimaryResult performOnPrimary(IndexShard primary, Request request) throws Exception; protected abstract PrimaryResult performOnPrimary(IndexShard primary, Request request) throws Exception;
@ -534,7 +560,7 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
@Override @Override
public ShardRouting routingEntry() { public ShardRouting routingEntry() {
return replicationGroup.primary.routingEntry(); return getPrimaryShard().routingEntry();
} }
@Override @Override
@ -544,37 +570,37 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
@Override @Override
public PrimaryResult perform(Request request) throws Exception { public PrimaryResult perform(Request request) throws Exception {
return performOnPrimary(replicationGroup.primary, request); return performOnPrimary(getPrimaryShard(), request);
} }
@Override @Override
public void updateLocalCheckpointForShard(String allocationId, long checkpoint) { public void updateLocalCheckpointForShard(String allocationId, long checkpoint) {
replicationGroup.getPrimary().updateLocalCheckpointForShard(allocationId, checkpoint); getPrimaryShard().updateLocalCheckpointForShard(allocationId, checkpoint);
} }
@Override @Override
public void updateGlobalCheckpointForShard(String allocationId, long globalCheckpoint) { public void updateGlobalCheckpointForShard(String allocationId, long globalCheckpoint) {
replicationGroup.getPrimary().updateGlobalCheckpointForShard(allocationId, globalCheckpoint); getPrimaryShard().updateGlobalCheckpointForShard(allocationId, globalCheckpoint);
} }
@Override @Override
public long localCheckpoint() { public long localCheckpoint() {
return replicationGroup.getPrimary().getLocalCheckpoint(); return getPrimaryShard().getLocalCheckpoint();
} }
@Override @Override
public long globalCheckpoint() { public long globalCheckpoint() {
return replicationGroup.getPrimary().getGlobalCheckpoint(); return getPrimaryShard().getGlobalCheckpoint();
} }
@Override @Override
public long maxSeqNoOfUpdatesOrDeletes() { public long maxSeqNoOfUpdatesOrDeletes() {
return replicationGroup.getPrimary().getMaxSeqNoOfUpdatesOrDeletes(); return getPrimaryShard().getMaxSeqNoOfUpdatesOrDeletes();
} }
@Override @Override
public org.elasticsearch.index.shard.ReplicationGroup getReplicationGroup() { public org.elasticsearch.index.shard.ReplicationGroup getReplicationGroup() {
return replicationGroup.primary.getReplicationGroup(); return getPrimaryShard().getReplicationGroup();
} }
} }
@ -588,10 +614,10 @@ public abstract class ESIndexLevelReplicationTestCase extends IndexShardTestCase
final long globalCheckpoint, final long globalCheckpoint,
final long maxSeqNoOfUpdatesOrDeletes, final long maxSeqNoOfUpdatesOrDeletes,
final ActionListener<ReplicationOperation.ReplicaResponse> listener) { final ActionListener<ReplicationOperation.ReplicaResponse> listener) {
IndexShard replica = replicationGroup.replicas.stream() IndexShard replica = replicationTargets.replicas.stream()
.filter(s -> replicaRouting.isSameAllocation(s.routingEntry())).findFirst().get(); .filter(s -> replicaRouting.isSameAllocation(s.routingEntry())).findFirst().get();
replica.acquireReplicaOperationPermit( replica.acquireReplicaOperationPermit(
replicationGroup.primary.getPendingPrimaryTerm(), getPrimaryShard().getPendingPrimaryTerm(),
globalCheckpoint, globalCheckpoint,
maxSeqNoOfUpdatesOrDeletes, maxSeqNoOfUpdatesOrDeletes,
new ActionListener<Releasable>() { new ActionListener<Releasable>() {