LuciferYang commented on PR #58004: URL: https://github.com/apache/spark/pull/58004#issuecomment-5345864439
Fixed the description and trimmed the message. Both gaps in finding 2 are now filed. **1. Description** Updated the three spots you listed, plus four more I found while checking: `SparkContext.scala:614` should have been `:623`, the `_3016` bullet claimed `sc.checkpointDir` "has exactly two writers" when `spark.checkpoint.dir` is a second entry point to the same setter rather than a second writer, the `_3020` bullet in the user-facing-change list was missing its old to new string, and the `_3017` rendered string still carried the sentence I trimmed below. I also dropped this sentence from the `_3020` bullet: "The only writer that can store `None` is `setCheckpointDir(null)`, which no production code calls, and the field is `private[spark]`." It is refutable with one grep, since `checkpointDir` is a `private[spark] var` and three mllib suites assign `None` to it directly. The reachability argument does not need it: one construction site, guarded by the `isEmpty` check in the same `synchronized` block, `cpDir` evaluated there, and one call site for the builder. **2. Both directions, filed separately** Trimmed the message to drop `, one per partition`, so it now claims only the contiguity the loop checks. That is the option you offered, and it is the honest one: the trailing-file case leaves the survivors "one per partition" while the RDD is short. The two gaps are different problems, so they are separate tickets rather than one: - [SPARK-58881](https://issues.apache.org/jira/browse/SPARK-58881) tightens the filter to `part-` followed by digits. Not `\d{5}`: `%05d` is a minimum width, so more than 100000 partitions gives `part-100000`. - [SPARK-58883](https://issues.apache.org/jira/browse/SPARK-58883) covers the truncation you found. This one is worse than the first and cannot be fixed by a filter: detecting it needs the expected partition count persisted at write time, which is a new on-disk format with a compatibility story (a directory written by an earlier version has no such file). `_partitioner` is the precedent. Keeping that out of a naming PR. **3. Nit** You are right that the old wording did not line up. The reason it stays untested is not the fake filesystem; it is that the read-back path has no original RDD to count against, which `getPartitions`' scaladoc already states. Reworded, and the write-path case (driver and executors seeing different contents) is what `local` mode cannot reproduce. -- 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]
