You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/05/10 01:47:52 UTC
[GitHub] [iceberg] szehon-ho opened a new pull request, #4736: WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests
szehon-ho opened a new pull request, #4736:
URL: https://github.com/apache/iceberg/pull/4736
Variation of https://github.com/apache/iceberg/pull/3457 that attempts to use time-travel on Manifest table to do the snapshot filtering. Demonstration, as it seems Manifest table does not support it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4736: WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #4736:
URL: https://github.com/apache/iceberg/pull/4736#issuecomment-1131020429
Thanks for tagging me Anton.
I think this is a good idea as well.
I won’t repeat others but I have some questions and possibly a few additional ideas but I need to go through some theoretical cases on paper first.
> One potential problem with that is that we will load the manifest list for every expired snapshot on the driver, which can become a bottleneck if we expire a lot of snapshots. I've seen such cases.
Two thoughts:
1) We should add an event / metric describing this replanning work. Could be used as a signal to perform table maintenance.
2) We might be able to track a metric to determine if we should do this initial replanning work in a distributed manner.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4736:
URL: https://github.com/apache/iceberg/pull/4736#issuecomment-2294473372
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.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #4736: WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #4736:
URL: https://github.com/apache/iceberg/pull/4736#issuecomment-1130232555
I think this is a really good idea, and I also like Anton's idea to filter manifests that are still live from the removed snapshots as well. Basically, we should be filtering the tree at every level (snapshot/manifest-list, manifest, files) before moving on to the next one.
1. Open the old metadata file and find snapshots that are no longer in the current metadata
3. Create a DF of the `manifests` table from all the expired snapshots
4. Create a DF of the `manifests` table from all the current snapshots
5. Remove any manifests from the expired set that are currently in the table (all files are still referenced)
6. Remove any manifests from the current set that have no EXISTING files (cannot contain old files). Optional because files removed and re-added would not be caught.
7. Transform the expired manifests DF to expired data files by reading each manifest
8. Transform the current manifests DF to current data files by reading each manifest
9. Remove any manifests from the expired data file set that are in the current data files set
Does that sound right? We may not want to do #6, but there is an opportunity to cut down on the number of manifests we read there by looking at the manifest file metadata.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #4736: WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on PR #4736:
URL: https://github.com/apache/iceberg/pull/4736#issuecomment-1129725672
Thanks for this complete thought, it makes sense to me. Yea I should have thought about the idea of passing a list of snapshot-ids as options. And I like the second optimization as well (expiredManifests.except(allManifests)).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #4736: WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on PR #4736:
URL: https://github.com/apache/iceberg/pull/4736#issuecomment-1176854697
Hi @aokolnychyi @rdblue , update on this discussion (see main changes in #3457). I implemented and was playing around with these optimizations in calculating the delete candidate files: (skipping valid reference_snapshot_ids, and skipping valid manifests). But I find actually that this skip :
```all_manifest table.reference_snapshot_id in set(expired snapshots ids)``` makes a positive difference. The result is the same as what I saw initially, reduce time of Spark job 30-40% in the case of expiring 1 snapshot out of many snapshots.
As each manifest in all-manifest table is now marked with snapshot id, I am thinking the first filter already kind of filters out any current reachable manifests. So implementing the skipping of valid manifests actually makes the performance worse, as it adds one more Spark read of all_manifest table and a Spark join for little gain. Unless there is a scenario I am not thinking of.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on pull request #4736: WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #4736:
URL: https://github.com/apache/iceberg/pull/4736#issuecomment-1130222468
@RussellSpitzer, could you cross-check the idea as you worked on this part in the past?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4736:
URL: https://github.com/apache/iceberg/pull/4736#issuecomment-2276927953
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 dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #4736: WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #4736:
URL: https://github.com/apache/iceberg/pull/4736#issuecomment-1130394038
> Do you refer to point 5 or 6 as optional?
You're right: it's 5 that is optional. My mistake.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on pull request #4736: WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on PR #4736:
URL: https://github.com/apache/iceberg/pull/4736#issuecomment-1130554447
The manifest skipping idea sounds good to me, basically lets us cut down on the "except" cost as well.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on pull request #4736: WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #4736:
URL: https://github.com/apache/iceberg/pull/4736#issuecomment-1130287115
@rdblue, yeah, that's what I am thinking too. Do you refer to point 5 or 6 as optional? I guess 5. The current code reads only live entries so filtering manifests without live files seems safe. What's the use case with re-added files? Won't that be covered too as the snapshot with re-added files will be also considered?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on pull request #4736: WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #4736:
URL: https://github.com/apache/iceberg/pull/4736#issuecomment-1129530885
I think this is an interesting idea. Let me summarize how I understand it.
Right now, we compute a diff between the reachability sets before and after snapshot expiry. Whenever we build the reachability set before the expiry, we read manifests of all snapshots and that seems suboptimal. The assumption is that it is sufficient to just build the reachability set for expired snapshots and compare that to the reachability set after the snapshot expiry. Did I get that right?
I guess one way to implement (and I believe this is what this PR tries to do) is to load the `FILES` metadata table for each expired snapshot and union `DataFrame`s to produce the reachability set. One potential problem with that is that we will load the manifest list for every expired snapshot on the driver, which can become a bottleneck if we expire a lot of snapshots. I've seen such cases.
An alternative idea is to add some sort of `snapshot-ids` option to `ALL_MANIFESTS` metadata table and read only manifest lists for snapshots whose ID is in that list. That way, we won't read manifest lists and manifests added after the last expired snapshot. That's already an optimization. However, we can go even further and remove still live manifests before opening them, which could give even a bigger performance boost. Suppose an expired snapshot references `manifest-1` but we know it is still live. In that case, we can skip opening it up.
Does that seem reasonable?
@szehon-ho @RussellSpitzer @flyrain @rdblue @kbendick
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4736: WIP: Improve performance of expire snapshot by not double-scanning non-expired manifests
URL: https://github.com/apache/iceberg/pull/4736
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org