Fixes a new transition not using new step start time if previous step… (

#381) * Fixes a new transition not using new step start time if previous step was also a transition * Adds delay between instants
opendistro-for-elasticsearch · Jan 20, 2021 · 0b4c8f2 · 0b4c8f2
1 parent ed27978
commit 0b4c8f2
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 3 deletions.
diff --git a/...n/com/amazon/opendistroforelasticsearch/indexmanagement/indexstatemanagement/step/Step.kt b/...n/com/amazon/opendistroforelasticsearch/indexmanagement/indexstatemanagement/step/Step.kt
@@ -58,10 +58,15 @@ abstract class Step(val name: String, val managedIndexMetaData: ManagedIndexMeta
     fun getStartingStepMetaData(): StepMetaData = StepMetaData(name, getStepStartTime().toEpochMilli(), StepStatus.STARTING)
 
     fun getStepStartTime(): Instant {
-        if (managedIndexMetaData.stepMetaData == null || managedIndexMetaData.stepMetaData.name != this.name) {
-            return Instant.now()
+        return when {
+            managedIndexMetaData.stepMetaData == null -> Instant.now()
+            managedIndexMetaData.stepMetaData.name != this.name -> Instant.now()
+            // The managed index metadata is a historical snapshot of the metadata and refers to what has happened from the previous
+            // execution, so if we ever see it as COMPLETED it means we are always going to be in a new step, this specifically
+            // helps with the Transition -> Transition (empty state) sequence which the above do not capture
+            managedIndexMetaData.stepMetaData.stepStatus == StepStatus.COMPLETED -> Instant.now()
+            else -> Instant.ofEpochMilli(managedIndexMetaData.stepMetaData.startTime)
         }
-        return Instant.ofEpochMilli(managedIndexMetaData.stepMetaData.startTime)
     }
 
     protected val indexName: String = managedIndexMetaData.index

diff --git a/...oforelasticsearch/indexmanagement/indexstatemanagement/step/AttemptTransitionStepTests.kt b/...oforelasticsearch/indexmanagement/indexstatemanagement/step/AttemptTransitionStepTests.kt
@@ -19,6 +19,7 @@ import com.amazon.opendistroforelasticsearch.indexmanagement.indexstatemanagemen
 import com.amazon.opendistroforelasticsearch.indexmanagement.indexstatemanagement.model.ManagedIndexMetaData
 import com.amazon.opendistroforelasticsearch.indexmanagement.indexstatemanagement.model.Transition
 import com.amazon.opendistroforelasticsearch.indexmanagement.indexstatemanagement.model.action.TransitionsActionConfig
+import com.amazon.opendistroforelasticsearch.indexmanagement.indexstatemanagement.model.managedindexmetadata.StepMetaData
 import com.amazon.opendistroforelasticsearch.indexmanagement.indexstatemanagement.step.transition.AttemptTransitionStep
 import com.nhaarman.mockitokotlin2.any
 import com.nhaarman.mockitokotlin2.doAnswer
@@ -40,6 +41,7 @@ import org.elasticsearch.index.shard.DocsStats
 import org.elasticsearch.rest.RestStatus
 import org.elasticsearch.test.ESTestCase
 import org.elasticsearch.transport.RemoteTransportException
+import java.time.Instant
 
 class AttemptTransitionStepTests : ESTestCase() {
 
@@ -103,6 +105,20 @@ class AttemptTransitionStepTests : ESTestCase() {
         }
     }
 
+    fun `test step start time resetting between two transitions`() {
+        val client = getClient(getAdminClient(getIndicesAdminClient(statsResponse, null)))
+
+        runBlocking {
+            val config = TransitionsActionConfig(listOf(Transition("some_state", null)))
+            val completedStartTime = Instant.now()
+            val managedIndexMetaData = ManagedIndexMetaData("test", "indexUuid", "policy_id", null, null, null, null, null, null, null, StepMetaData("attempt_transition", completedStartTime.toEpochMilli(), Step.StepStatus.COMPLETED), null, null)
+            val step = AttemptTransitionStep(clusterService, client, config, managedIndexMetaData)
+            Thread.sleep(50) // Make sure we give enough time for the instants to be different
+            val startTime = step.getStepStartTime()
+            assertNotEquals("Two separate transitions should not have same start time", completedStartTime.toEpochMilli(), startTime.toEpochMilli())
+        }
+    }
+
     private fun getClient(adminClient: AdminClient): Client = mock { on { admin() } doReturn adminClient }
     private fun getAdminClient(indicesAdminClient: IndicesAdminClient): AdminClient = mock { on { indices() } doReturn indicesAdminClient }
     private fun getIndicesAdminClient(statsResponse: IndicesStatsResponse?, exception: Exception?): IndicesAdminClient {