Fixes inefficient loading of snapshot repository data (#24510)
This commit fixes inefficient (worst case exponential) loading of snapshot repository data when checking for incompatible snapshots, that was introduced in #22267. When getting snapshot information, getRepositoryData() was called on every snapshot, so if there are a large number of snapshots in the repository and _all snapshots were requested, the performance degraded exponentially. This commit fixes the issue by only calling getRepositoryData once and using the data from it in all subsequent calls to get snapshot information. Closes #24509
This commit is contained in:
parent
bd3717a0f8
commit
fec1802e2f
|
@ -30,6 +30,7 @@ import org.elasticsearch.cluster.service.ClusterService;
|
||||||
import org.elasticsearch.common.inject.Inject;
|
import org.elasticsearch.common.inject.Inject;
|
||||||
import org.elasticsearch.common.regex.Regex;
|
import org.elasticsearch.common.regex.Regex;
|
||||||
import org.elasticsearch.common.settings.Settings;
|
import org.elasticsearch.common.settings.Settings;
|
||||||
|
import org.elasticsearch.repositories.RepositoryData;
|
||||||
import org.elasticsearch.snapshots.SnapshotId;
|
import org.elasticsearch.snapshots.SnapshotId;
|
||||||
import org.elasticsearch.snapshots.SnapshotInfo;
|
import org.elasticsearch.snapshots.SnapshotInfo;
|
||||||
import org.elasticsearch.snapshots.SnapshotMissingException;
|
import org.elasticsearch.snapshots.SnapshotMissingException;
|
||||||
|
@ -82,13 +83,14 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
|
||||||
List<SnapshotInfo> snapshotInfoBuilder = new ArrayList<>();
|
List<SnapshotInfo> snapshotInfoBuilder = new ArrayList<>();
|
||||||
final Map<String, SnapshotId> allSnapshotIds = new HashMap<>();
|
final Map<String, SnapshotId> allSnapshotIds = new HashMap<>();
|
||||||
final List<SnapshotId> currentSnapshotIds = new ArrayList<>();
|
final List<SnapshotId> currentSnapshotIds = new ArrayList<>();
|
||||||
|
final RepositoryData repositoryData = snapshotsService.getRepositoryData(repository);
|
||||||
for (SnapshotInfo snapshotInfo : snapshotsService.currentSnapshots(repository)) {
|
for (SnapshotInfo snapshotInfo : snapshotsService.currentSnapshots(repository)) {
|
||||||
SnapshotId snapshotId = snapshotInfo.snapshotId();
|
SnapshotId snapshotId = snapshotInfo.snapshotId();
|
||||||
allSnapshotIds.put(snapshotId.getName(), snapshotId);
|
allSnapshotIds.put(snapshotId.getName(), snapshotId);
|
||||||
currentSnapshotIds.add(snapshotId);
|
currentSnapshotIds.add(snapshotId);
|
||||||
}
|
}
|
||||||
if (isCurrentSnapshotsOnly(request.snapshots()) == false) {
|
if (isCurrentSnapshotsOnly(request.snapshots()) == false) {
|
||||||
for (SnapshotId snapshotId : snapshotsService.getRepositoryData(repository).getAllSnapshotIds()) {
|
for (SnapshotId snapshotId : repositoryData.getAllSnapshotIds()) {
|
||||||
allSnapshotIds.put(snapshotId.getName(), snapshotId);
|
allSnapshotIds.put(snapshotId.getName(), snapshotId);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -119,7 +121,8 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
snapshotInfoBuilder.addAll(snapshotsService.snapshots(repository, new ArrayList<>(toResolve), request.ignoreUnavailable()));
|
snapshotInfoBuilder.addAll(snapshotsService.snapshots(
|
||||||
|
repository, new ArrayList<>(toResolve), repositoryData.getIncompatibleSnapshotIds(), request.ignoreUnavailable()));
|
||||||
listener.onResponse(new GetSnapshotsResponse(snapshotInfoBuilder));
|
listener.onResponse(new GetSnapshotsResponse(snapshotInfoBuilder));
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
listener.onFailure(e);
|
listener.onFailure(e);
|
||||||
|
|
|
@ -490,15 +490,6 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public SnapshotInfo getSnapshotInfo(final SnapshotId snapshotId) {
|
public SnapshotInfo getSnapshotInfo(final SnapshotId snapshotId) {
|
||||||
if (getRepositoryData().getIncompatibleSnapshotIds().contains(snapshotId)) {
|
|
||||||
// an incompatible snapshot - cannot read its snapshot metadata file, just return
|
|
||||||
// a SnapshotInfo indicating its incompatible
|
|
||||||
return SnapshotInfo.incompatible(snapshotId);
|
|
||||||
}
|
|
||||||
return getSnapshotInfoInternal(snapshotId);
|
|
||||||
}
|
|
||||||
|
|
||||||
private SnapshotInfo getSnapshotInfoInternal(final SnapshotId snapshotId) {
|
|
||||||
try {
|
try {
|
||||||
return snapshotFormat.read(snapshotsBlobContainer, snapshotId.getUUID());
|
return snapshotFormat.read(snapshotsBlobContainer, snapshotId.getUUID());
|
||||||
} catch (NoSuchFileException ex) {
|
} catch (NoSuchFileException ex) {
|
||||||
|
|
|
@ -164,11 +164,15 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus
|
||||||
*
|
*
|
||||||
* @param repositoryName repository name
|
* @param repositoryName repository name
|
||||||
* @param snapshotIds snapshots for which to fetch snapshot information
|
* @param snapshotIds snapshots for which to fetch snapshot information
|
||||||
|
* @param incompatibleSnapshotIds snapshots for which not to fetch snapshot information
|
||||||
* @param ignoreUnavailable if true, snapshots that could not be read will only be logged with a warning,
|
* @param ignoreUnavailable if true, snapshots that could not be read will only be logged with a warning,
|
||||||
* if false, they will throw an error
|
* if false, they will throw an error
|
||||||
* @return list of snapshots
|
* @return list of snapshots
|
||||||
*/
|
*/
|
||||||
public List<SnapshotInfo> snapshots(final String repositoryName, List<SnapshotId> snapshotIds, final boolean ignoreUnavailable) {
|
public List<SnapshotInfo> snapshots(final String repositoryName,
|
||||||
|
final List<SnapshotId> snapshotIds,
|
||||||
|
final List<SnapshotId> incompatibleSnapshotIds,
|
||||||
|
final boolean ignoreUnavailable) {
|
||||||
final Set<SnapshotInfo> snapshotSet = new HashSet<>();
|
final Set<SnapshotInfo> snapshotSet = new HashSet<>();
|
||||||
final Set<SnapshotId> snapshotIdsToIterate = new HashSet<>(snapshotIds);
|
final Set<SnapshotId> snapshotIdsToIterate = new HashSet<>(snapshotIds);
|
||||||
// first, look at the snapshots in progress
|
// first, look at the snapshots in progress
|
||||||
|
@ -182,7 +186,13 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus
|
||||||
final Repository repository = repositoriesService.repository(repositoryName);
|
final Repository repository = repositoriesService.repository(repositoryName);
|
||||||
for (SnapshotId snapshotId : snapshotIdsToIterate) {
|
for (SnapshotId snapshotId : snapshotIdsToIterate) {
|
||||||
try {
|
try {
|
||||||
|
if (incompatibleSnapshotIds.contains(snapshotId)) {
|
||||||
|
// an incompatible snapshot - cannot read its snapshot metadata file, just return
|
||||||
|
// a SnapshotInfo indicating its incompatible
|
||||||
|
snapshotSet.add(SnapshotInfo.incompatible(snapshotId));
|
||||||
|
} else {
|
||||||
snapshotSet.add(repository.getSnapshotInfo(snapshotId));
|
snapshotSet.add(repository.getSnapshotInfo(snapshotId));
|
||||||
|
}
|
||||||
} catch (Exception ex) {
|
} catch (Exception ex) {
|
||||||
if (ignoreUnavailable) {
|
if (ignoreUnavailable) {
|
||||||
logger.warn((Supplier<?>) () -> new ParameterizedMessage("failed to get snapshot [{}]", snapshotId), ex);
|
logger.warn((Supplier<?>) () -> new ParameterizedMessage("failed to get snapshot [{}]", snapshotId), ex);
|
||||||
|
|
Loading…
Reference in New Issue