honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[7.x] Fix negative limiting with fewer PARTIAL snapshots than minimum required (#58563) (#58569) 

In SLM retention, when a minimum number of snapshots is required for retention, we prefer to remove
the oldest snapshots first. To perform this, we limit one of the streams, in a rare case this can
cause:

```
[mynode] error during snapshot retention task
java.lang.IllegalArgumentException: -5
	at java.util.stream.ReferencePipeline.limit(ReferencePipeline.java:469) ~[?:?]
	at org.elasticsearch.xpack.core.slm.SnapshotRetentionConfiguration.lambda$getSnapshotDeletionPredicate$6(SnapshotRetentionConfiguration.java:195) ~[?:?]
	at org.elasticsearch.xpack.slm.SnapshotRetentionTask.snapshotEligibleForDeletion(SnapshotRetentionTask.java:245) ~[?:?]
	at org.elasticsearch.xpack.slm.SnapshotRetentionTask$1.lambda$onResponse$0(SnapshotRetentionTask.java:163) ~[?:?]
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:176) ~[?:?]
	at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1624) ~[?:?]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) ~[?:?]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) ~[?:?]
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913) ~[?:?]
```

When certain criteria are met. This commit fixes the negative limiting with `Math.max(0, ...)` and
adds a unit test for the behavior.

Resolves #58515
This commit is contained in:
Lee Hinman 2020-06-25 14:16:34 -06:00 committed by GitHub
parent 5f52bc4c9f
commit f732003370
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 13 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotRetentionConfiguration.java
View File

@ -186,7 +186,7 @@ public class SnapshotRetentionConfiguration implements ToXContentObject, Writeab
final TimeValue snapshotAge = new TimeValue(nowSupplier.getAsLong() - si.startTime());
if (this.minimumSnapshotCount != null) {
final long eligibleForExpiration = successfulSnapshotCount - minimumSnapshotCount;
final long eligibleForExpiration = Math.max(0, successfulSnapshotCount - minimumSnapshotCount);
// Only the oldest N snapshots are actually eligible, since if we went below this we
// would fall below the configured minimum number of snapshots to keep

13
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionConfigurationTests.java
View File

@ -227,7 +227,6 @@ public class SnapshotRetentionConfigurationTests extends ESTestCase {
assertThat(conf.getSnapshotDeletionPredicate(infos).test(oldInfo), equalTo(true));
}
public void testMostRecentSuccessfulTimestampIsUsed() {
boolean failureBeforePartial = randomBoolean();
SnapshotRetentionConfiguration conf = new SnapshotRetentionConfiguration(() -> 1, null, 2, 2);
@ -243,6 +242,18 @@ public class SnapshotRetentionConfigurationTests extends ESTestCase {
assertThat(conf.getSnapshotDeletionPredicate(infos).test(s4), equalTo(false));
}
public void testFewerSuccessesThanMinWithPartial() {
SnapshotRetentionConfiguration conf = new SnapshotRetentionConfiguration(() -> 1, TimeValue.timeValueSeconds(5), 10, 20);
SnapshotInfo s1 = makeInfo(1);
SnapshotInfo sP = makePartialInfo(2);
SnapshotInfo s2 = makeInfo(3);
List<SnapshotInfo> infos = Arrays.asList(s1 , sP, s2);
assertThat(conf.getSnapshotDeletionPredicate(infos).test(s1), equalTo(false));
assertThat(conf.getSnapshotDeletionPredicate(infos).test(sP), equalTo(false));
assertThat(conf.getSnapshotDeletionPredicate(infos).test(s2), equalTo(false));
}
private SnapshotInfo makeInfo(long startTime) {
final Map<String, Object> meta = new HashMap<>();
meta.put(SnapshotLifecyclePolicy.POLICY_ID_METADATA_FIELD, REPO);