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

Optimize SLM Policy Queries #79341

Merged
merged 5 commits into from
Oct 19, 2021
Merged

Optimize SLM Policy Queries #79341

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6920186
Optimize SLM Policy Queries
original-brownbear Oct 17, 2021
5bf3ed7
formatting
original-brownbear Oct 18, 2021
7e89a09
missing javadoc
original-brownbear Oct 18, 2021
62f870f
Merge remote-tracking branch 'elastic/master' into slm-store-policy-r…
original-brownbear Oct 19, 2021
cccca50
nicer generics :)
original-brownbear Oct 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...nternalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ public void testEnforcedCooldownPeriod() throws IOException {
SnapshotState.SUCCESS,
SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION.minimumCompatibilityVersion(),
0L, // -1 would refresh RepositoryData and find the real version
0L // -1 would refresh RepositoryData and find the real version
0L, // -1 would refresh RepositoryData and find the real version,
null // null would refresh RepositoryData and find the real version
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment suggests that this shouldn't be null.

)));
final BytesReference serialized = BytesReference.bytes(modifiedRepositoryData.snapshotsToXContent(XContentFactory.jsonBuilder(),
SnapshotsService.OLD_SNAPSHOT_FORMAT));
Expand Down
2 changes: 1 addition & 1 deletion .../internalClusterTest/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ public void testHandlingMissingRootLevelSnapshotMetadata() throws Exception {
.collect(
Collectors.toMap(
SnapshotId::getUUID,
s -> new RepositoryData.SnapshotDetails(repositoryData.getSnapshotState(s), null, -1, -1)
s -> new RepositoryData.SnapshotDetails(repositoryData.getSnapshotState(s), null, -1, -1, null)
)
),
Collections.emptyMap(),
Expand Down
65 changes: 47 additions & 18 deletions ...ava/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ private static Predicate<SnapshotInfo> buildAfterPredicate(
}
}

