Hi Yufei, I was wondering if we also want to support the use case of moving tables in this proposal? For example, users might have various reasons to change the table location, however, there is no good way to move original data files to the new location unless we are doing data files rewrite, but it seems that we are misusing the functionality.
On Wed, Jul 10, 2024 at 9:37 AM Ajantha Bhat <ajanthab...@gmail.com> wrote: > For RemoveExpiredFiles, I'm admittedly a bit skeptical if it's required >> since orphan file removal should be able to cleanup the files in the >> copied table. Are we able to elaborate why there's a concern with removing >> snapshots on the copied table and subsequently relying on orphan file >> removal on the copied table to remove the actual files? Is it around >> listing? > > > I have the same concern as Amogh. I already mentioned the same thing in > the PR yesterday > <https://github.com/apache/iceberg/pull/10643#discussion_r1669739401>. > I suggested renaming it as *RemoveTableCopyOrphanFiles. *Thinking more on > this today, I think we should atomically (implicitly) handle cleaning up of > orphan files as part of copy table action instead of a separate action. > > Also, very happy to see the progress on this one. This will help users to > move the data from one location to another seamlessly. > > - Ajantha > > > On Wed, Jul 10, 2024 at 7:35 AM Amogh Jahagirdar <2am...@gmail.com> wrote: > >> Thanks Yufei! >> >> +1 on having a copy table action, I think that's pretty valuable. I have >> some ideas on interfaces based on previous work I've done for >> region/multi-cloud replication of Iceberg tables. The absolute vs relative >> path discussion is interesting, I have some questions on how relative >> pathing would look like but I'll wait for Anurag's input. >> >> On CheckSnapshotIntegrity, I think I'd probably advocate for having a >> more general "Repair Metadata" procedure. Currently, it looks like >> CheckSnapshotIntegrity just tells a user what files are missing in its >> output. I think we could go a step further and attempt to handle cases >> where a manifest entry refers to a file which no longer exists. We could >> attempt a recovery of that file if the fileIO implementation supports that >> via some sort of a SupportsRecovery mixin. There's also another corruption >> case where duplicate file entries end up in manifests, we can define an >> approach on reconciling that and write out new manifests. >> There's actually been two attempts on this, one from Szehon quite a while >> back https://github.com/apache/iceberg/pull/2608 and another more >> recently from Matt https://github.com/apache/iceberg/pull/10445 . >> Perhaps we could review both of these and figure out a path forward for >> this? >> For just verifying the integrity of the copy table, we could have a dry >> run option for the repair metadata operation which would output any missing >> files, or manifests with duplicates without performing any recovery/fixing >> up. >> >> For RemoveExpiredFiles, I'm admittedly a bit skeptical if it's required >> since orphan file removal should be able to cleanup the files in the >> copied table. Are we able to elaborate why there's a concern with removing >> snapshots on the copied table and subsequently relying on orphan file >> removal on the copied table to remove the actual files? Is it around >> listing? >> >> Overall this is great to see. >> >> Thanks, >> Amogh Jahagirdar >> >> >> >> >> On Tue, Jul 9, 2024 at 10:59 AM Anurag Mantripragada >> <amantriprag...@apple.com.invalid> wrote: >> >>> Agreed with Peter. I will bring relative paths changes up in the next >>> community sync. I will help drive this. >>> >>> >>> ~ Anurag Mantripragada >>> >>> >>> >>> >>> >>> >>> On Jul 8, 2024, at 10:50 PM, Péter Váry <peter.vary.apa...@gmail.com> >>> wrote: >>> >>> I think in most cases the copy table action doesn't require a query >>> engine to read and generate the new metadata files. This means, that it >>> would be nice to provide a pure Java implementation in the core, and it >>> could be extended/reused by different engines, like Spark, to execute it in >>> a distributed manner, when distributed execution is needed. >>> >>> About the copy vs. relative path debate: >>> - I have seen the relative path requirement coming up multiple times in >>> the past. Seems like a feature requested by multiple users, so I think it >>> would be the best to discuss it in a different thread. The Copy Table >>> Action might be used to move absolute path tables to relative path tables >>> when migration is needed. >>> >>> On Mon, Jul 8, 2024, 21:52 Anurag Mantripragada >>> <amantriprag...@apple.com.invalid> wrote: >>> >>>> Hi Yufei. >>>> >>>> Thanks for the proposal. While the actions are great, they still need >>>> to do a lot of work which can be reduced if we have the relative path >>>> changes. I still support adding these actions as moving data was out of >>>> scope for the relative path design and we can use these actions as helpers >>>> when the spec change is done. >>>> >>>> Anurag Mantripragada >>>> >>>> On Jul 8, 2024, at 10:55 AM, Pucheng Yang <pucheng.yo...@gmail.com> >>>> wrote: >>>> >>>> Thanks for picking this up, I think this is a very valuable addition. >>>> >>>> On Mon, Jul 8, 2024 at 10:48 AM Yufei Gu <flyrain...@gmail.com> wrote: >>>> >>>>> Hi folks, >>>>> >>>>> I'd like to share a recent progress of adding actions to copy tables >>>>> across different places. >>>>> >>>>> There is a constant need to copy tables across different places for >>>>> purposes such as disaster recovery and testing. Due to the absolute file >>>>> paths in Iceberg metadata, it doesn't work automatically. There are three >>>>> generic solutions: >>>>> 1. Rebuild the metadata: This is a proven approach widely used across >>>>> various companies. >>>>> 2. S3 access point: Effective when both the source and target >>>>> locations are in S3, but not applicable to other storage systems. >>>>> 3. Relative path: It requires changes to the table specification. >>>>> >>>>> We focus on the first approach in this thread. While the code has been >>>>> shared 2 years ago here <https://github.com/apache/iceberg/pull/4705>, >>>>> it has never been merged. We picked it up recently. Here are the active >>>>> PRs >>>>> related to this action. Would really appreciate any feedback and review: >>>>> >>>>> - PR to add CopyTable action: >>>>> https://github.com/apache/iceberg/pull/10024 >>>>> - PR to add CheckSnapshotIntegrity action: >>>>> https://github.com/apache/iceberg/pull/10642 >>>>> - PR to add RemoveExpiredFiles action: >>>>> https://github.com/apache/iceberg/pull/10643 >>>>> >>>>> Here is a google doc with more details to clarify the goals and >>>>> approach: >>>>> https://docs.google.com/document/d/15oPj7ylgWQG8bhk_5aTjzHl7mlc-9f4OAH-oEpKavSc/edit?usp=sharing >>>>> >>>>> Yufei >>>>> >>>> >>>> >>>
