apache / iceberg

Apache Iceberg
https://iceberg.apache.org/
Apache License 2.0
6.08k stars 2.12k forks source link

Equality delete lost after compact data files #10312

Open CodingJun opened 3 months ago

CodingJun commented 3 months ago

Apache Iceberg version

1.5.1

Query engine

Spark

Please describe the bug 🐞

I have a program that continuously write streaming data to iceberg, and regularly use spark to compact data files. But I found that after compact the data files, some of the data was not deleted correctly. The following are the examples to reproduce:

Original table:

id value
1 a
2 b
3 c

Writing process:

Result:

id value
1 a
2 b
3 c
4 d

The correct result should be:

id value
1 a
3 c
4 d

PS:

When I set use-starting-sequence-number = false for rewriteDataFiles, Thread 1 compact data files failed at t4. stacktrace:

Caused by: org.apache.iceberg.exceptions.ValidationException: Cannot commit, found new delete for replaced data file: GenericDataFile{content=data, file_path=/var/folders/5z/dqrlv_ts0wqf36vd39bb384h0000gn/T/junit17491575750166086656/9f77fae8-d62a-426d-971f-a342b6775c44/test_schema/test_table/data/00000-2-52ae94aa-b796-4c42-bf9c-92d36c52e522-00001.parquet, file_format=PARQUET, spec_id=0, partition=PartitionData{}, record_count=1, file_size_in_bytes=407, column_sizes=null, value_counts=org.apache.iceberg.util.SerializableMap@0, null_value_counts=org.apache.iceberg.util.SerializableMap@1, nan_value_counts=org.apache.iceberg.util.SerializableMap@0, lower_bounds=org.apache.iceberg.SerializableByteBufferMap@e1782, upper_bounds=org.apache.iceberg.SerializableByteBufferMap@e1782, key_metadata=null, split_offsets=[4], equality_ids=null, sort_order_id=null}
    at org.apache.iceberg.exceptions.ValidationException.check(ValidationException.java:50)
    at org.apache.iceberg.MergingSnapshotProducer.validateNoNewDeletesForDataFiles(MergingSnapshotProducer.java:418)
    at org.apache.iceberg.MergingSnapshotProducer.validateNoNewDeletesForDataFiles(MergingSnapshotProducer.java:367)
    at org.apache.iceberg.BaseRewriteFiles.validate(BaseRewriteFiles.java:108)
    at org.apache.iceberg.SnapshotProducer.apply(SnapshotProducer.java:175)
    at org.apache.iceberg.SnapshotProducer.lambda$commit$2(SnapshotProducer.java:296)
    at org.apache.iceberg.util.Tasks$Builder.runTaskWithRetry(Tasks.java:404)
    at org.apache.iceberg.util.Tasks$Builder.runSingleThreaded(Tasks.java:214)
    at org.apache.iceberg.util.Tasks$Builder.run(Tasks.java:198)
    at org.apache.iceberg.util.Tasks$Builder.run(Tasks.java:190)
    at org.apache.iceberg.SnapshotProducer.commit(SnapshotProducer.java:295)
    at org.apache.iceberg.actions.RewriteDataFilesCommitManager.commitFileGroups(RewriteDataFilesCommitManager.java:89)
    at org.apache.iceberg.actions.RewriteDataFilesCommitManager.commitOrClean(RewriteDataFilesCommitManager.java:110)
    at org.apache.iceberg.spark.actions.RewriteDataFilesSparkAction.doExecute(RewriteDataFilesSparkAction.java:291)
    ... 8 more

Question:

Why are the equality deleted files lost? Is this correct or a bug?

lurnagao-dahua commented 3 months ago

Is there any error log for equality delete?

CodingJun commented 3 months ago

Is there any error log for equality delete?

No error, If I read directly from snapshot id: 3, the result is correct.

CodingJun commented 3 months ago

I found the code to drop the equality delete files here.

https://github.com/apache/iceberg/blob/2b21020aedb63c26295005d150c05f0a5a5f0eb2/core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java#L813-L832

I think the base.lastSequenceNumber() should be 1 at the start snapshot instead of 3.

pvary commented 3 months ago

@CodingJun: Your analysis seems correct to me. We need to take the minDataSequenceNumber and startingSequenceNumber.

@RussellSpitzer and @aokolnychyi might know more.

lurnagao-dahua commented 3 months ago

your process is in use-starting-sequence-number = true ? I test with use-starting-sequence-number = true and compact failed(apache iceberg1.4.3): Exception in thread "main" org.apache.iceberg.exceptions.ValidationException: Cannot commit, found new delete for replaced data file: GenericDataFile ...

