kunwp1 commented on code in PR #6046:
URL: https://github.com/apache/texera/pull/6046#discussion_r4001148390
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitManagingResource.scala:
##########
@@ -61,18 +67,211 @@ import org.apache.texera.service.util.{
KubernetesClient
}
import org.jooq.{DSLContext, EnumType}
+import org.jooq.impl.DSL.{boolOr, max}
+import org.slf4j.LoggerFactory
import play.api.libs.json._
import java.sql.Timestamp
import scala.annotation.unused
import scala.jdk.CollectionConverters.CollectionHasAsScala
+import scala.util.control.NonFatal
object ComputingUnitManagingResource {
+ private[resource] val logger =
LoggerFactory.getLogger(classOf[ComputingUnitManagingResource])
+
private def context: DSLContext =
SqlServer
.getInstance()
.createDSLContext()
+ private[resource] case class IdleComputingUnitCleanupConfig(
+ enabled: Boolean,
+ idleTimeoutMinutes: Long
+ )
+
+ private[resource] case class IdleComputingUnitCandidate(
+ unit: WorkflowComputingUnit,
+ username: Option[String]
+ )
+
+ /**
+ * The codes persisted in `workflow_executions.status`. These are the
collapsed codes produced by
+ * amber's `Utils.maptoStatusCode`, NOT the ordinals of
`WorkflowAggregatedState`, and this
+ * service cannot depend on the amber module to reuse either. Keep this in
sync with
+ * `Utils.maptoStatusCode`: 0=UNINITIALIZED/READY, 1=RUNNING, 2=PAUSED,
3=COMPLETED, 4=FAILED,
+ * 5=KILLED. Only the non-terminal codes are listed here, since that is all
the sweep needs.
+ */
+ private[resource] object WorkflowExecutionStatus extends Enumeration {
+ val UninitializedOrReady: Value = Value(0)
+ val Running: Value = Value(1)
+ val Paused: Value = Value(2)
+
+ def toDbStatus(status: Value): java.lang.Short =
Short.box(status.id.toShort)
+ }
+
+ private[resource] trait KubernetesPodOperations {
+ val podExists: Int => Boolean
+ val deletePod: Int => Unit
+ }
+
+ private[resource] object DefaultKubernetesPodOperations extends
KubernetesPodOperations {
+ override val podExists: Int => Boolean = KubernetesClient.podExists
+ override val deletePod: Int => Unit = KubernetesClient.deletePod
+ }
+
+ private[resource] def lastComputingUnitActivityTime(
+ unit: WorkflowComputingUnit,
+ latestUpdateTime: Option[Timestamp],
+ latestStartTime: Option[Timestamp]
+ ): Timestamp =
+ Seq(
+ latestUpdateTime,
+ latestStartTime,
+ Option(unit.getCreationTime)
+ ).flatten.maxBy(_.getTime)
+
+ private[resource] def shouldTerminateIdleComputingUnit(
+ hasActiveExecution: Boolean,
+ lastExecutionTime: Timestamp,
+ cutoff: Timestamp
+ ): Boolean =
+ !hasActiveExecution && lastExecutionTime.before(cutoff)
+
+ def terminateIdleKubernetesComputingUnits():
List[TerminatedComputingUnitInfo] =
+ runIdleKubernetesComputingUnitCleanup(
+ IdleComputingUnitCleanupConfig(
+ KubernetesConfig.kubernetesComputingUnitEnabled,
+ KubernetesConfig.computingUnitIdleTimeoutMinutes
+ ),
+ () => new Timestamp(System.currentTimeMillis()),
+ DefaultKubernetesPodOperations
+ )
+
+ private[resource] def runIdleKubernetesComputingUnitCleanup(
+ cleanupConfig: IdleComputingUnitCleanupConfig,
+ currentTime: () => Timestamp,
+ podOperations: KubernetesPodOperations
+ ): List[TerminatedComputingUnitInfo] = {
+ if (!cleanupConfig.enabled || cleanupConfig.idleTimeoutMinutes <= 0) {
+ return List.empty
+ }
+
+ val now = currentTime()
+ val cutoff = new Timestamp(now.getTime - cleanupConfig.idleTimeoutMinutes
* 60 * 1000)
+
+ idleKubernetesComputingUnitCandidates(cutoff).flatMap(candidate =>
+ terminateIdleKubernetesComputingUnitCandidate(candidate, now,
podOperations)
+ )
+ }
+
+ private[resource] def idleKubernetesComputingUnitCandidates(
+ cutoff: Timestamp
+ ): List[IdleComputingUnitCandidate] = {
+ val activeStatuses = Seq(
+ WorkflowExecutionStatus.UninitializedOrReady,
+ WorkflowExecutionStatus.Running,
+ WorkflowExecutionStatus.Paused
+ ).map(WorkflowExecutionStatus.toDbStatus)
+
+ // All three questions asked per computing unit -- is any execution still
active, when did an
+ // execution last report progress, when did one last start -- are
aggregates over the same rows
+ // grouped by the same key, so one grouped query answers them for every
unit at once. The left
+ // joins keep units that have no executions (both max() are NULL) and
units whose owner row is
+ // gone (name is NULL), matching what a per-unit scan would produce.
+ val latestUpdateTime = max(WORKFLOW_EXECUTIONS.LAST_UPDATE_TIME)
+ val latestStartTime = max(WORKFLOW_EXECUTIONS.STARTING_TIME)
+ val hasActiveExecution =
boolOr(WORKFLOW_EXECUTIONS.STATUS.in(activeStatuses: _*))
+
+ withTransaction(context) { ctx =>
+ ctx
+ .select(
+ WORKFLOW_COMPUTING_UNIT.asterisk(),
+ USER.NAME,
+ latestUpdateTime,
+ latestStartTime,
+ hasActiveExecution
+ )
+ .from(WORKFLOW_COMPUTING_UNIT)
+ .leftJoin(WORKFLOW_EXECUTIONS)
+ .on(WORKFLOW_EXECUTIONS.CUID.eq(WORKFLOW_COMPUTING_UNIT.CUID))
+ .leftJoin(USER)
+ .on(USER.UID.eq(WORKFLOW_COMPUTING_UNIT.UID))
+ .where(
+ WORKFLOW_COMPUTING_UNIT.TYPE
+ .eq(WorkflowComputingUnitTypeEnum.kubernetes)
+ .and(WORKFLOW_COMPUTING_UNIT.TERMINATE_TIME.isNull)
+ )
+ .groupBy(WORKFLOW_COMPUTING_UNIT.CUID, USER.NAME)
+ .fetch()
+ .asScala
+ .flatMap { record =>
+ val unit =
record.into(WORKFLOW_COMPUTING_UNIT).into(classOf[WorkflowComputingUnit])
+ val lastExecutionTime = lastComputingUnitActivityTime(
+ unit,
+ Option(record.get(latestUpdateTime)),
+ Option(record.get(latestStartTime))
+ )
+
+ // bool_or over zero matching executions yields NULL, which means
"no active execution"
+ val active =
Option(record.get(hasActiveExecution)).exists(_.booleanValue())
+ if (shouldTerminateIdleComputingUnit(active, lastExecutionTime,
cutoff)) {
+ Some(
+ IdleComputingUnitCandidate(
+ unit,
+ Option(record.get(USER.NAME)).filter(_.nonEmpty)
+ )
+ )
+ } else {
+ None
+ }
+ }
+ .toList
+ }
+ }
+
+ private[resource] def terminateIdleKubernetesComputingUnitCandidate(
+ candidate: IdleComputingUnitCandidate,
+ terminationTime: Timestamp,
+ podOperations: KubernetesPodOperations
+ ): Option[TerminatedComputingUnitInfo] = {
+ val cuid = candidate.unit.getCuid
+ try {
+ if (podOperations.podExists(cuid)) {
+ podOperations.deletePod(cuid)
Review Comment:
Looks like this is doing two round trips to the Kubernetes layer for each CU
which looks inefficient. I see that in the below, you have a transaction to
update the database. Can you delete the pod after updating the corresponding DB
record?
##########
sql/updates/42.sql:
##########
Review Comment:
Can you rebase this repo to the latest main and resolve the conflict?
##########
common/config/src/main/resources/kubernetes.conf:
##########
@@ -41,6 +41,13 @@ kubernetes {
max-num-of-running-computing-units-per-user = 10
max-num-of-running-computing-units-per-user =
${?MAX_NUM_OF_RUNNING_COMPUTING_UNITS_PER_USER}
+ # Terminate Kubernetes CUs whose latest workflow execution is older than
this.
+ computing-unit-idle-timeout-minutes = 1440
+ computing-unit-idle-timeout-minutes =
${?KUBERNETES_COMPUTING_UNIT_IDLE_TIMEOUT_MINUTES}
+
+ computing-unit-idle-check-interval-minutes = 60
+ computing-unit-idle-check-interval-minutes =
${?KUBERNETES_COMPUTING_UNIT_IDLE_CHECK_INTERVAL_MINUTES}
+
Review Comment:
Also, shouldn't we add the vars to `values.yaml` and
`values-deployment.yaml` file?
##########
common/config/src/main/resources/kubernetes.conf:
##########
@@ -41,6 +41,13 @@ kubernetes {
max-num-of-running-computing-units-per-user = 10
max-num-of-running-computing-units-per-user =
${?MAX_NUM_OF_RUNNING_COMPUTING_UNITS_PER_USER}
+ # Terminate Kubernetes CUs whose latest workflow execution is older than
this.
+ computing-unit-idle-timeout-minutes = 1440
+ computing-unit-idle-timeout-minutes =
${?KUBERNETES_COMPUTING_UNIT_IDLE_TIMEOUT_MINUTES}
+
+ computing-unit-idle-check-interval-minutes = 60
+ computing-unit-idle-check-interval-minutes =
${?KUBERNETES_COMPUTING_UNIT_IDLE_CHECK_INTERVAL_MINUTES}
+
Review Comment:
I feel like this feature is a huge change but also risky and needs more
testing. Can we add an `enabled` flag and disable the feature by default?
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitManagingResource.scala:
##########
@@ -61,18 +67,211 @@ import org.apache.texera.service.util.{
KubernetesClient
}
import org.jooq.{DSLContext, EnumType}
+import org.jooq.impl.DSL.{boolOr, max}
+import org.slf4j.LoggerFactory
import play.api.libs.json._
import java.sql.Timestamp
import scala.annotation.unused
import scala.jdk.CollectionConverters.CollectionHasAsScala
+import scala.util.control.NonFatal
object ComputingUnitManagingResource {
+ private[resource] val logger =
LoggerFactory.getLogger(classOf[ComputingUnitManagingResource])
+
private def context: DSLContext =
SqlServer
.getInstance()
.createDSLContext()
+ private[resource] case class IdleComputingUnitCleanupConfig(
+ enabled: Boolean,
+ idleTimeoutMinutes: Long
+ )
+
+ private[resource] case class IdleComputingUnitCandidate(
+ unit: WorkflowComputingUnit,
+ username: Option[String]
+ )
+
+ /**
+ * The codes persisted in `workflow_executions.status`. These are the
collapsed codes produced by
+ * amber's `Utils.maptoStatusCode`, NOT the ordinals of
`WorkflowAggregatedState`, and this
+ * service cannot depend on the amber module to reuse either. Keep this in
sync with
+ * `Utils.maptoStatusCode`: 0=UNINITIALIZED/READY, 1=RUNNING, 2=PAUSED,
3=COMPLETED, 4=FAILED,
+ * 5=KILLED. Only the non-terminal codes are listed here, since that is all
the sweep needs.
+ */
+ private[resource] object WorkflowExecutionStatus extends Enumeration {
+ val UninitializedOrReady: Value = Value(0)
+ val Running: Value = Value(1)
+ val Paused: Value = Value(2)
+
+ def toDbStatus(status: Value): java.lang.Short =
Short.box(status.id.toShort)
+ }
+
+ private[resource] trait KubernetesPodOperations {
+ val podExists: Int => Boolean
+ val deletePod: Int => Unit
+ }
+
+ private[resource] object DefaultKubernetesPodOperations extends
KubernetesPodOperations {
+ override val podExists: Int => Boolean = KubernetesClient.podExists
+ override val deletePod: Int => Unit = KubernetesClient.deletePod
+ }
+
+ private[resource] def lastComputingUnitActivityTime(
+ unit: WorkflowComputingUnit,
+ latestUpdateTime: Option[Timestamp],
+ latestStartTime: Option[Timestamp]
+ ): Timestamp =
+ Seq(
+ latestUpdateTime,
+ latestStartTime,
+ Option(unit.getCreationTime)
+ ).flatten.maxBy(_.getTime)
+
+ private[resource] def shouldTerminateIdleComputingUnit(
+ hasActiveExecution: Boolean,
+ lastExecutionTime: Timestamp,
+ cutoff: Timestamp
+ ): Boolean =
+ !hasActiveExecution && lastExecutionTime.before(cutoff)
+
+ def terminateIdleKubernetesComputingUnits():
List[TerminatedComputingUnitInfo] =
+ runIdleKubernetesComputingUnitCleanup(
+ IdleComputingUnitCleanupConfig(
+ KubernetesConfig.kubernetesComputingUnitEnabled,
+ KubernetesConfig.computingUnitIdleTimeoutMinutes
+ ),
+ () => new Timestamp(System.currentTimeMillis()),
+ DefaultKubernetesPodOperations
+ )
+
+ private[resource] def runIdleKubernetesComputingUnitCleanup(
+ cleanupConfig: IdleComputingUnitCleanupConfig,
+ currentTime: () => Timestamp,
+ podOperations: KubernetesPodOperations
+ ): List[TerminatedComputingUnitInfo] = {
+ if (!cleanupConfig.enabled || cleanupConfig.idleTimeoutMinutes <= 0) {
+ return List.empty
+ }
+
+ val now = currentTime()
+ val cutoff = new Timestamp(now.getTime - cleanupConfig.idleTimeoutMinutes
* 60 * 1000)
+
+ idleKubernetesComputingUnitCandidates(cutoff).flatMap(candidate =>
+ terminateIdleKubernetesComputingUnitCandidate(candidate, now,
podOperations)
+ )
+ }
+
+ private[resource] def idleKubernetesComputingUnitCandidates(
+ cutoff: Timestamp
+ ): List[IdleComputingUnitCandidate] = {
+ val activeStatuses = Seq(
+ WorkflowExecutionStatus.UninitializedOrReady,
+ WorkflowExecutionStatus.Running,
+ WorkflowExecutionStatus.Paused
+ ).map(WorkflowExecutionStatus.toDbStatus)
+
+ // All three questions asked per computing unit -- is any execution still
active, when did an
+ // execution last report progress, when did one last start -- are
aggregates over the same rows
+ // grouped by the same key, so one grouped query answers them for every
unit at once. The left
+ // joins keep units that have no executions (both max() are NULL) and
units whose owner row is
+ // gone (name is NULL), matching what a per-unit scan would produce.
+ val latestUpdateTime = max(WORKFLOW_EXECUTIONS.LAST_UPDATE_TIME)
+ val latestStartTime = max(WORKFLOW_EXECUTIONS.STARTING_TIME)
+ val hasActiveExecution =
boolOr(WORKFLOW_EXECUTIONS.STATUS.in(activeStatuses: _*))
+
+ withTransaction(context) { ctx =>
+ ctx
+ .select(
+ WORKFLOW_COMPUTING_UNIT.asterisk(),
+ USER.NAME,
+ latestUpdateTime,
+ latestStartTime,
+ hasActiveExecution
+ )
+ .from(WORKFLOW_COMPUTING_UNIT)
+ .leftJoin(WORKFLOW_EXECUTIONS)
+ .on(WORKFLOW_EXECUTIONS.CUID.eq(WORKFLOW_COMPUTING_UNIT.CUID))
+ .leftJoin(USER)
+ .on(USER.UID.eq(WORKFLOW_COMPUTING_UNIT.UID))
+ .where(
+ WORKFLOW_COMPUTING_UNIT.TYPE
+ .eq(WorkflowComputingUnitTypeEnum.kubernetes)
+ .and(WORKFLOW_COMPUTING_UNIT.TERMINATE_TIME.isNull)
+ )
+ .groupBy(WORKFLOW_COMPUTING_UNIT.CUID, USER.NAME)
+ .fetch()
+ .asScala
+ .flatMap { record =>
+ val unit =
record.into(WORKFLOW_COMPUTING_UNIT).into(classOf[WorkflowComputingUnit])
+ val lastExecutionTime = lastComputingUnitActivityTime(
+ unit,
+ Option(record.get(latestUpdateTime)),
+ Option(record.get(latestStartTime))
+ )
+
+ // bool_or over zero matching executions yields NULL, which means
"no active execution"
+ val active =
Option(record.get(hasActiveExecution)).exists(_.booleanValue())
+ if (shouldTerminateIdleComputingUnit(active, lastExecutionTime,
cutoff)) {
+ Some(
+ IdleComputingUnitCandidate(
+ unit,
+ Option(record.get(USER.NAME)).filter(_.nonEmpty)
+ )
+ )
+ } else {
+ None
+ }
+ }
+ .toList
+ }
+ }
+
+ private[resource] def terminateIdleKubernetesComputingUnitCandidate(
+ candidate: IdleComputingUnitCandidate,
+ terminationTime: Timestamp,
+ podOperations: KubernetesPodOperations
+ ): Option[TerminatedComputingUnitInfo] = {
+ val cuid = candidate.unit.getCuid
+ try {
+ if (podOperations.podExists(cuid)) {
+ podOperations.deletePod(cuid)
+ }
+
+ withTransaction(context) { ctx =>
+ val cuDao = new WorkflowComputingUnitDao(ctx.configuration())
+ val unit = cuDao.fetchOneByCuid(cuid)
+ if (
+ unit == null ||
+ unit.getTerminateTime != null ||
+ unit.getType != WorkflowComputingUnitTypeEnum.kubernetes
+ ) {
+ None
+ } else {
+ val reason =
WorkflowComputingUnitTerminationReasonEnum.GARBAGE_COLLECTED
+ unit.setTerminateTime(terminationTime)
+ unit.setTerminationReason(reason)
+ cuDao.update(unit)
+ Some(
+ TerminatedComputingUnitInfo(
+ cuid = unit.getCuid,
+ name = unit.getName,
+ uid = unit.getUid,
+ username = candidate.username,
+ reason = reason
+ )
+ )
+ }
Review Comment:
Can you convert this whole logic into a single "UPDATE" query? It shouldn't
be this complicated.
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitManagingResource.scala:
##########
@@ -61,18 +67,211 @@ import org.apache.texera.service.util.{
KubernetesClient
}
import org.jooq.{DSLContext, EnumType}
+import org.jooq.impl.DSL.{boolOr, max}
+import org.slf4j.LoggerFactory
import play.api.libs.json._
import java.sql.Timestamp
import scala.annotation.unused
import scala.jdk.CollectionConverters.CollectionHasAsScala
+import scala.util.control.NonFatal
object ComputingUnitManagingResource {
+ private[resource] val logger =
LoggerFactory.getLogger(classOf[ComputingUnitManagingResource])
+
private def context: DSLContext =
SqlServer
.getInstance()
.createDSLContext()
+ private[resource] case class IdleComputingUnitCleanupConfig(
+ enabled: Boolean,
+ idleTimeoutMinutes: Long
+ )
+
+ private[resource] case class IdleComputingUnitCandidate(
+ unit: WorkflowComputingUnit,
+ username: Option[String]
+ )
+
+ /**
+ * The codes persisted in `workflow_executions.status`. These are the
collapsed codes produced by
+ * amber's `Utils.maptoStatusCode`, NOT the ordinals of
`WorkflowAggregatedState`, and this
+ * service cannot depend on the amber module to reuse either. Keep this in
sync with
+ * `Utils.maptoStatusCode`: 0=UNINITIALIZED/READY, 1=RUNNING, 2=PAUSED,
3=COMPLETED, 4=FAILED,
+ * 5=KILLED. Only the non-terminal codes are listed here, since that is all
the sweep needs.
+ */
+ private[resource] object WorkflowExecutionStatus extends Enumeration {
+ val UninitializedOrReady: Value = Value(0)
+ val Running: Value = Value(1)
+ val Paused: Value = Value(2)
+
+ def toDbStatus(status: Value): java.lang.Short =
Short.box(status.id.toShort)
+ }
+
+ private[resource] trait KubernetesPodOperations {
+ val podExists: Int => Boolean
+ val deletePod: Int => Unit
+ }
+
+ private[resource] object DefaultKubernetesPodOperations extends
KubernetesPodOperations {
+ override val podExists: Int => Boolean = KubernetesClient.podExists
+ override val deletePod: Int => Unit = KubernetesClient.deletePod
+ }
+
+ private[resource] def lastComputingUnitActivityTime(
+ unit: WorkflowComputingUnit,
+ latestUpdateTime: Option[Timestamp],
+ latestStartTime: Option[Timestamp]
+ ): Timestamp =
+ Seq(
+ latestUpdateTime,
+ latestStartTime,
+ Option(unit.getCreationTime)
+ ).flatten.maxBy(_.getTime)
+
+ private[resource] def shouldTerminateIdleComputingUnit(
+ hasActiveExecution: Boolean,
+ lastExecutionTime: Timestamp,
+ cutoff: Timestamp
+ ): Boolean =
+ !hasActiveExecution && lastExecutionTime.before(cutoff)
+
+ def terminateIdleKubernetesComputingUnits():
List[TerminatedComputingUnitInfo] =
+ runIdleKubernetesComputingUnitCleanup(
+ IdleComputingUnitCleanupConfig(
+ KubernetesConfig.kubernetesComputingUnitEnabled,
+ KubernetesConfig.computingUnitIdleTimeoutMinutes
+ ),
+ () => new Timestamp(System.currentTimeMillis()),
+ DefaultKubernetesPodOperations
+ )
+
+ private[resource] def runIdleKubernetesComputingUnitCleanup(
+ cleanupConfig: IdleComputingUnitCleanupConfig,
+ currentTime: () => Timestamp,
+ podOperations: KubernetesPodOperations
+ ): List[TerminatedComputingUnitInfo] = {
+ if (!cleanupConfig.enabled || cleanupConfig.idleTimeoutMinutes <= 0) {
+ return List.empty
+ }
+
+ val now = currentTime()
+ val cutoff = new Timestamp(now.getTime - cleanupConfig.idleTimeoutMinutes
* 60 * 1000)
+
+ idleKubernetesComputingUnitCandidates(cutoff).flatMap(candidate =>
+ terminateIdleKubernetesComputingUnitCandidate(candidate, now,
podOperations)
+ )
+ }
+
+ private[resource] def idleKubernetesComputingUnitCandidates(
+ cutoff: Timestamp
+ ): List[IdleComputingUnitCandidate] = {
+ val activeStatuses = Seq(
+ WorkflowExecutionStatus.UninitializedOrReady,
+ WorkflowExecutionStatus.Running,
+ WorkflowExecutionStatus.Paused
+ ).map(WorkflowExecutionStatus.toDbStatus)
Review Comment:
I checked `Utils.scala` and looks like the status code can also be `-1`. Is
this a complete non-terminal set?
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/ComputingUnitManagingService.scala:
##########
@@ -32,9 +32,65 @@ import org.apache.texera.service.resource.{
ComputingUnitManagingResource,
HealthCheckResource
}
+import
org.apache.texera.service.resource.ComputingUnitManagingResource.TerminatedComputingUnitInfo
+import org.slf4j.LoggerFactory
import java.nio.file.Path
+import java.util.concurrent.TimeUnit
class ComputingUnitManagingService extends
Application[ComputingUnitManagingServiceConfiguration] {
+ private val logger =
LoggerFactory.getLogger(classOf[ComputingUnitManagingService])
+
+ private def initSqlServer(): Unit =
+ SqlServer.initConnection(
+ StorageConfig.jdbcUrl,
+ StorageConfig.jdbcUsername,
+ StorageConfig.jdbcPassword
+ )
+
+ private[service] def registerIdleComputingUnitCleanup(
Review Comment:
Can you refer to `registerStagedFileCleanup` and reuse the existing
scheduled job shape? I think it will simplify the code.
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/ComputingUnitManagingService.scala:
##########
@@ -32,9 +32,65 @@ import org.apache.texera.service.resource.{
ComputingUnitManagingResource,
HealthCheckResource
}
+import
org.apache.texera.service.resource.ComputingUnitManagingResource.TerminatedComputingUnitInfo
+import org.slf4j.LoggerFactory
import java.nio.file.Path
+import java.util.concurrent.TimeUnit
class ComputingUnitManagingService extends
Application[ComputingUnitManagingServiceConfiguration] {
+ private val logger =
LoggerFactory.getLogger(classOf[ComputingUnitManagingService])
+
+ private def initSqlServer(): Unit =
+ SqlServer.initConnection(
+ StorageConfig.jdbcUrl,
+ StorageConfig.jdbcUsername,
+ StorageConfig.jdbcPassword
+ )
+
+ private[service] def registerIdleComputingUnitCleanup(
+ environment: Environment,
+ kubernetesComputingUnitEnabled: Boolean =
KubernetesConfig.kubernetesComputingUnitEnabled,
+ idleTimeoutMinutes: Long =
KubernetesConfig.computingUnitIdleTimeoutMinutes,
+ idleCheckIntervalMinutes: Long =
KubernetesConfig.computingUnitIdleCheckIntervalMinutes,
+ terminateIdleComputingUnits: () => List[TerminatedComputingUnitInfo] =
() =>
+ ComputingUnitManagingResource.terminateIdleKubernetesComputingUnits(),
+ logTerminatedUnits: String => Unit = message => logger.info(message),
+ logCleanupFailure: Throwable => Unit = throwable =>
+ logger.warn("Failed to terminate idle Kubernetes computing units",
throwable),
+ scheduleWithFixedDelay: Option[(Runnable, Long, Long, TimeUnit) => Unit]
= None
+ ): Unit = {
+ if (!kubernetesComputingUnitEnabled || idleTimeoutMinutes <= 0) {
+ return
+ }
+ // scheduleWithFixedDelay rejects a non-positive delay, which would abort
service startup.
+ // A misconfigured interval leaves the rest of the service usable, so log
it and skip the sweep.
+ if (idleCheckIntervalMinutes <= 0) {
+ logger.warn(
+ s"Idle Kubernetes computing unit cleanup is disabled: check interval
must be positive " +
+ s"but is $idleCheckIntervalMinutes minute(s)"
+ )
+ return
+ }
+
+ val scheduler = scheduleWithFixedDelay.getOrElse((command, initialDelay,
delay, unit) =>
+ environment.lifecycle
+ .scheduledExecutorService("idle-computing-unit-terminator")
+ .threads(1)
+ .build()
+ .scheduleWithFixedDelay(command, initialDelay, delay, unit)
+ )
+ scheduler(
+ () =>
+ ComputingUnitManagingService.runIdleComputingUnitCleanup(
+ terminateIdleComputingUnits,
+ logTerminatedUnits,
+ logCleanupFailure
+ ),
+ idleCheckIntervalMinutes,
+ idleCheckIntervalMinutes,
Review Comment:
I see that the initial delay is same as the interval (60 minutes). Consider
changing the initial delay to be `1L` because `StagedFileCleanupJob` has a
similar logic
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitManagingResource.scala:
##########
@@ -61,18 +67,211 @@ import org.apache.texera.service.util.{
KubernetesClient
}
import org.jooq.{DSLContext, EnumType}
+import org.jooq.impl.DSL.{boolOr, max}
+import org.slf4j.LoggerFactory
import play.api.libs.json._
import java.sql.Timestamp
import scala.annotation.unused
import scala.jdk.CollectionConverters.CollectionHasAsScala
+import scala.util.control.NonFatal
object ComputingUnitManagingResource {
+ private[resource] val logger =
LoggerFactory.getLogger(classOf[ComputingUnitManagingResource])
+
private def context: DSLContext =
SqlServer
.getInstance()
.createDSLContext()
+ private[resource] case class IdleComputingUnitCleanupConfig(
+ enabled: Boolean,
+ idleTimeoutMinutes: Long
+ )
+
+ private[resource] case class IdleComputingUnitCandidate(
+ unit: WorkflowComputingUnit,
+ username: Option[String]
+ )
+
+ /**
+ * The codes persisted in `workflow_executions.status`. These are the
collapsed codes produced by
+ * amber's `Utils.maptoStatusCode`, NOT the ordinals of
`WorkflowAggregatedState`, and this
+ * service cannot depend on the amber module to reuse either. Keep this in
sync with
+ * `Utils.maptoStatusCode`: 0=UNINITIALIZED/READY, 1=RUNNING, 2=PAUSED,
3=COMPLETED, 4=FAILED,
+ * 5=KILLED. Only the non-terminal codes are listed here, since that is all
the sweep needs.
+ */
+ private[resource] object WorkflowExecutionStatus extends Enumeration {
+ val UninitializedOrReady: Value = Value(0)
+ val Running: Value = Value(1)
+ val Paused: Value = Value(2)
+
+ def toDbStatus(status: Value): java.lang.Short =
Short.box(status.id.toShort)
+ }
+
+ private[resource] trait KubernetesPodOperations {
+ val podExists: Int => Boolean
+ val deletePod: Int => Unit
+ }
+
+ private[resource] object DefaultKubernetesPodOperations extends
KubernetesPodOperations {
+ override val podExists: Int => Boolean = KubernetesClient.podExists
+ override val deletePod: Int => Unit = KubernetesClient.deletePod
+ }
Review Comment:
Can you refer to `KubernetesClient` and `ComputingUnitHelpers` and mimic how
they use KubernetesClient? In that way, we can simplify the code by removing
these codes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
