Skip to content

Commit 9b41c7f

Browse files
committed
fix
Signed-off-by: Yuan <yuanzhou@apache.org>
1 parent 6595e1f commit 9b41c7f

1 file changed

Lines changed: 65 additions & 45 deletions

File tree

backends-velox/src/test/scala/org/apache/gluten/backendsapi/velox/VeloxIteratorApiFsConfSuite.scala

Lines changed: 65 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ import org.apache.spark.sql.test.SharedSparkSession
2525
* Tests that [[VeloxIteratorApi.genPartitions]] captures fs.azure.*, fs.s3a.*, and fs.gs.* keys
2626
* from the driver-side Hadoop configuration and embeds them in [[GlutenPartition.fsConf]], so they
2727
* are available on executors where Spark's SQLConf propagation does not reach "fs.*" keys.
28+
*
29+
* Keys must be set on `sparkContext.hadoopConfiguration` (the mutable base configuration) because
30+
* `sessionState.newHadoopConf()` creates a fresh copy each time - mutations to its return value are
31+
* discarded before the next call.
2832
*/
2933
class VeloxIteratorApiFsConfSuite extends SharedSparkSession {
3034

@@ -38,89 +42,105 @@ class VeloxIteratorApiFsConfSuite extends SharedSparkSession {
3842
private def emptyWsCtx: WholeStageTransformContext =
3943
WholeStageTransformContext(PlanBuilder.empty())
4044

45+
/**
46+
* Set Hadoop conf keys on the underlying mutable configuration and restore their previous values
47+
* (or unset them) after the block. `sessionState.newHadoopConf()` copies from
48+
* `sparkContext.hadoopConfiguration`, so this is the correct mutation point.
49+
*/
50+
private def withHadoopConf(pairs: (String, String)*)(body: => Unit): Unit = {
51+
// scalastyle:off hadoopconfiguration
52+
val hadoopConf = spark.sparkContext.hadoopConfiguration
53+
// scalastyle:off hadoopconfiguration
54+
val prev: Seq[(String, Option[String])] = pairs.map {
55+
case (k, _) => k -> Option(hadoopConf.get(k))
56+
}
57+
pairs.foreach { case (k, v) => hadoopConf.set(k, v) }
58+
try body
59+
finally prev.foreach {
60+
case (k, Some(old)) => hadoopConf.set(k, old)
61+
case (k, None) => hadoopConf.unset(k)
62+
}
63+
}
64+
4165
test("genPartitions embeds fs.azure.* keys from Hadoop conf into GlutenPartition.fsConf") {
42-
val hadoopConf = spark.sessionState.newHadoopConf()
43-
hadoopConf.set("fs.azure.account.auth.type.myaccount.dfs.core.windows.net", "OAuth")
44-
hadoopConf.set("fs.azure.account.oauth.provider.type", "ClientCredentials")
45-
try {
66+
withHadoopConf(
67+
"fs.azure.account.auth.type.myaccount.dfs.core.windows.net" -> "OAuth",
68+
"fs.azure.account.oauth.provider.type" -> "ClientCredentials"
69+
) {
4670
val partitions = api.genPartitions(emptyWsCtx, Seq(Seq.empty), Seq.empty)
4771
assert(partitions.size == 1)
4872
val fsConf = partitions.head.asInstanceOf[GlutenPartition].fsConf
4973
assert(
5074
fsConf.contains("fs.azure.account.auth.type.myaccount.dfs.core.windows.net"),
51-
s"Expected fs.azure key not found; got: $fsConf")
52-
assert(
53-
fsConf("fs.azure.account.auth.type.myaccount.dfs.core.windows.net") == "OAuth")
75+
s"Expected fs.azure key not found; got: ${fsConf.keys.mkString(", ")}")
76+
assert(fsConf("fs.azure.account.auth.type.myaccount.dfs.core.windows.net") == "OAuth")
5477
assert(
5578
fsConf.contains("fs.azure.account.oauth.provider.type"),
56-
s"Expected fs.azure key not found; got: $fsConf")
57-
} finally {
58-
hadoopConf.unset("fs.azure.account.auth.type.myaccount.dfs.core.windows.net")
59-
hadoopConf.unset("fs.azure.account.oauth.provider.type")
79+
s"Expected fs.azure key not found; got: ${fsConf.keys.mkString(", ")}")
80+
assert(fsConf("fs.azure.account.oauth.provider.type") == "ClientCredentials")
6081
}
6182
}
6283

6384
test("genPartitions embeds fs.s3a.* keys from Hadoop conf into GlutenPartition.fsConf") {
64-
val hadoopConf = spark.sessionState.newHadoopConf()
65-
hadoopConf.set("fs.s3a.access.key", "AKIAIOSFODNN7EXAMPLE")
66-
hadoopConf.set("fs.s3a.secret.key", "wJalrXUtnFEMI")
67-
try {
85+
withHadoopConf(
86+
"fs.s3a.access.key" -> "AKIAIOSFODNN7EXAMPLE",
87+
"fs.s3a.secret.key" -> "wJalrXUtnFEMI"
88+
) {
6889
val partitions = api.genPartitions(emptyWsCtx, Seq(Seq.empty), Seq.empty)
6990
assert(partitions.size == 1)
7091
val fsConf = partitions.head.asInstanceOf[GlutenPartition].fsConf
71-
assert(fsConf.contains("fs.s3a.access.key"), s"Expected fs.s3a key not found; got: $fsConf")
92+
assert(
93+
fsConf.contains("fs.s3a.access.key"),
94+
s"Expected fs.s3a.access.key not found; got: ${fsConf.keys.mkString(", ")}")
7295
assert(fsConf("fs.s3a.access.key") == "AKIAIOSFODNN7EXAMPLE")
7396
assert(fsConf.contains("fs.s3a.secret.key"))
74-
} finally {
75-
hadoopConf.unset("fs.s3a.access.key")
76-
hadoopConf.unset("fs.s3a.secret.key")
97+
assert(fsConf("fs.s3a.secret.key") == "wJalrXUtnFEMI")
7798
}
7899
}
79100

80101
test("genPartitions embeds fs.gs.* keys from Hadoop conf into GlutenPartition.fsConf") {
81-
val hadoopConf = spark.sessionState.newHadoopConf()
82-
hadoopConf.set("fs.gs.auth.service.account.json.keyfile", "/tmp/sa.json")
83-
try {
102+
withHadoopConf("fs.gs.auth.service.account.json.keyfile" -> "/tmp/sa.json") {
84103
val partitions = api.genPartitions(emptyWsCtx, Seq(Seq.empty), Seq.empty)
85104
assert(partitions.size == 1)
86105
val fsConf = partitions.head.asInstanceOf[GlutenPartition].fsConf
87106
assert(
88107
fsConf.contains("fs.gs.auth.service.account.json.keyfile"),
89-
s"Expected fs.gs key not found; got: $fsConf")
108+
s"Expected fs.gs key not found; got: ${fsConf.keys.mkString(", ")}")
90109
assert(fsConf("fs.gs.auth.service.account.json.keyfile") == "/tmp/sa.json")
91-
} finally {
92-
hadoopConf.unset("fs.gs.auth.service.account.json.keyfile")
93110
}
94111
}
95112

96113
test("genPartitions does not include non-fs.* keys in GlutenPartition.fsConf") {
97-
val hadoopConf = spark.sessionState.newHadoopConf()
98-
hadoopConf.set("fs.s3a.access.key", "KEY")
99-
hadoopConf.set("spark.some.conf", "value")
100-
hadoopConf.set("mapreduce.input.fileinputformat.split.maxsize", "128000000")
101-
try {
114+
withHadoopConf(
115+
"fs.s3a.access.key" -> "KEY",
116+
"mapreduce.input.fileinputformat.split.maxsize" -> "128000000"
117+
) {
102118
val partitions = api.genPartitions(emptyWsCtx, Seq(Seq.empty), Seq.empty)
103119
val fsConf = partitions.head.asInstanceOf[GlutenPartition].fsConf
104-
assert(!fsConf.contains("spark.some.conf"), "Non-fs key must not appear in fsConf")
105120
assert(
106121
!fsConf.contains("mapreduce.input.fileinputformat.split.maxsize"),
107122
"Non-fs key must not appear in fsConf")
108-
assert(fsConf.contains("fs.s3a.access.key"))
109-
} finally {
110-
hadoopConf.unset("fs.s3a.access.key")
111-
hadoopConf.unset("spark.some.conf")
112-
hadoopConf.unset("mapreduce.input.fileinputformat.split.maxsize")
123+
// fs.s3a.access.key must be captured
124+
assert(
125+
fsConf.contains("fs.s3a.access.key"),
126+
s"Expected fs.s3a.access.key not found; got: ${fsConf.keys.mkString(", ")}")
113127
}
114128
}
115129

116-
test("genPartitions produces empty fsConf when no fs.* keys are set") {
117-
// Use a key guaranteed not to exist in the Hadoop conf under any test profile.
118-
val hadoopConf = spark.sessionState.newHadoopConf()
119-
val uniqueKey = "fs.azure.__test_only_unique_key__"
120-
hadoopConf.unset(uniqueKey)
121-
// Count only keys matching our prefixes.
122-
val partitions = api.genPartitions(emptyWsCtx, Seq(Seq.empty), Seq.empty)
123-
val fsConf = partitions.head.asInstanceOf[GlutenPartition].fsConf
124-
assert(!fsConf.contains(uniqueKey))
130+
test("genPartitions produces one partition per input split group") {
131+
withHadoopConf("fs.s3a.endpoint" -> "s3.amazonaws.com") {
132+
// Two split groups => two GlutenPartitions
133+
val partitions =
134+
api.genPartitions(emptyWsCtx, Seq(Seq.empty, Seq.empty), Seq.empty)
135+
assert(partitions.size == 2)
136+
partitions.foreach {
137+
p =>
138+
val fsConf = p.asInstanceOf[GlutenPartition].fsConf
139+
assert(
140+
fsConf.contains("fs.s3a.endpoint"),
141+
s"Expected fs.s3a.endpoint not found; got: ${fsConf.keys.mkString(", ")}")
142+
assert(fsConf("fs.s3a.endpoint") == "s3.amazonaws.com")
143+
}
144+
}
125145
}
126146
}

0 commit comments

Comments
 (0)