LuciferYang commented on code in PR #43578:
URL: https://github.com/apache/spark/pull/43578#discussion_r1378687555
##########
core/src/test/scala/org/apache/spark/util/TimeStampedHashMapSuite.scala:
##########
@@ -118,18 +118,18 @@ class TimeStampedHashMapSuite extends SparkFunSuite {
assert(testMap2.iterator.toSeq.head === (("k1", "v1")))
// +
- val testMap3 = testMap2 + (("k0", "v0"))
+ val testMap3 = testMap2 ++ Map("k0" -> "v0")
assert(testMap3.size === 2)
assert(testMap3.get("k1").isDefined)
assert(testMap3("k1") === "v1")
assert(testMap3.get("k0").isDefined)
assert(testMap3("k0") === "v0")
// -
- val testMap4 = testMap3 - "k0"
- assert(testMap4.size === 1)
- assert(testMap4.get("k1").isDefined)
- assert(testMap4("k1") === "v1")
+ testMap3.remove("k0")
Review Comment:
I found that `TimeStampedHashMap` is now a utility class that is no longer
used by Spark, and only test cases are still using it.
I personally have the following optional suggestions:
1. Since it is `private[spark]` visibility, perhaps we can directly delete
`TimeStampedHashMap`.
2. Or we can mark it as deprecated in Spark 4.0, suppress the compilation
warnings with @nowarn, and then remove it in Spark 4.1.
3. Refactor `TimeStampedHashMap` into an `immutable` implementation to
maintain the current usage.
cc @dongjoon-hyun FYI
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
