hudi-agent commented on code in PR #20362:
URL: https://github.com/apache/hudi/pull/20362#discussion_r4290899959


##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/testutils/HoodieSparkClientTestHarness.java:
##########
@@ -219,6 +251,213 @@ protected void initSparkContexts(String appName) {
     //       as such by Spark
     System.setProperty("spark.testing", "true");
     sparkSession.sparkContext().persistentRdds().foreach(rdd -> 
rdd._2.persist());
+    if (reuse) {
+      sharedSparkContext = new SharedSparkContext(getClass(), extraConf, 
sparkSessionExtensionsInjector.isPresent(),
+          jsc, sparkSession, sqlContext);
+    }
+  }
+
+  /**
+   * Whether this class may run all of its tests on one Spark context when 
{@link #REUSE_SPARK_CONTEXT_PROP} is set.
+   * Classes that need a fresh {@link SparkContext} per test (e.g. ones that 
assert on context-wide state such as
+   * listeners, accumulators or executor-side statics) override this to return 
{@code false}.
+   */
+  protected boolean reuseSparkContextAcrossTests() {
+    return true;
+  }
+
+  private boolean isSparkContextReuseEnabled() {
+    return Boolean.getBoolean(REUSE_SPARK_CONTEXT_PROP) && 
reuseSparkContextAcrossTests();
+  }
+
+  private void attachSharedSparkContext() {
+    jsc = sharedSparkContext.jsc;
+    sparkSession = sharedSparkContext.sparkSession;
+    sqlContext = sharedSparkContext.sqlContext;
+    SparkSession.setActiveSession(sparkSession);
+    SparkSession.setDefaultSession(sparkSession);
+    storageConf = HadoopFSUtils.getStorageConf(jsc.hadoopConfiguration());
+    context = new HoodieSparkEngineContext(jsc, sqlContext);
+  }
+
+  private static void stopSharedSparkContext() {
+    if (sharedSparkContext != null) {
+      SharedSparkContext shared = sharedSparkContext;
+      sharedSparkContext = null;
+      shared.stop();
+    }
+  }
+
+  /**
+   * One {@link SparkSession}/{@link JavaSparkContext} reused by the tests of 
a single class. Between tests it is
+   * reset to the state it had right after creation: SQL confs, Hadoop conf, 
local properties, listeners, cached
+   * data, temp views and catalog entries. If a reset step fails the context 
is stopped, and the next test creates
+   * a fresh one.
+   */
+  private static final class SharedSparkContext {
+    private final Class<?> ownerClass;
+    private final Map<String, String> extraConf;
+    private final boolean withExtensions;
+    private final JavaSparkContext jsc;
+    private final SparkSession sparkSession;
+    private final SQLContext sqlContext;
+    private final Map<String, String> initialSqlConf;

Review Comment:
   🤖 nit: could you import `Configuration`, `SparkListenerInterface`, `Set`, 
`Catalog`, etc. instead of using fully qualified names throughout 
`SharedSparkContext`?
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/testutils/HoodieSparkClientTestHarness.java:
##########
@@ -121,9 +121,32 @@
 @Slf4j
 public abstract class HoodieSparkClientTestHarness extends 
HoodieWriterClientTestHarness {
 
+  /**
+   * Opt-in switch for reusing one Spark context per test class (see {@link 
#reuseSparkContextAcrossTests()}).
+   */
+  public static final String REUSE_SPARK_CONTEXT_PROP = 
"hoodie.test.spark.reuse.context";

Review Comment:
   🤖 non-blocking: Since you offered, I'd lean toward dropping the shared Spark 
context for now, because no job turns it on and it showed no measurable gain, 
so its roughly 250 lines of reset logic (SQL conf, Hadoop conf, catalog, 
listeners, jobs) would likely fall out of date with nothing to catch it; it 
could come back in its own PR along with a CI lane that enables it.
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



-- 
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]

Reply via email to