search

NVIDIA / spark-rapids

Spark RAPIDS plugin - accelerate Apache Spark with GPUs
https://nvidia.github.io/spark-rapids
Apache License 2.0
783 stars 228 forks source link

[AUDIT] Evaluate if SPARK-46957's shuffle cleanup fix is needed in the plugin #11181

Closed mythrocks closed 1 week ago

mythrocks commented 1 month ago

This concerns the shuffle cleanup fix introduced in SPARK-46957. Per description:

This is a long-standing bug in decommission where the migrated shuffle files can't be cleaned up from the executor. Normally, the shuffle files are tracked by taskIdMapsForShuffle during the map task execution. Upon receiving the RemoveShuffle(shuffleId) request from driver, executor can clean up those shuffle files by searching taskIdMapsForShuffle. However, for the migrated shuffle files by decommission, they lose the track in the destination executor's taskIdMapsForShuffle and can't be deleted as a result.

This might need evaluation by someone familiar with Spark Shuffle. It appears to be a missed condition where shuffle files might not be cleaned up.

abellina commented 1 month ago

I'll take a look @mythrocks

abellina commented 1 week ago

Sorry this took forever. I don't think we need to make this change. We don't instantiate our own IndexShuffleBlockResolver, instead we use whatever was there already for the MT shuffle path, so we'd be OK there.

For the UCX path, we don't support dynamic allocation/migration or anything like that. There is plenty broken if we were told to migrate data from executor a to executor b today.