-
Notifications
You must be signed in to change notification settings - Fork 4.8k
HIVE-29417: Basic Iceberg table MSCK repair #6273
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
base: master
Are you sure you want to change the base?
Conversation
aa597e8 to
51dd092
Compare
51dd092 to
9262146
Compare
9262146 to
bb38578
Compare
bb38578 to
d5f568c
Compare
d5f568c to
7602e32
Compare
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java
Outdated
Show resolved
Hide resolved
|
Do we need to add this new action in |
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java
Outdated
Show resolved
Hide resolved
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckResult.java
Outdated
Show resolved
Hide resolved
|
Otherwise LGTM |
|
Thanks for the review, @Aggarwal-Raghav! I’ve been tied up with other tasks, I’ll start addressing the comments. |
i don't think so, msck is a manual cmd, user can execute it on demand |
7602e32 to
740d4c1
Compare
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.
Thanx @deniskuzZ for the initiative
Couple of questions:
- Is this operation reversible, like tmrw, I land up restoring my files, can I rollback to the previous snapshots and kind of restore my table to original?
- What happens if the Manifest* files go missing? How do we repair that.
- We handle the
DataFilemissing scenario but forDeleteFiles/DV's? - Once a
DataFileis dropped what about theDeleteFIles/DV'sassociated with it? - I just skimmed over the implementation, we are doing
planTaskon the entire table, is that batched? I am doubtful like whether at scale it will lead toOOMkind of stuff within the HS2 - Did you explore rather than operating on the main table, rather get the entires from the
ALL_FILESmetadata table or some other relevant. - Should we like have a split batch as well, like you found 1K files missing lets hold -> commit & start again, like to avoid memory pressure.
- Fundamentally, I am not sure we should discuss whether this should be within
MSCKor an independent command withinALTER TABLE EXECUTE <SOME FANCY Thing>. MSCK I believe was to fix the inconsistency b/w the Metadata & Actual Data, like you ingested data or so. This is like fixing the Metadata post a Data Loss, this is bit debatable though, different people different but still we should maybe think once. I know Spark handles such things via there DeleteFile Action or some action they have, but it doesn't find the missing ones on its own
If any of the cases are handled we should extend the tests, If not already, (I had a very quick pass)
Yes, the operation is reversible. The repair operation creates a new snapshot with updated manifests that remove references to missing data files. Iceberg maintains a complete snapshot history, allowing you to rollback to any previous snapshot.
This PR does not address missing manifest files. The current implementation follows the same basic repair functionality that Impala already implemented (commit fdad9d32041a736108b876704bd0354090a88d29), which focuses on detecting and removing references to missing data files. Missing manifest files represent a more severe form of corruption that would require reconstructing metadata from available manifests or data files, which is beyond the scope of this basic repair functionality.
The repair operation cannot proceed if there are missing delete files. This is a limitation of Iceberg's DeleteFiles API, which only allows removing data files, not delete files or deletion vectors. This aligns with the Impala implementation's scope and the fundamental constraints of the Iceberg DeleteFiles API.
The planFiles() method returns a CloseableIterable, which is a lazy iterator that does not load all files into memory at once. This design prevents OOM issues even for very large tables.
The repair operation needs to check files referenced in the current table snapshot, which planFiles() provides directly
Batching commits is not necessary for this implementation. The operation only stores file paths (strings) in memory, not file contents or metadata, making the memory footprint minimal.
Integrating repair functionality into MSCK REPAIR TABLE is the appropriate design choice for Hive, as it aligns with MSCK's core purpose of synchronizing metadata with existing data files. MSCK is designed to repair inconsistencies between the Hive Metastore and the actual data files in storage. |
740d4c1 to
e063a9e
Compare
|




What changes were proposed in this pull request?
Remove dangling file references for the missing files
Why are the changes needed?
Addresses java.io.FileNotFoundException: File does not exist during table reads
Does this PR introduce any user-facing change?
No
How was this patch tested?
mvn test -Dtest=TestIcebergCliDriver -Dqfile=iceberg_msck_repair.q
mvn test -Dtest=TestHiveIcebergRepairTable