feat(sql): Add additional dynamic config for event cleanup agent (#4934)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

clouddriver-sql/src/main/kotlin/com/netflix/spinnaker/clouddriver/sql/event/SqlEventCleanupAgent.kt

@@ -23,16 +23,15 @@ import com.netflix.spinnaker.clouddriver.sql.SqlAgent
 import com.netflix.spinnaker.config.ConnectionPools
 import com.netflix.spinnaker.config.SqlEventCleanupAgentConfigProperties
 import com.netflix.spinnaker.config.SqlEventCleanupAgentConfigProperties.Companion.EVENT_CLEANUP_LIMIT
+import com.netflix.spinnaker.kork.annotations.VisibleForTesting
 import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
 import com.netflix.spinnaker.kork.sql.routing.withPool
 import java.sql.Timestamp
 import java.time.Duration
 import java.time.Instant
 import org.jooq.DSLContext
-import org.jooq.impl.DSL.currentTimestamp
 import org.jooq.impl.DSL.field
 import org.jooq.impl.DSL.table
-import org.jooq.impl.DSL.timestampDiff
 import org.slf4j.LoggerFactory
 
 /**
@@ -53,24 +52,17 @@ class SqlEventCleanupAgent(
   override fun run() {
     val duration = Duration.ofDays(properties.maxAggregateAgeDays)
     val cutoff = Instant.now().minus(duration)
-    val limit = dynamicConfigService.getConfig(Int::class.java, EVENT_CLEANUP_CONFIG_KEY, EVENT_CLEANUP_LIMIT)
+    val limit = dynamicConfigService.getConfig(Int::class.java, EVENT_CLEANUP_LIMIT_KEY, EVENT_CLEANUP_LIMIT)
 
     log.info("Deleting aggregates last updated earlier than $cutoff ($duration), max $limit events")
 
     registry.timer(timingId).record {
+      val threshold = Instant.now().minus(duration)
+
       withPool(ConnectionPools.EVENTS.value) {
         val rs = jooq.select(field("aggregate_type"), field("aggregate_id"))
           .from(table("event_aggregates"))
-          .where(
-            // jOOQ always converts timestampDiff into 'INTERVAL DAY TO SECOND' type, which then no longer performs
-            // comparison queries correctly. Doing `timestampDiff().greaterThan()` will cast the result of
-            // timestampDiff to an int, as needed, but then cast the duration into an INTERVAL DAY TO SECOND type,
-            // which will always produce a query returning incorrect results.
-            timestampDiff(currentTimestamp(), field("last_change_timestamp", Timestamp::class.java))
-              .cast(Integer::class.java)
-              // timestampDiff will return `microsecond / 1000`
-              .gt(duration.toMillis().toInt() as Integer)
-          )
+          .where(field("last_change_timestamp").lt(Timestamp(threshold.toEpochMilli())))
           .limit(limit)
           .fetch()
           .intoResultSet()
@@ -94,10 +86,17 @@ class SqlEventCleanupAgent(
 
   override fun getAgentType(): String = javaClass.simpleName
   override fun getProviderName(): String = CoreProvider.PROVIDER_NAME
-  override fun getPollIntervalMillis() = properties.frequency.toMillis()
-  override fun getTimeoutMillis() = properties.timeout.toMillis()
 
-  companion object {
-    private const val EVENT_CLEANUP_CONFIG_KEY = "spinnaker.clouddriver.eventing.cleanup-agent.cleanup-limit"
+  override fun getPollIntervalMillis() =
+    Duration.parse(dynamicConfigService.getConfig(String::class.java, EVENT_CLEANUP_INTERVAL_KEY, "PT1M")).toMillis()
+
+  override fun getTimeoutMillis() =
+    Duration.parse(dynamicConfigService.getConfig(String::class.java, EVENT_CLEANUP_TIMEOUT_KEY, "PT45S")).toMillis()
+
+  @VisibleForTesting
+  internal companion object {
+    const val EVENT_CLEANUP_LIMIT_KEY = "spinnaker.clouddriver.eventing.cleanup-agent.cleanup-limit"
+    const val EVENT_CLEANUP_INTERVAL_KEY = "spinnaker.clouddriver.eventing.cleanup-agent.frequency"
+    const val EVENT_CLEANUP_TIMEOUT_KEY = "spinnaker.clouddriver.event.cleanup-agent.timeout"
   }
 }

clouddriver-sql/src/main/kotlin/com/netflix/spinnaker/config/SqlEventCleanupAgentConfigProperties.kt

@@ -24,15 +24,15 @@ import org.springframework.validation.annotation.Validated
 @ConfigurationProperties("spinnaker.clouddriver.eventing.cleanup-agent")
 class SqlEventCleanupAgentConfigProperties {
   /**
-   * The frequency, in milliseconds, of how often the cleanup agent will run. Defaults to 30 minutes.
+   * The frequency, in milliseconds, of how often the cleanup agent will run. Defaults to 1 minute.
    */
-  var frequency: Duration = Duration.ofMinutes(30)
+  var frequency: Duration = Duration.ofMinutes(1)
 
   /**
    * The ceiling execution time that the agent should be allowed to run before it will be timed out and available for
-   * reschedule onto a different Clouddriver instance. Defaults to 1 hour.
+   * reschedule onto a different Clouddriver instance. Defaults to 45 seconds.
    */
-  var timeout: Duration = Duration.ofHours(1)
+  var timeout: Duration = Duration.ofSeconds(45)
 
   /**
    * The max age of an [Aggregate]. Defaults to 7 days.
@@ -41,12 +41,12 @@ class SqlEventCleanupAgentConfigProperties {
   var maxAggregateAgeDays: Long = 7
 
   /**
-   * The max number of events to cleanup in each agent invocation.
+   * The max number of events to cleanup in each agent invocation. Defaults to 1000.
    */
   @Positive
   var cleanupLimit: Int = EVENT_CLEANUP_LIMIT
 
   companion object {
-    const val EVENT_CLEANUP_LIMIT = 100_000
+    const val EVENT_CLEANUP_LIMIT = 1_000
   }
 }