CodingJun commented 3 months ago

your process is in use-starting-sequence-number = true ? I test with use-starting-sequence-number = true and compact failed(apache iceberg1.4.3): Exception in thread "main" org.apache.iceberg.exceptions.ValidationException: Cannot commit, found new delete for replaced data file: GenericDataFile ...

Yes, The default setting is true. Can you debug it to see if the configuration is effective?

CodingJun commented 3 months ago

Do you know if this is a bug? @RussellSpitzer @aokolnychyi

pvary commented 3 weeks ago

@szehon-ho: This is a potential bug with compaction and equality delete files. Could you please help me understand if this is a valid scenarion?

RussellSpitzer commented 3 weeks ago

The validation error looks correct. Since we shouldn't be able to do the rewrite if the equality delete has been written in the middle.

When we se using starting sequence number to false i'm not sure we are guaranteeing serializability

pvary commented 3 weeks ago

@RussellSpitzer: The compaction started at t1 and it will replace files created before t1. The equality delete is created later at t2. So if we use the same sequence number for the new files as it was for t1 then theoretically we should be able to change files and the result should be correct. Do I missunderstand how use-starting-sequence-number is handled?

szehon-ho commented 3 weeks ago

yea I think @RussellSpitzer is right, we should rely on validation error to prevent this scenario here, ie T1 should not be able to commit successfully.

i need to understand one thing:

When I set use-starting-sequence-number = false for rewriteDataFiles, Thread 1 compact data files failed at t4. stacktrace:

Caused by: org.apache.iceberg.exceptions.ValidationException: Cannot commit, found new delete for replaced data file: GenericDataFile{content=data, file_path=/var/folders/5z/dqrlv_ts0wqf36vd39bb384h0000gn/T/junit17491575750166086656/9f77fae8-d62a-426d-971f-a342b6775c44/test_schema/test_table/data/00000-2-52ae94aa-b796-4c42-bf9c-92d36c52e522-00001.parquet, file_format=PARQUET, spec_id=0, partition=PartitionData{}, record_count=1, file_size_in_bytes=407, column_sizes=null, value_counts=org.apache.iceberg.util.SerializableMap@0, null_value_counts=org.apache.iceberg.util.SerializableMap@1, nan_value_counts=org.apache.iceberg.util.SerializableMap@0, lower_bounds=org.apache.iceberg.SerializableByteBufferMap@e1782, upper_bounds=org.apache.iceberg.SerializableByteBufferMap@e1782, key_metadata=null, split_offsets=[4], equality_ids=null, sort_order_id=null} at org.apache.iceberg.exceptions.ValidationException.check(ValidationException.java:50) at org.apache.iceberg.MergingSnapshotProducer.validateNoNewDeletesForDataFiles(MergingSnapshotProducer.java:418) at org.apache.iceberg.MergingSnapshotProducer.validateNoNewDeletesForDataFiles(MergingSnapshotProducer.java:367) at org.apache.iceberg.BaseRewriteFiles.validate(BaseRewriteFiles.java:108) at org.apache.iceberg.SnapshotProducer.apply(SnapshotProducer.java:175) at org.apache.iceberg.SnapshotProducer.lambda$commit$2(SnapshotProducer.java:296) at org.apache.iceberg.util.Tasks$Builder.runTaskWithRetry(Tasks.java:404) at org.apache.iceberg.util.Tasks$Builder.runSingleThreaded(Tasks.java:214) at org.apache.iceberg.util.Tasks$Builder.run(Tasks.java:198) at org.apache.iceberg.util.Tasks$Builder.run(Tasks.java:190) at org.apache.iceberg.SnapshotProducer.commit(SnapshotProducer.java:295) at org.apache.iceberg.actions.RewriteDataFilesCommitManager.commitFileGroups(RewriteDataFilesCommitManager.java:89) at org.apache.iceberg.actions.RewriteDataFilesCommitManager.commitOrClean(RewriteDataFilesCommitManager.java:110) at org.apache.iceberg.spark.actions.RewriteDataFilesSparkAction.doExecute(RewriteDataFilesSparkAction.java:291) ... 8 more

your process is in use-starting-sequence-number = true ? I test with use-starting-sequence-number = true and compact failed(apache iceberg1.4.3): Exception in thread "main" org.apache.iceberg.exceptions.ValidationException: Cannot commit, found new delete for replaced data file: GenericDataFile ...

from above conversation it seem we get the validationException in both code-paths, isnt it?