Skip to content

Commit a8acbaa

Browse files
author
stheppi
committed
fix(cloud-sink): ensure stop() completes cleanup even if writerManager.close() throws
Wrap each teardown step in Try so a failure in one step does not skip the others. Previously, an exception from writerManager.close() would propagate out of stop() before indexManager.close() ran, leaking the GC and sweep ScheduledExecutorService threads, and before CloudSinkMetricsRegistrar.unregister ran, leaving a stale MBean registered. Mirrors the closeOnFailure pattern already used in createWriterMan. Each suppressed exception is logged at WARN so failures are visible. Made-with: Cursor
1 parent 6cc70b0 commit a8acbaa

1 file changed

Lines changed: 21 additions & 6 deletions

File tree

  • kafka-connect-cloud-common/src/main/scala/io/lenses/streamreactor/connect/cloud/common/sink

kafka-connect-cloud-common/src/main/scala/io/lenses/streamreactor/connect/cloud/common/sink/CloudSinkTask.scala

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -303,22 +303,37 @@ abstract class CloudSinkTask[MD <: FileMetadata, C <: CloudSinkConfig[CC], CC <:
303303
}
304304

305305
override def stop(): Unit = {
306-
logger.debug("[{}] Stop", Option(connectorTaskId).map(_.show).getOrElse("Unnamed"))
306+
val taskIdStr = Option(connectorTaskId).map(_.show).getOrElse("Unnamed")
307+
logger.debug("[{}] Stop", taskIdStr)
308+
309+
// Each teardown step is wrapped in Try so a failure in one does not skip the
310+
// remaining steps. In particular, an exception from writerManager.close() must
311+
// not prevent indexManager.close() (which shuts down the GC/sweep
312+
// ScheduledExecutorService threads) or CloudSinkMetricsRegistrar.unregister
313+
// (which removes the MBean) from running. Mirrors the closeOnFailure pattern
314+
// in createWriterMan.
307315

308316
// Defensive: close writers in case stop() is called without a preceding close()
309317
// (e.g. during error recovery or non-standard Connect runtimes). WriterManager.close()
310318
// is idempotent -- on the normal close-then-stop path, writers are already closed and
311-
// the map is empty, so this is a no-op. Removing this call saves no meaningful work
312-
// but eliminates a safety net for edge cases.
313-
Option(writerManager).foreach(_.close())
319+
// the map is empty, so this is a no-op.
320+
Try(Option(writerManager).foreach(_.close())).failed.foreach { t =>
321+
logger.warn(s"[$taskIdStr] writerManager.close() failed during stop()", t)
322+
}
314323
writerManager = null
324+
315325
// indexManager.close() shuts down background executors and performs a final synchronous
316326
// drainGcQueue(). seekedOffsets is still populated (close() does not clear it), so the
317327
// drain correctly identifies owned partitions. The state is GC'd with the indexManager
318328
// reference immediately after.
319-
Option(indexManager).foreach(_.close())
329+
Try(Option(indexManager).foreach(_.close())).failed.foreach { t =>
330+
logger.warn(s"[$taskIdStr] indexManager.close() failed during stop()", t)
331+
}
320332
indexManager = null
321-
Option(connectorTaskId).foreach(CloudSinkMetricsRegistrar.unregister)
333+
334+
Try(Option(connectorTaskId).foreach(CloudSinkMetricsRegistrar.unregister)).failed.foreach { t =>
335+
logger.warn(s"[$taskIdStr] CloudSinkMetricsRegistrar.unregister failed during stop()", t)
336+
}
322337
}
323338

324339
def createClient(config: CC): Either[Throwable, CT]

0 commit comments

Comments
 (0)