openinx commented on issue #2308: URL: https://github.com/apache/iceberg/issues/2308#issuecomment-800021752
@RussellSpitzer If we just fail the rewrite operation as @stevenzwu said, Yes it should be no data semantic issue. The failure could be acceptable for batch jobs, I mean we could just fail the rewrite operation if someone write few new update/deletes into the base iceberg table, the batch update/deletes job is a rare operation, the probability of conflict between rewrite operation and update operation is small. But in streaming case, that is a different story. Because the streaming job is always running and continues to commit the delta changes to iceberg table periodlly ( such as 1 minutes), the probability of conflict between rewrite operation and update operation is very large. So failing the rewrite operation and re-do the whole expensive job is unacceptable for this case. I'm trying to propose the solution that we don't have to re-do the rewrite operation, the simplest way is following the current opportunistic concurrency control, I mean the rewrite operation could just grab the table lock again and re-commit the generated files to iceberg table with the original sequence number from base snapshot, then the delete files will always effect the rewritten data files. But it does not work for the position delete files because once we rewrite the data files then all of the row offset are changed, finally the delete files could not be applied to the rewrite files. Luckily, we st reaming update/upsert job won't produce any position delete files that delete the old data files, so that should be OK. ( btw, I provided an unit test to demonstrate why the data semantics will be an issue if just try to reuse the files from rewrite operation here https://github.com/apache/iceberg/pull/2303/commits/27e47a95d2761fa36d7724f8eac21b75b91f280c#diff-e573276c8dbbcd32f174f0e778334aeea106c67a19efabfefedf0afa4779a499R155-R218, the [line](https://github.com/apache/iceberg/pull/2303/commits/27e47a95d2761fa36d7724f8eac21b75b91f280c#diff-e573276c8dbbcd32f174f0e778334aeea106c67a19efabfefedf0afa4779a499R217) will be broken because the rewrite action makes the `<2, B>` visible again. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
