SteNicholas commented on code in PR #213:
URL: https://github.com/apache/paimon-cpp/pull/213#discussion_r3840316081
##########
src/paimon/core/operation/file_store_commit_impl.cpp:
##########
@@ -931,6 +936,30 @@ Result<int64_t> FileStoreCommitImpl::CommitWithProgress(
std::shared_ptr<ManifestCommittable> committable =
CreateManifestCommittable(identifier, commit_messages, watermark,
/*properties=*/{});
+ PAIMON_ASSIGN_OR_RAISE(std::vector<std::shared_ptr<ManifestCommittable>>
pending_committables,
+ FilterCommitted({committable}));
+ const bool identifier_committed = pending_committables.empty();
Review Comment:
`FilterCommitted` only compares this identifier with the latest identifier
for the commit user; it does not establish that this exact `(commit_user,
commit_identifier)` produced a snapshot. If commit A succeeds but its response
is lost, and a later commit B from the same user succeeds before A is retried,
this branch can classify A as committed merely because B has a higher
identifier. When A’s ranges are covered it then returns the latest snapshot (B)
at line 959, despite the public API promising the snapshot produced by this
commit. Please look up the snapshot for the exact identity and return its ID;
if the ranges were committed by another identity, reject the retry.
--
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]
