SteNicholas commented on code in PR #3718:
URL: https://github.com/apache/celeborn/pull/3718#discussion_r3354577861
##########
client/src/main/scala/org/apache/celeborn/client/commit/MapPartitionCommitHandler.scala:
##########
@@ -76,6 +76,19 @@ class MapPartitionCommitHandler(
// shuffleId -> boolean, records whether the shuffle is visible at the
segment level, facilitating future optimization of worker read and write
processes
private val shuffleIsSegmentGranularityVisible =
JavaUtils.newConcurrentHashMap[Int, Boolean]
+ private val shuffleIntegrityCheckEnabled =
conf.clientShuffleIntegrityCheckEnabled
+
+ // Write-side per-subpartition checksums of one finished map partition
(indexed by subpartition).
+ private case class MapPartitionWriteMetadata(crc32: Array[Int],
bytesWritten: Array[Long]) {
+ require(
+ crc32.length == bytesWritten.length,
+ s"crc32 length ${crc32.length} != bytesWritten length
${bytesWritten.length}")
+ }
+
+ // shuffleId -> (mapPartitionId -> write-side metadata).
+ private val commitMetadataForMapPartition =
Review Comment:
@xumingming, Good question. A few points on the footprint:
- It's gated behind `celeborn.client.shuffle.integrityCheck.enabled`
(default `false`), so the default JobManager footprint is unchanged.
- Each retained entry is small: a `crc32` int (4B) + a `bytesWritten` long
(8B) per subpartition, i.e. ~12·N bytes per map partition and ~12·M·N bytes
total (e.g. M=N=4096 ≈ 150MB).
- It's bounded by the shuffle lifetime and cleared in `removeExpiredShuffle`
(line 149), not accumulated across shuffles.
- The per-subpartition granularity is required: a reader consumes an
arbitrary `[startSubIndex, endSubIndex]` range and we combine the write-side
checksums over exactly that range, so we can't pre-aggregate. This is the same
O(M·N) the write side already produces at `MapperEnd`; the new cost is the
driver *retaining* it until commit/expiry.
So when enabled it is genuinely O(M·N), but it's opt-in and short-lived. If
it turns out to be a concern for very large jobs, a follow-up could compact it.
WDYT?
--
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]
