aiborodin commented on code in PR #14312:
URL: https://github.com/apache/iceberg/pull/14312#discussion_r2605871302
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/DynamicCommitter.java:
##########
@@ -119,6 +110,11 @@ public void
commit(Collection<CommitRequest<DynamicCommittable>> commitRequests)
return;
}
+ /*
+ TODO: Replace List<CommitRequest<DynamicCommittable>> with a single
CommitRequest<DynamicCommittable>
Review Comment:
I updated the comment.
> We could still have multiple DynamicCommittable for a checkpoint - when we
have incoming data for multiple branches. So we still need to have a list. Am I
right?
Each branch would have a separate incoming commit request because commit
requests are grouped by `TableKey` (table and branch). There must be only one
commit request per TableKey (table and branch).
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/DynamicCommitter.java:
##########
@@ -155,6 +152,7 @@ public void
commit(Collection<CommitRequest<DynamicCommittable>> commitRequests)
}
}
+ /** TODO: Reuse {@link
org.apache.iceberg.flink.sink.SinkUtil#getMaxCommittedCheckpointId} * */
Review Comment:
I will remove this duplication when porting
https://github.com/apache/iceberg/pull/14517 to the regular `IcebergSink`.
--
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]