clouddriver-sql/src/test/kotlin/com/netflix/spinnaker/clouddriver/sql/event/SqlEventCleanupAgentTest.kt

@@ -17,6 +17,9 @@
 package com.netflix.spinnaker.clouddriver.sql.event
 
 import com.netflix.spectator.api.NoopRegistry
+import com.netflix.spinnaker.clouddriver.sql.event.SqlEventCleanupAgent.Companion.EVENT_CLEANUP_INTERVAL_KEY
+import com.netflix.spinnaker.clouddriver.sql.event.SqlEventCleanupAgent.Companion.EVENT_CLEANUP_LIMIT_KEY
+import com.netflix.spinnaker.clouddriver.sql.event.SqlEventCleanupAgent.Companion.EVENT_CLEANUP_TIMEOUT_KEY
 import com.netflix.spinnaker.config.SqlEventCleanupAgentConfigProperties
 import com.netflix.spinnaker.config.SqlEventCleanupAgentConfigProperties.Companion.EVENT_CLEANUP_LIMIT
 import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
@@ -82,7 +85,9 @@ class SqlEventCleanupAgentTest : JUnit5Minutests {
     )
 
     init {
-      every { dynamicConfigService.getConfig(eq(Int::class.java), any(), eq(EVENT_CLEANUP_LIMIT)) } returns EVENT_CLEANUP_LIMIT
+      every { dynamicConfigService.getConfig(eq(Int::class.java), eq(EVENT_CLEANUP_LIMIT_KEY), eq(EVENT_CLEANUP_LIMIT)) } returns EVENT_CLEANUP_LIMIT
+      every { dynamicConfigService.getConfig(eq(String::class.java), eq(EVENT_CLEANUP_INTERVAL_KEY), eq("PT1M")) } returns "PT1M"
+      every { dynamicConfigService.getConfig(eq(String::class.java), eq(EVENT_CLEANUP_TIMEOUT_KEY), eq("PT45S")) } returns "PT45S"
     }
   }
 }