wgtmac commented on code in PR #890:
URL: https://github.com/apache/iceberg-cpp/pull/890#discussion_r3838949050
##########
src/iceberg/transaction.cc:
##########
@@ -375,15 +377,55 @@ Result<std::shared_ptr<Table>> Transaction::Commit() {
int32_t max_wait_ms = props.Get(TableProperties::kCommitMaxRetryWaitMs);
int32_t total_timeout_ms =
props.Get(TableProperties::kCommitTotalRetryTimeMs);
+ // Snapshot id before the commit, to detect whether this commit advanced it
(a
+ // data commit) versus a metadata-only commit that adds no snapshot.
+ const int64_t base_current_snapshot_id =
ctx_->table->metadata()->current_snapshot_id;
bool is_first_attempt = true;
+ int32_t attempt = 0;
+ std::string last_error;
auto commit_result =
MakeCommitRetryRunner(num_retries, min_wait_ms, max_wait_ms,
total_timeout_ms)
- .Run([this, &is_first_attempt]() -> Result<std::shared_ptr<Table>> {
+ .Run([this, &is_first_attempt, &attempt,
+ &last_error]() -> Result<std::shared_ptr<Table>> {
+ ++attempt;
+ // The runner only re-invokes this task when it has decided to
retry, so
+ // attempt > 1 here means a genuine retry after a retryable
failure.
+ if (attempt > 1) {
+ ICEBERG_LOG_WARN("Retrying transaction commit (attempt {})
after: {}",
+ attempt, last_error);
+ }
auto result = CommitOnce(is_first_attempt);
is_first_attempt = false;
+ if (!result.has_value()) {
+ last_error = result.error().message;
+ }
return result;
});
+ if (commit_result.has_value()) {
+ // Name the resulting snapshot only when this commit produced one (current
+ // snapshot advanced); metadata-only commits report a plain success.
+ std::string detail;
+ if (auto snapshot = commit_result.value()->metadata()->Snapshot();
+ snapshot.has_value() &&
+ snapshot.value()->snapshot_id != base_current_snapshot_id) {
Review Comment:
This can mislabel commits: a metadata-only retry may report another writer’s
snapshot, while StageOnly/ToBranch commits report none. Can we detect newly
added snapshots instead of comparing current IDs?
--
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]
