PR feedback

Author: nielsbauman

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ResizeIndexStep.java

@@ -29,10 +29,14 @@
  * Resizes an index with the specified settings, using the name that was generated in a previous {@link GenerateUniqueIndexNameStep} step.
  */
 public class ResizeIndexStep extends AsyncActionStep {
+
+    public static final String SHRINK = "shrink";
+    public static final String CLONE = "clone";
     private static final Logger logger = LogManager.getLogger(ResizeIndexStep.class);
 
     private final ResizeType resizeType;
     private final BiFunction<String, LifecycleExecutionState, String> targetIndexNameSupplier;
+    /** A supplier that takes the index metadata of the <i>original</i> index and returns settings for the target index . */
     private final Function<IndexMetadata, Settings> targetIndexSettingsSupplier;
     @Nullable
     private final ByteSizeValue maxPrimaryShardSize;
@@ -51,6 +55,7 @@ public ResizeIndexStep(
         this.targetIndexNameSupplier = targetIndexNameSupplier;
         this.targetIndexSettingsSupplier = targetIndexSettingsSupplier;
         this.maxPrimaryShardSize = maxPrimaryShardSize;
+        assert resizeType == ResizeType.SHRINK || maxPrimaryShardSize == null : "maxPrimaryShardSize can only be set for shrink operations";
     }
 
     @Override
@@ -94,9 +99,13 @@ public void performAction(
             TimeValue.MAX_VALUE
         );
         resizeRequest.setResizeType(resizeType);
-        resizeRequest.setMaxPrimaryShardSize(maxPrimaryShardSize);
         resizeRequest.getTargetIndexRequest().settings(relevantTargetSettings);
+        if (resizeType == ResizeType.SHRINK) {
+            resizeRequest.setMaxPrimaryShardSize(maxPrimaryShardSize);
+        }
 
+        // This request does not wait for (successful) completion of the resize operation - it fires-and-forgets.
+        // It's up to a subsequent step to check for the existence of the target index and wait for it to be green.
         getClient(currentState.projectId()).admin()
             .indices()
             .resizeIndex(resizeRequest, listener.delegateFailureAndWrap((l, response) -> l.onResponse(null)));

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ShrinkAction.java

@@ -48,7 +48,6 @@ public class ShrinkAction implements LifecycleAction {
     public static final ParseField ALLOW_WRITE_AFTER_SHRINK = new ParseField("allow_write_after_shrink");
     public static final String CONDITIONAL_SKIP_SHRINK_STEP = BranchingStep.NAME + "-check-prerequisites";
     public static final String CLEANUP_SHRINK_INDEX_STEP = "cleanup-shrink-index";
-    public static final String SHRINK_STEP = "shrink";
     public static final String CONDITIONAL_DATASTREAM_CHECK_KEY = BranchingStep.NAME + "-on-datastream-check";
 
     private static final ConstructingObjectParser<ShrinkAction, Void> PARSER = new ConstructingObjectParser<>(
@@ -175,7 +174,7 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
         StepKey generateShrinkIndexNameKey = new StepKey(phase, NAME, GenerateUniqueIndexNameStep.NAME);
         StepKey setSingleNodeKey = new StepKey(phase, NAME, SetSingleNodeAllocateStep.NAME);
         StepKey allocationRoutedKey = new StepKey(phase, NAME, CheckShrinkReadyStep.NAME);
-        StepKey shrinkKey = new StepKey(phase, NAME, SHRINK_STEP);
+        StepKey shrinkKey = new StepKey(phase, NAME, ResizeIndexStep.SHRINK);
         StepKey enoughShardsKey = new StepKey(phase, NAME, ShrunkShardsAllocatedStep.NAME);
         StepKey copyMetadataKey = new StepKey(phase, NAME, CopyExecutionStateStep.NAME);
         StepKey dataStreamCheckBranchingKey = new StepKey(phase, NAME, CONDITIONAL_DATASTREAM_CHECK_KEY);

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ResizeIndexStepTests.java

@@ -39,7 +39,9 @@ public ResizeIndexStep createRandomInstance() {
         ResizeType resizeType = randomFrom(ResizeType.values());
         Settings.Builder settings = Settings.builder();
         ByteSizeValue maxPrimaryShardSize = null;
-        if (randomBoolean()) {
+        // Only shrink supports max_primary_shard_size, so if we pick shrink we sometimes set it, otherwise we always
+        // set number_of_shards.
+        if (resizeType != ResizeType.SHRINK || randomBoolean()) {
             settings.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, randomIntBetween(1, 20));
         } else {
             maxPrimaryShardSize = ByteSizeValue.ofBytes(between(1, 100));
@@ -65,7 +67,14 @@ public ResizeIndexStep mutateInstance(ResizeIndexStep instance) {
         switch (between(0, 2)) {
             case 0 -> key = new StepKey(key.phase(), key.action(), key.name() + randomAlphaOfLength(5));
             case 1 -> nextKey = new StepKey(nextKey.phase(), nextKey.action(), nextKey.name() + randomAlphaOfLength(5));
-            case 2 -> resizeType = randomValueOtherThan(resizeType, () -> randomFrom(ResizeType.values()));
+            case 2 -> {
+                if (resizeType != ResizeType.SHRINK || randomBoolean()) {
+                    resizeType = randomValueOtherThan(resizeType, () -> randomFrom(ResizeType.values()));
+                    maxPrimaryShardSize = null;
+                } else {
+                    maxPrimaryShardSize = randomValueOtherThan(maxPrimaryShardSize, () -> ByteSizeValue.ofBytes(between(1, 100)));
+                }
+            }
             default -> throw new AssertionError("Illegal randomisation branch");
         }
 

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrinkActionTests.java

@@ -290,7 +290,7 @@ public void testToSteps() {
         StepKey generateShrinkIndexNameKey = new StepKey(phase, ShrinkAction.NAME, GenerateUniqueIndexNameStep.NAME);
         StepKey setSingleNodeKey = new StepKey(phase, ShrinkAction.NAME, SetSingleNodeAllocateStep.NAME);
         StepKey allocationRoutedKey = new StepKey(phase, ShrinkAction.NAME, CheckShrinkReadyStep.NAME);
-        StepKey shrinkKey = new StepKey(phase, ShrinkAction.NAME, ShrinkAction.SHRINK_STEP);
+        StepKey shrinkKey = new StepKey(phase, ShrinkAction.NAME, ResizeIndexStep.SHRINK);
         StepKey enoughShardsKey = new StepKey(phase, ShrinkAction.NAME, ShrunkShardsAllocatedStep.NAME);
         StepKey copyMetadataKey = new StepKey(phase, ShrinkAction.NAME, CopyExecutionStateStep.NAME);
         StepKey dataStreamCheckBranchingKey = new StepKey(phase, ShrinkAction.NAME, ShrinkAction.CONDITIONAL_DATASTREAM_CHECK_KEY);

x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleService.java

@@ -48,6 +48,7 @@
 import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
 import org.elasticsearch.xpack.core.ilm.OperationMode;
 import org.elasticsearch.xpack.core.ilm.OperationModeUpdateTask;
+import org.elasticsearch.xpack.core.ilm.ResizeIndexStep;
 import org.elasticsearch.xpack.core.ilm.SetSingleNodeAllocateStep;
 import org.elasticsearch.xpack.core.ilm.ShrinkAction;
 import org.elasticsearch.xpack.core.ilm.ShrunkShardsAllocatedStep;
@@ -83,7 +84,7 @@ public class IndexLifecycleService
         IndexEventListener,
         ShutdownAwarePlugin {
     private static final Logger logger = LogManager.getLogger(IndexLifecycleService.class);
-    private static final Set<String> IGNORE_STEPS_MAINTENANCE_REQUESTED = Set.of(ShrinkAction.SHRINK_STEP, DownsampleStep.NAME);
+    private static final Set<String> IGNORE_STEPS_MAINTENANCE_REQUESTED = Set.of(ResizeIndexStep.SHRINK, DownsampleStep.NAME);
     private volatile boolean isMaster = false;
     private volatile TimeValue pollInterval;
 
@@ -617,7 +618,7 @@ static boolean hasIndicesInDangerousStepForNodeShutdown(ClusterState state, Stri
                     String step = indexToMetadata.getValue().getLifecycleExecutionState().step();
                     return SetSingleNodeAllocateStep.NAME.equals(step)
                         || CheckShrinkReadyStep.NAME.equals(step)
-                        || ShrinkAction.SHRINK_STEP.equals(step)
+                        || ResizeIndexStep.SHRINK.equals(step)
                         || ShrunkShardsAllocatedStep.NAME.equals(step);
                 })
                 // Only look at indices where the node picked for the shrink is the node marked as shutting down

x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/IndexLifecycleServiceTests.java

@@ -50,6 +50,7 @@
 import org.elasticsearch.xpack.core.ilm.OperationMode;
 import org.elasticsearch.xpack.core.ilm.OperationModeUpdateTask;
 import org.elasticsearch.xpack.core.ilm.Phase;
+import org.elasticsearch.xpack.core.ilm.ResizeIndexStep;
 import org.elasticsearch.xpack.core.ilm.SetSingleNodeAllocateStep;
 import org.elasticsearch.xpack.core.ilm.ShrinkAction;
 import org.elasticsearch.xpack.core.ilm.ShrunkShardsAllocatedStep;
@@ -183,7 +184,7 @@ public void testStoppedModeSkip() {
     }
 
     public void testRequestedStopOnShrink() {
-        Step.StepKey mockShrinkStep = new Step.StepKey(randomAlphaOfLength(4), ShrinkAction.NAME, ShrinkAction.SHRINK_STEP);
+        Step.StepKey mockShrinkStep = new Step.StepKey(randomAlphaOfLength(4), ShrinkAction.NAME, ResizeIndexStep.SHRINK);
         String policyName = randomAlphaOfLengthBetween(1, 20);
         IndexLifecycleRunnerTests.MockClusterStateActionStep mockStep = new IndexLifecycleRunnerTests.MockClusterStateActionStep(
             mockShrinkStep,
@@ -231,7 +232,7 @@ public void testRequestedStopInShrinkActionButNotShrinkStep() {
         action.toSteps(mock(Client.class), "warm", randomStepKey())
             .stream()
             .map(sk -> sk.getKey().name())
-            .filter(name -> name.equals(ShrinkAction.SHRINK_STEP) == false)
+            .filter(name -> name.equals(ResizeIndexStep.SHRINK) == false)
             .forEach(this::verifyCanStopWithStep);
     }
 
@@ -562,7 +563,7 @@ public void testHasIndicesInDangerousStepForNodeShutdown() {
                             randomFrom(
                                 SetSingleNodeAllocateStep.NAME,
                                 CheckShrinkReadyStep.NAME,
-                                ShrinkAction.SHRINK_STEP,
+                                ResizeIndexStep.SHRINK,
                                 ShrunkShardsAllocatedStep.NAME
                             )
                         )

x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/TransportGetShutdownStatusActionTests.java

@@ -54,6 +54,7 @@
 import org.elasticsearch.xpack.core.ilm.ErrorStep;
 import org.elasticsearch.xpack.core.ilm.LifecycleOperationMetadata;
 import org.elasticsearch.xpack.core.ilm.OperationMode;
+import org.elasticsearch.xpack.core.ilm.ResizeIndexStep;
 import org.elasticsearch.xpack.core.ilm.ShrinkAction;
 import org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType;
 import org.hamcrest.Matcher;
@@ -693,7 +694,7 @@ public void testExplainThrottled() {
     public void testIlmShrinkingIndexAvoidsStall() {
         LifecycleExecutionState executionState = LifecycleExecutionState.builder()
             .setAction(ShrinkAction.NAME)
-            .setStep(ShrinkAction.SHRINK_STEP)
+            .setStep(ResizeIndexStep.SHRINK)
             .setPhase(randomFrom("hot", "warm"))
             .build();
         checkStalledShardWithIlmState(executionState, OperationMode.RUNNING, SingleNodeShutdownMetadata.Status.IN_PROGRESS);
@@ -702,15 +703,15 @@ public void testIlmShrinkingIndexAvoidsStall() {
     public void testIlmShrinkingWithIlmStoppingIndexAvoidsStall() {
         LifecycleExecutionState executionState = LifecycleExecutionState.builder()
             .setAction(ShrinkAction.NAME)
-            .setStep(ShrinkAction.SHRINK_STEP)
+            .setStep(ResizeIndexStep.SHRINK)
             .build();
         checkStalledShardWithIlmState(executionState, OperationMode.STOPPING, SingleNodeShutdownMetadata.Status.IN_PROGRESS);
     }
 
     public void testIlmShrinkingButIlmStoppedDoesNotAvoidStall() {
         LifecycleExecutionState executionState = LifecycleExecutionState.builder()
             .setAction(ShrinkAction.NAME)
-            .setStep(ShrinkAction.SHRINK_STEP)
+            .setStep(ResizeIndexStep.SHRINK)
             .build();
         checkStalledShardWithIlmState(executionState, OperationMode.STOPPED, SingleNodeShutdownMetadata.Status.STALLED);
     }