-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Spark-3.5: Add procedure to compute partition stats #12451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| LOG.info("Computing partition stats for {} (branch {})", table.name(), branch); | ||
| PartitionStatisticsFile statisticsFile; | ||
| try { | ||
| statisticsFile = PartitionStatsHandler.computeAndWriteStatsFile(table, branch); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[doubt] wouldn't it be better if we do stats computation in executor, rather than just computing it in driver in a multi-threaded way ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We did benchmarks and it turns out local algorithm is more performant than distributed way.
#9437 (comment)
So, going with non-distributed way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, thank you for sharing the benchmark, just for my understanding what was the machine configuration used for running the benchmark ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was JMH benchmarks on my machine (macbook m2 max), Anton also did similar test and commented on that PR.
#9437 (comment)
api/src/main/java/org/apache/iceberg/actions/ComputePartitionStats.java
Outdated
Show resolved
Hide resolved
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions. |
|
Not stale |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
|
the branch was force pushed when this PR was closed as stale. So, Can't reopen this now. Continued on: #13480 |
Depends on #12450 (Hence kept in Draft).
Fixes: #10106
CALL catalog_name.system.compute_partition_stats('db.sample');And observe the stats files registered to table
table.partitionStatisticsFiles()