Remove Needless Context Switch From Snapshot Finalization (#56871) (#59443)

No need to do any switch to the `SNAPSHOT` pool here, the blob store
repo handles all its writes async on the `SNAPSHOT` pool so we're just
needlessly context-switching to enqueue those tasks there.
Also cleaned up the source only repository (the only override to `finalizeSnapshot`)
to make it clear that no IO is happening there and we don't need to run it on the
`SNAPSHOT` pool either.
This commit is contained in:
Armin Braun 2020-07-13 20:11:07 +02:00 committed by GitHub
parent 31be3a3645
commit bde92fc5fc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 47 additions and 50 deletions

View File

@ -1074,10 +1074,7 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus
removeSnapshotFromClusterState(entry.snapshot(), new SnapshotException(snapshot, "Aborted on initialization"), null); removeSnapshotFromClusterState(entry.snapshot(), new SnapshotException(snapshot, "Aborted on initialization"), null);
return; return;
} }
threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(new AbstractRunnable() { try {
@Override
protected void doRun() {
final Repository repository = repositoriesService.repository(snapshot.getRepository());
final String failure = entry.failure(); final String failure = entry.failure();
logger.trace("[{}] finalizing snapshot in repository, state: [{}], failure[{}]", snapshot, entry.state(), failure); logger.trace("[{}] finalizing snapshot in repository, state: [{}], failure[{}]", snapshot, entry.state(), failure);
ArrayList<SnapshotShardFailure> shardFailures = new ArrayList<>(); ArrayList<SnapshotShardFailure> shardFailures = new ArrayList<>();
@ -1094,7 +1091,7 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus
} }
} }
final ShardGenerations shardGenerations = buildGenerations(entry, metadata); final ShardGenerations shardGenerations = buildGenerations(entry, metadata);
repository.finalizeSnapshot( repositoriesService.repository(snapshot.getRepository()).finalizeSnapshot(
snapshot.getSnapshotId(), snapshot.getSnapshotId(),
shardGenerations, shardGenerations,
entry.startTime(), entry.startTime(),
@ -1119,11 +1116,13 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus
} }
endingSnapshots.remove(snapshot); endingSnapshots.remove(snapshot);
logger.info("snapshot [{}] completed with state [{}]", snapshot, result.v2().state()); logger.info("snapshot [{}] completed with state [{}]", snapshot, result.v2().state());
}, this::onFailure)); }, e -> handleFinalizationFailure(e, entry)));
} catch (Exception e) {
handleFinalizationFailure(e, entry);
}
} }
@Override private void handleFinalizationFailure(Exception e, SnapshotsInProgress.Entry entry) {
public void onFailure(final Exception e) {
Snapshot snapshot = entry.snapshot(); Snapshot snapshot = entry.snapshot();
if (ExceptionsHelper.unwrap(e, NotMasterException.class, FailedToCommitClusterStateException.class) != null) { if (ExceptionsHelper.unwrap(e, NotMasterException.class, FailedToCommitClusterStateException.class) != null) {
// Failure due to not being master any more, don't try to remove snapshot from cluster state the next master // Failure due to not being master any more, don't try to remove snapshot from cluster state the next master
@ -1137,8 +1136,6 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus
removeSnapshotFromClusterState(snapshot, e, null); removeSnapshotFromClusterState(snapshot, e, null);
} }
} }
});
}
private static ClusterState stateWithoutSnapshot(ClusterState state, Snapshot snapshot) { private static ClusterState stateWithoutSnapshot(ClusterState state, Snapshot snapshot) {
SnapshotsInProgress snapshots = state.custom(SnapshotsInProgress.TYPE); SnapshotsInProgress snapshots = state.custom(SnapshotsInProgress.TYPE);