private static Predicate<SnapshotInfo> filterBySLMPolicies(String[] slmPolicies) {
private static Tuple<Predicate<SnapshotInfo>, BiPredicate<SnapshotId, RepositoryData>> filterBySLMPolicies(String[] slmPolicies) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Tuple<Predicate<SnapshotInfo>, BiPredicate<SnapshotId, RepositoryData>> is getting a bit convoluted, really it's just the following but without the names or anywhere to hang docs:

interface SomeName {
  boolean method1(SnapshotInfo snapshotInfo);
  boolean method2(SnapshotId snapshotId, RepositoryData repositoryData);
}

I think the names & docs would be useful, let's make this a thing. Maybe we should be subclassing SnapshotPredicates rather than having these higher-order fields.

Copy link
Member Author

Choose a reason for hiding this comment

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

++ I did this for this specific case, now but I agree, it's a worthwhile cleanup all over :) I'll do it in a follow-up after FF

final List<String> includePatterns = new ArrayList<>();
final List<String> excludePatterns = new ArrayList<>();
boolean seenWildcard = false;
Expand All @@ -686,7 +686,7 @@ private static Predicate<SnapshotInfo> filterBySLMPolicies(String[] slmPolicies)
final String[] includes = includePatterns.toArray(Strings.EMPTY_ARRAY);
final String[] excludes = excludePatterns.toArray(Strings.EMPTY_ARRAY);
final boolean matchWithoutPolicy = matchNoPolicy;
return snapshotInfo -> {
return Tuple.tuple(snapshotInfo -> {
final Map<String, Object> metadata = snapshotInfo.userMetadata();
final String policy;
if (metadata == null) {
Expand All @@ -695,14 +695,30 @@ private static Predicate<SnapshotInfo> filterBySLMPolicies(String[] slmPolicies)
final Object policyFound = metadata.get(SnapshotsService.POLICY_ID_METADATA_FIELD);
policy = policyFound instanceof String ? (String) policyFound : null;
}
if (policy == null) {
return matchWithoutPolicy;
}
if (Regex.simpleMatch(includes, policy) == false) {
return false;
return matchPolicy(includes, excludes, matchWithoutPolicy, policy);
}, (snapshotId, repositoryData) -> {
final RepositoryData.SnapshotDetails details = repositoryData.getSnapshotDetails(snapshotId);
final String policy;
if (details == null || (details.getSlmPolicy() == null)) {
// no SLM policy recorded
return true;
} else {
final String policyFound = details.getSlmPolicy();
// empty string means that snapshot was not created by an SLM policy
policy = policyFound.isEmpty() ? null : policyFound;
}
return excludes.length == 0 || Regex.simpleMatch(excludes, policy) == false;
};
return matchPolicy(includes, excludes, matchWithoutPolicy, policy);
});
}

private static boolean matchPolicy(String[] includes, String[] excludes, boolean matchWithoutPolicy, @Nullable String policy) {
if (policy == null) {
return matchWithoutPolicy;
}
if (Regex.simpleMatch(includes, policy) == false) {
return false;
}
return excludes.length == 0 || Regex.simpleMatch(excludes, policy) == false;
}

private static Predicate<SnapshotInfo> filterByLongOffset(ToLongFunction<SnapshotInfo> extractor, long after, SortOrder order) {
Expand Down Expand Up @@ -759,19 +775,23 @@ private static final class SnapshotPredicates {
Predicate<SnapshotInfo> snapshotPredicate = null;
final String[] slmPolicies = request.policies();
final String fromSortValue = request.fromSortValue();
BiPredicate<SnapshotId, RepositoryData> preflightPredicate = null;
if (slmPolicies.length > 0) {
snapshotPredicate = filterBySLMPolicies(slmPolicies);
final Tuple<Predicate<SnapshotInfo>, BiPredicate<SnapshotId, RepositoryData>> predicates = filterBySLMPolicies(slmPolicies);
snapshotPredicate = predicates.v1();
preflightPredicate = predicates.v2();
}
final GetSnapshotsRequest.SortBy sortBy = request.sort();
final SortOrder order = request.order();
if (fromSortValue == null) {
preflightPredicate = null;
this.preflightPredicate = preflightPredicate;
} else {
final Predicate<SnapshotInfo> fromSortValuePredicate;
final BiPredicate<SnapshotId, RepositoryData> preflightPred;
switch (sortBy) {
case START_TIME:
final long after = Long.parseLong(fromSortValue);
preflightPredicate = order == SortOrder.ASC ? (snapshotId, repositoryData) -> {
preflightPred = order == SortOrder.ASC ? (snapshotId, repositoryData) -> {
final long startTime = getStartTime(snapshotId, repositoryData);
return startTime == -1 || after <= startTime;
} : (snapshotId, repositoryData) -> {
Expand All @@ -781,14 +801,14 @@ private static final class SnapshotPredicates {
fromSortValuePredicate = filterByLongOffset(SnapshotInfo::startTime, after, order);
break;
case NAME:
preflightPredicate = order == SortOrder.ASC
preflightPred = order == SortOrder.ASC
? (snapshotId, repositoryData) -> fromSortValue.compareTo(snapshotId.getName()) <= 0
: (snapshotId, repositoryData) -> fromSortValue.compareTo(snapshotId.getName()) >= 0;
fromSortValuePredicate = null;
break;
case DURATION:
final long afterDuration = Long.parseLong(fromSortValue);
preflightPredicate = order == SortOrder.ASC ? (snapshotId, repositoryData) -> {
preflightPred = order == SortOrder.ASC ? (snapshotId, repositoryData) -> {
final long duration = getDuration(snapshotId, repositoryData);
return duration == -1 || afterDuration <= duration;
} : (snapshotId, repositoryData) -> {
Expand All @@ -799,22 +819,22 @@ private static final class SnapshotPredicates {
break;
case INDICES:
final int afterIndexCount = Integer.parseInt(fromSortValue);
preflightPredicate = order == SortOrder.ASC
preflightPred = order == SortOrder.ASC
? (snapshotId, repositoryData) -> afterIndexCount <= indexCount(snapshotId, repositoryData)
: (snapshotId, repositoryData) -> afterIndexCount >= indexCount(snapshotId, repositoryData);
fromSortValuePredicate = null;
break;
case REPOSITORY:
// already handled in #maybeFilterRepositories
preflightPredicate = null;
preflightPred = null;
fromSortValuePredicate = null;
break;
case SHARDS:
preflightPredicate = null;
preflightPred = null;
fromSortValuePredicate = filterByLongOffset(SnapshotInfo::totalShards, Integer.parseInt(fromSortValue), order);
break;
case FAILED_SHARDS:
preflightPredicate = null;
preflightPred = null;
fromSortValuePredicate = filterByLongOffset(SnapshotInfo::failedShards, Integer.parseInt(fromSortValue), order);
break;
default:
Expand All @@ -826,6 +846,15 @@ private static final class SnapshotPredicates {
} else if (fromSortValuePredicate != null) {
snapshotPredicate = fromSortValuePredicate.and(snapshotPredicate);
}
if (preflightPredicate == null) {
this.preflightPredicate = preflightPred;
} else {
if (preflightPred != null) {
this.preflightPredicate = preflightPredicate.and(preflightPred);
} else {
this.preflightPredicate = preflightPredicate;
}
}
}
this.snapshotPredicate = snapshotPredicate;
}
Expand Down
42 changes: 36 additions & 6 deletions server/src/main/java/org/elasticsearch/repositories/RepositoryData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,8 @@ public boolean hasMissingDetails(SnapshotId snapshotId) {
return snapshotDetails == null
|| snapshotDetails.getVersion() == null
|| snapshotDetails.getStartTimeMillis() == -1
|| snapshotDetails.getEndTimeMillis() == -1;
|| snapshotDetails.getEndTimeMillis() == -1
|| snapshotDetails.getSlmPolicy() == null;
}

/**
Expand Down Expand Up @@ -639,6 +640,7 @@ public Map<String, IndexId> resolveNewIndices(List<String> indicesToResolve, Map
private static final String MIN_VERSION = "min_version";
private static final String START_TIME_MILLIS = "start_time_millis";
private static final String END_TIME_MILLIS = "end_time_millis";
private static final String SLM_POLICY = "slm_policy";

/**
* Writes the snapshots metadata and the related indices metadata to x-content.
Expand Down Expand Up @@ -725,6 +727,9 @@ public XContentBuilder snapshotsToXContent(final XContentBuilder builder, final
if (snapshotDetails.getEndTimeMillis() != -1) {
builder.field(END_TIME_MILLIS, snapshotDetails.getEndTimeMillis());
}
if (snapshotDetails.getSlmPolicy() != null) {
builder.field(SLM_POLICY, snapshotDetails.getSlmPolicy());
}

builder.endObject();
}
Expand Down Expand Up @@ -891,6 +896,7 @@ private static void parseSnapshots(
Version version = null;
long startTimeMillis = -1;
long endTimeMillis = -1;
String slmPolicy = null;
while (parser.nextToken() != XContentParser.Token.END_OBJECT) {
String currentFieldName = parser.currentName();
parser.nextToken();
Expand Down Expand Up @@ -918,12 +924,14 @@ private static void parseSnapshots(
assert endTimeMillis == -1;
endTimeMillis = parser.longValue();
break;
case SLM_POLICY:
slmPolicy = stringDeduplicator.computeIfAbsent(parser.text(), Function.identity());
}
}
assert (startTimeMillis == -1) == (endTimeMillis == -1) : "unexpected: " + startTimeMillis + ", " + endTimeMillis + ", ";
final SnapshotId snapshotId = new SnapshotId(name, uuid);
if (state != null || version != null) {
snapshotsDetails.put(uuid, new SnapshotDetails(state, version, startTimeMillis, endTimeMillis));
snapshotsDetails.put(uuid, new SnapshotDetails(state, version, startTimeMillis, endTimeMillis, slmPolicy));
}
snapshots.put(uuid, snapshotId);
if (metaGenerations != null && metaGenerations.isEmpty() == false) {
Expand Down Expand Up @@ -1037,7 +1045,7 @@ private static String parseLegacySnapshotUUID(XContentParser parser) throws IOEx
*/
public static class SnapshotDetails {

public static SnapshotDetails EMPTY = new SnapshotDetails(null, null, -1, -1);
public static SnapshotDetails EMPTY = new SnapshotDetails(null, null, -1, -1, null);

@Nullable // TODO forbid nulls here, this only applies to very old repositories
private final SnapshotState snapshotState;
Expand All @@ -1051,11 +1059,23 @@ public static class SnapshotDetails {
// May be -1 if unknown, which happens if the snapshot was taken before 7.14 and hasn't been updated yet
private final long endTimeMillis;

public SnapshotDetails(@Nullable SnapshotState snapshotState, @Nullable Version version, long startTimeMillis, long endTimeMillis) {
// May be null if unknown, which happens if the snapshot was taken before 7.16 and hasn't been updated yet. Empty string indicates
// that this snapshot was not created by an SLM policy.
@Nullable
private final String slmPolicy;

public SnapshotDetails(
@Nullable SnapshotState snapshotState,
@Nullable Version version,
long startTimeMillis,
long endTimeMillis,
@Nullable String slmPolicy
) {
this.snapshotState = snapshotState;
this.version = version;
this.startTimeMillis = startTimeMillis;
this.endTimeMillis = endTimeMillis;
this.slmPolicy = slmPolicy;
}

@Nullable
Expand All @@ -1082,6 +1102,15 @@ public long getEndTimeMillis() {
return endTimeMillis;
}

/**
* @return the SLM policy that the snapshot was created by or an empty string if it was not created by an SLM policy or
* {@code null} if unknown.
*/
@Nullable
public String getSlmPolicy() {
return slmPolicy;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand All @@ -1090,12 +1119,13 @@ public boolean equals(Object o) {
return startTimeMillis == that.startTimeMillis
&& endTimeMillis == that.endTimeMillis
&& snapshotState == that.snapshotState
&& Objects.equals(version, that.version);
&& Objects.equals(version, that.version)
&& Objects.equals(slmPolicy, that.slmPolicy);
}

@Override
public int hashCode() {
return Objects.hash(snapshotState, version, startTimeMillis, endTimeMillis);
return Objects.hash(snapshotState, version, startTimeMillis, endTimeMillis, slmPolicy);
}

}
Expand Down
Loading