From 564bc971c2484b74742b6152218631c86e84e426 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Wed, 20 May 2026 19:20:15 -0600 Subject: [PATCH] fix: log shuffle-manager-missing warning once per session isCometLoaded runs from every plan rule application, so the warning introduced in #4328 was emitted hundreds of times per Spark SQL test suite for sessions that never register CometShuffleManager. Track SQLConf identities in a synchronized weak set so the warning fires at most once per session. Refs #4377. --- .../comet/CometSparkSessionExtensions.scala | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala b/spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala index 1ae90e1845..30f8e104e9 100644 --- a/spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala +++ b/spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala @@ -114,6 +114,14 @@ class CometSparkSessionExtensions object CometSparkSessionExtensions extends Logging { lazy val isBigEndian: Boolean = ByteOrder.nativeOrder().equals(ByteOrder.BIG_ENDIAN) + // isCometLoaded is invoked once per plan rule application, so the shuffle-manager + // warning would otherwise fire many times per session. Track SQLConf identities we + // have already warned for; the weak map drops entries when the session is GC'd. + private val warnedMissingShuffleManager: java.util.Set[SQLConf] = + java.util.Collections.synchronizedSet( + java.util.Collections.newSetFromMap( + new java.util.WeakHashMap[SQLConf, java.lang.Boolean]())) + /** * Checks whether Comet extension should be loaded for Spark. */ @@ -128,12 +136,14 @@ object CometSparkSessionExtensions extends Logging { } if (COMET_EXEC_SHUFFLE_ENABLED.get(conf) && !isCometShuffleManagerEnabled(conf)) { - logWarning( - "Comet extension is disabled because spark.shuffle.manager is not set to " + - "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager. " + - "Comet provides limited benefit without its shuffle manager. " + - s"Set ${COMET_EXEC_SHUFFLE_ENABLED.key}=false to keep Comet enabled with " + - "Spark's default shuffle manager.") + if (warnedMissingShuffleManager.add(conf)) { + logWarning( + "Comet extension is disabled because spark.shuffle.manager is not set to " + + "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager. " + + "Comet provides limited benefit without its shuffle manager. " + + s"Set ${COMET_EXEC_SHUFFLE_ENABLED.key}=false to keep Comet enabled with " + + "Spark's default shuffle manager.") + } return false }