voonhous commented on code in PR #19921:
URL: https://github.com/apache/hudi/pull/19921#discussion_r3995326324
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/HoodieSparkSqlTestBase.scala:
##########
@@ -71,25 +75,53 @@ class HoodieSparkSqlTestBase extends FunSuite with
BeforeAndAfterAll {
// is consistent with the fixtures
DateTimeZone.setDefault(DateTimeZone.UTC)
TimeZone.setDefault(DateTimeUtils.getTimeZone("UTC"))
- protected lazy val spark: SparkSession = SparkSession.builder()
- .config("spark.sql.warehouse.dir", sparkWareHouse.getCanonicalPath)
- .config("spark.sql.session.timeZone", "UTC")
- .config("hoodie.insert.shuffle.parallelism", "4")
- .config("hoodie.upsert.shuffle.parallelism", "4")
- .config("hoodie.delete.shuffle.parallelism", "4")
- .config(sparkConf())
- .getOrCreate()
+ protected lazy val spark: SparkSession = if (sharedSessionEnabled) {
+ val session = HoodieSparkSqlTestBase.sharedBaseSession().newSession()
+ SparkSession.setActiveSession(session)
+ applySuiteConfToSharedSession(session)
+ session
+ } else {
+ HoodieSparkSqlTestBase.sessionBuilder(sparkWareHouse,
sparkConf()).getOrCreate()
+ }
private var tableId = new AtomicInteger(0)
private var extraConf = Map[String, String]()
+ // Shared mode: spark.hadoop.* keys this suite set on the shared Hadoop
conf, with the value they replaced.
+ private var hadoopConfOverrides: Seq[(String, String)] = Seq.empty
+
def sparkConf(): SparkConf = {
val conf = getSparkConfForTest("Hoodie SQL Test")
conf.setAll(extraConf)
conf
}
+ /**
+ * Shared mode: the context-level SparkConf is fixed, so the deltas a suite
adds through extraConf or a
+ * sparkConf() override go to its session conf, except spark.hadoop.* keys,
which the write client reads
+ * from sparkContext.hadoopConfiguration and which are restored in afterAll.
Any other context-level key
+ * fails loudly in RuntimeConfig.set.
+ */
+ private def applySuiteConfToSharedSession(session: SparkSession): Unit = {
+ val defaults = getSparkConfForTest("Hoodie SQL Test").getAll.toMap
+ val hadoopConf = session.sparkContext.hadoopConfiguration
+ sparkConf().getAll.filterNot { case (k, v) => defaults.get(k).contains(v)
}.foreach {
+ case (k, v) if k.startsWith("spark.hadoop.") =>
+ val key = k.stripPrefix("spark.hadoop.")
+ hadoopConfOverrides :+= (key -> hadoopConf.get(key))
+ hadoopConf.set(key, v)
+ case (k, v) => session.conf.set(k, v)
+ }
+ }
+
+ override protected def beforeAll(): Unit = {
+ super.beforeAll()
+ if (sharedSessionEnabled) {
+ SparkSession.setActiveSession(spark)
Review Comment:
Agreed, forcing the lazy val in beforeAll made the extraConf snapshot
mode-dependent. Dropped the override in 55bed9e8c7de: the lazy val initializer
sets the active session on the thread that forces it and the test wrapper
re-pins it per test, so nothing else was relying on it. No suite in the tree
sets extraConf after super.beforeAll today, so this removes a divergence rather
than a live failure.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/HoodieSparkSqlTestBase.scala:
##########
@@ -71,25 +75,53 @@ class HoodieSparkSqlTestBase extends FunSuite with
BeforeAndAfterAll {
// is consistent with the fixtures
DateTimeZone.setDefault(DateTimeZone.UTC)
TimeZone.setDefault(DateTimeUtils.getTimeZone("UTC"))
- protected lazy val spark: SparkSession = SparkSession.builder()
- .config("spark.sql.warehouse.dir", sparkWareHouse.getCanonicalPath)
- .config("spark.sql.session.timeZone", "UTC")
- .config("hoodie.insert.shuffle.parallelism", "4")
- .config("hoodie.upsert.shuffle.parallelism", "4")
- .config("hoodie.delete.shuffle.parallelism", "4")
- .config(sparkConf())
- .getOrCreate()
+ protected lazy val spark: SparkSession = if (sharedSessionEnabled) {
+ val session = HoodieSparkSqlTestBase.sharedBaseSession().newSession()
+ SparkSession.setActiveSession(session)
+ applySuiteConfToSharedSession(session)
+ session
+ } else {
+ HoodieSparkSqlTestBase.sessionBuilder(sparkWareHouse,
sparkConf()).getOrCreate()
+ }
private var tableId = new AtomicInteger(0)
private var extraConf = Map[String, String]()
+ // Shared mode: spark.hadoop.* keys this suite set on the shared Hadoop
conf, with the value they replaced.
+ private var hadoopConfOverrides: Seq[(String, String)] = Seq.empty
+
def sparkConf(): SparkConf = {
val conf = getSparkConfForTest("Hoodie SQL Test")
conf.setAll(extraConf)
conf
}
+ /**
+ * Shared mode: the context-level SparkConf is fixed, so the deltas a suite
adds through extraConf or a
+ * sparkConf() override go to its session conf, except spark.hadoop.* keys,
which the write client reads
+ * from sparkContext.hadoopConfiguration and which are restored in afterAll.
Any other context-level key
+ * fails loudly in RuntimeConfig.set.
+ */
+ private def applySuiteConfToSharedSession(session: SparkSession): Unit = {
+ val defaults = getSparkConfForTest("Hoodie SQL Test").getAll.toMap
+ val hadoopConf = session.sparkContext.hadoopConfiguration
+ sparkConf().getAll.filterNot { case (k, v) => defaults.get(k).contains(v)
}.foreach {
+ case (k, v) if k.startsWith("spark.hadoop.") =>
+ val key = k.stripPrefix("spark.hadoop.")
+ hadoopConfOverrides :+= (key -> hadoopConf.get(key))
+ hadoopConf.set(key, v)
+ case (k, v) => session.conf.set(k, v)
Review Comment:
Right, RuntimeConfig.set only rejects static SQL confs and registered core
entries. In Spark 3.5 spark.serializer, spark.driver.host and
spark.executor.memory are registered, but spark.default.parallelism and
spark.master are not and would have landed in the session conf with no effect.
55bed9e8c7de routes hoodie.* and spark.sql.* keys to the session,
spark.hadoop.* to the shared Hadoop conf, and throws on any other spark.* key;
the scaladoc now says exactly that.
--
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]
