Skip to content

HIVE-29853: Iceberg CompactionEvaluator double-counts input files - #6737

Open
Aggarwal-Raghav wants to merge 1 commit into
apache:masterfrom
Aggarwal-Raghav:doubleCounting
Open

HIVE-29853: Iceberg CompactionEvaluator double-counts input files#6737
Aggarwal-Raghav wants to merge 1 commit into
apache:masterfrom
Aggarwal-Raghav:doubleCounting

Conversation

@Aggarwal-Raghav

@Aggarwal-Raghav Aggarwal-Raghav commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Removed the redundant call to addFiles() from the CompactionEvaluator constructor.

Why are the changes needed?

The addFiles() iterates through a table's Iceberg snapshot and adds files to the CommonPartitionEvaluator. Because this was being called in both the constructor and again in isEligibleForCompaction(), every file and byte was double-counted in memory.

Does this PR introduce any user-facing change?

Yes. The evaluator will now accurately respect the configured byte sizes and file count thresholds (compactor.threshold.target.size and compactor.threshold.min.input.files).

How was this patch tested?

CI outcome

@Aggarwal-Raghav Aggarwal-Raghav changed the title WIP HIVE-29853: Iceberg CompactionEvaluator double-counts input files Aug 29, 2026
@Aggarwal-Raghav

Copy link
Copy Markdown
Contributor Author

CC @difin

return false;
}

addFiles();

@Aggarwal-Raghav Aggarwal-Raghav Aug 29, 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.

addFiles is here as well. So two times counting same data files. To keep the intention of q files same I have just pumped up more inserts to incease the total file size to be eligibe for exception. The failinig q files can be checked in run 1

@Aggarwal-Raghav
Aggarwal-Raghav marked this pull request as ready for review August 29, 2026 08:50
@sonarqubecloud

Copy link
Copy Markdown

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