liucao-dd commented on PR #18008:
URL: https://github.com/apache/iceberg/pull/18008#issuecomment-6176191070

   Thanks for identifying the root cause and proposing a fix, but I suggest a 
different approach.
   
   I see three problems with the current fallback approach:
   
   1. The catch is too broad. shouldProcess also throws IllegalStateException 
for delete and overwrite snapshots. The fallback hides these errors.
   2. latestOffset() can return a replace snapshot or a skipped snapshot as the 
cap. The planner never stops at that snapshot. The query can then read new 
appends and not stop.
   3. The cause stays. The cap still comes from initialOffset, and that offset 
can expire.
   
   I propose to compute the cap from the real start offset. 
prepareForTriggerAvailableNow() sets a flag only. The first 
latestOffset(startOffset, limit) call computes the cap. In that call, Spark 
gives the committed start offset.
   
   Spark Kafka and Delta use this pattern. In 
[KafkaMicroBatchStream](https://github.com/apache/spark/blob/master/connector/kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/KafkaMicroBatchStream.scala),
 prepareForTriggerAvailableNow() sets a flag. The first latestOffset then 
computes the cap from the start that Spark gives.
   
   Delta had the same eager pattern as Iceberg, and the cap was incorrect on 
restart. Delta fixed this in 
[29d3a09](https://github.com/delta-io/delta/commit/29d3a09289f5d799714df448ea9818d2fb5fcdd7).
 The fix moves the cap computation into the first latestOffset or getBatch 
call, which has the real start offset.
   
   With this approach, the initial offset does not have to stay available. 
Happy to open a different PR if desired by the original author


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to