hudi-bot opened a new issue, #16631: URL: https://github.com/apache/hudi/issues/16631
Our current logic for rollbacks is, we first generate commit times for ingestion in-memory and then check for any pending commits to rollback. If there are any, then we trigger rollbacks. So, what this means is that, in timeline, we could see some out of ordered timestamp. It may not have any material impact, but would be nice to get this resolved. Say, we have t1.dc, t2.dc and t2.dc crashed mid way. Then we start t5.dc say. just when we start t5.dc, hudi detects pending commit and triggers a rollback. And this rollback will get an *instant time of t6 (t6.rb). Note that rollback's commit time is greater than t5 or current ongoing delta commit.* So, once rollback completes, we proceed on w/ ingestion writes. So, finally this is how the timeline might look like. t1.dc.req, t1.dc.inflight, t1.dc.completed, t6.rb.req, t6.rb.inflight, t6.rb.completed, t5dc.req, t5.dc.inflight, t5.dc.completed So, why not we fix the timestamp generation logic to ensure we try to keep it monotonically increasing atleast for single writer. Note: https://issues.apache.org/jira/browse/HUDI-8248 already fixes the above data consistency issue by fixing the log record reading logic. So, this Jira is trying to make an attempt to see if we can fix the commit time generation logic as well. But fix for 8248 alone suffice to ensure there won't be any data consistency issues. ## JIRA info - Link: https://issues.apache.org/jira/browse/HUDI-8249 - Type: Improvement - Fix version(s): - 0.16.0 - 1.1.0 --- ## Comments 24/Sep/24 00:44;shivnarayan;High level, we are thinking do we even need to fix this bcoz, of the complexities it might bring and there is not really a lot of ROI we get out of it. With 1.x, we are anyway moving to completion time based querying across the board. So, having rollback commit times strictly lesser than the current on-going commit might not matter. But still, lets go ahead and discuss how we can solve this if we align to go ahead with it. Approach A: introduce a lazy time generation for ingestion writes. Challenges: - In Deltastreamer writer use-cases, we use the instant time to prepare records (auto record key generation) which is triggered before rollbacks can kick in. So, we can't really make the instant time generation for ingestion writes lazy. Approach B: Make rollback instant time as an argument to startCommit() method in BaseHoodieWriteClient. Challenge: We could have multiple writes that could have failed. So, we can't really get this done elegantly as well. Approach C: We can make rollbackFailedWrites a separate public method in Writeclient. And expect every writer to call into it before calling start Commit. This is the only viable approach. but every writer(batch writers and streaming writer) needs to make sure that they call rollback failed writes before starting a new commit which again could leave traces for future bugs if some new writer comes up(in future). Given all these, I feel we should leave the rollback commit time generation logic untouched. But open to hear thoughts if anyone else have compelling reasons. ;;; -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
