Skip to content

[CELEBORN-2439][TEZ] Report accurate partition statistics for shuffle output - #3821

Open
SparksFyz wants to merge 1 commit into
apache:mainfrom
SparksFyz:fyz/CELEBORN-2439
Open

[CELEBORN-2439][TEZ] Report accurate partition statistics for shuffle output#3821
SparksFyz wants to merge 1 commit into
apache:mainfrom
SparksFyz:fyz/CELEBORN-2439

Conversation

@SparksFyz

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This pull request reports accurate per-partition size statistics for Celeborn Tez shuffle output.

The main changes are:

  • Retain the partition byte counters maintained by DataPusher in CelebornTezWriter.
  • Account for the bytes returned by mergeData.
  • Use the same statistics source for ordered and unordered shuffle.
  • Remove the incorrect bytesPerPartition counters from CelebornSortBasedPusher.
  • Preserve the existing behavior when partition statistics reporting is disabled.

Why are the changes needed?

The current Tez shuffle implementation reports incorrect partition statistics:

  • Ordered shuffle reports zero for every partition because the inherited partitionStats array is never populated.
  • Unordered shuffle reports the number of records instead of bytes because bytesPerPartition is incremented once per record.

These incorrect statistics may affect Tez partition-size estimation and reducer parallelism decisions.

Does this PR resolve a correctness bug?

  • Yes

Does this PR introduce any user-facing change?

  • Yes

How was this patch tested?

Added CelebornTezWriterSuiteJ to verify:

  • Partition statistics snapshot behavior.
  • mergeData accounting using the bytes returned by ShuffleClient.
  • Asynchronous pushData accounting after the writer is closed.
  • The close order of DataPusher.waitOnTermination, pushMergedData, and mapperEnd.

@SteNicholas SteNicholas changed the title [CELEBORN-2439][TEZ] Report accurate partition statistics for shuffle… [CELEBORN-2439][TEZ] Report accurate partition statistics for shuffle output Aug 24, 2026
import java.util.concurrent.atomic.LongAdder;

import org.junit.Test;
import org.mockito.InOrder;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add focused coverage for the two Tez consumers changed in this patch? This suite verifies the counters inside CelebornTezWriter, but it would still pass if CelebornSorter or CelebornUnorderedPartitionedKVWriter failed to expose the snapshot, or if disabled partition-stat reporting returned a non-null array. Exercising enabled and disabled TEZ_RUNTIME_REPORT_PARTITION_STATS through both paths would protect the user-visible behavior this PR is fixing.

@SparksFyz SparksFyz Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SteNicholas Thanks for the review. I added focused tests for both CelebornSorter and CelebornUnorderedPartitionedKVWriter, covering explicit PRECISE, the default MEMORY_OPTIMIZED, and disabled (NONE) partition-stat reporting. The tests verify that enabled reporting exposes the snapshot after CelebornTezWriter is closed, while disabled reporting returns null without querying the writer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants