mzzz-zzm commented on code in PR #982:
URL: https://github.com/apache/iceberg-go/pull/982#discussion_r3202226525
##########
table/table.go:
##########
@@ -483,6 +490,19 @@ func (t Table) doCommit(ctx context.Context, updates
[]Update, reqs []Requiremen
return nil, err
}
+ // Delete manifest-list files that were written during failed retry
+ // attempts and have now been superseded by the committed rebuild.
+ // These are orphaned objects that will never be referenced again.
+ if len(orphanedManifests) > 0 {
Review Comment:
The explicit post-loop cleanup block is replaced with a `defer` guarded by a
`cleanupOrphans` flag (default `true`):
```go
cleanupOrphans := true
defer func() {
if !cleanupOrphans || len(orphanedManifests) == 0 {
return
}
for _, path := range orphanedManifests {
if removeErr := wfs.Remove(path); removeErr != nil {
log.Printf("Warning: failed to delete orphaned manifest list %s:
%v", path, removeErr)
}
}
}()
```
The defer fires on every exit path. `cleanupOrphans = false` is set only for
the non-`ErrCommitFailed` `CommitTable` error (5xx / unknown-state), where one
of the orphaned files may actually be the snapshot the catalog silently
accepted:
```go
if !errors.Is(err, ErrCommitFailed) {
cleanupOrphans = false // outcome unknown; catalog may have accepted an
orphan
return nil, err
}
```
All other exits — success, `ErrCommitDiverged`, validator errors, context
cancellation — leave `cleanupOrphans = true` and the defer removes the orphaned
files.
Tests added: `TestDoCommit_OrphanCleanedOnSuccess`,
`TestDoCommit_OrphanNotCleanedOnUnknownError`,
`TestDoCommit_OrphanCleanedOnCommitDiverged`,
`TestDoCommit_OrphanCleanedOnRetriesExhausted` (every attempt fails with
`ErrCommitFailed` until budget is exhausted — defer fires with
`cleanupOrphans=true`; orphans removed).
--
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]
