Skip to content

fix(table): avoid empty manifest errors when merging historical deleted entries #2037

Description

@michaelchendd

Proposed Change

When manifest merging is enabled, a merge bin may contain only DELETED entries from snapshots older than the snapshot currently being committed.

Today, manifestMergeManager.createManifest creates a ManifestWriter before inspecting the entries:

https://github.com/apache/iceberg-go/blob/main/table/snapshot_producers.go#L362-L407

Historical DELETED entries are intentionally not copied into the merged manifest. If every entry in the bin is such a deletion, no entries are written. Closing the already-created writer then returns:

empty manifest file has been written

This causes the commit to fail even though producing no replacement manifest appears to be a valid result for that bin.

This error has occurred intermittently during production commits. It has not yet been confirmed that this exact path caused those occurrences, but the edge case exists independently in the current implementation.

Proposed change:

  • Skip historical DELETED entries before creating the output writer.
  • Create the writer lazily when the first retained entry is encountered.
  • If no entries are retained, return no output manifest and no error.
  • Update mergeGroup to append the result only when a non-nil manifest was created.
  • Keep ManifestWriter's existing empty-manifest validation unchanged. The merge caller, rather than the general-purpose writer, should handle this case.

Correctness requirements:

  • A merge bin containing only historical DELETED entries must produce no manifest and no error.
  • DELETED entries belonging to the current snapshot must still be retained.
  • ADDED entries belonging to the current snapshot must still be written as added.
  • Older added entries and existing entries must still be written as existing.
  • Manifest read errors must continue to propagate and must not be treated as an empty merge.
  • Writers and underlying output files must still be closed on success and failure.
  • A nil manifest must never be added to the resulting manifest list.
  • ManifestWriter.Close must continue returning ErrEmptyManifest when callers directly attempt to write an empty manifest.

Validation:

Add regression tests covering:

  • a merge bin containing only historical deletions;
  • current-snapshot deletions being retained;
  • a read error occurring before an output writer is created;
  • existing non-empty merge behavior remaining unchanged.

For the historical-deletion-only case, the test should also verify that no output writer was opened.

Backward compatibility:

  • No public API changes are required.
  • Non-empty manifest merges retain their existing behavior.
  • Manifest merging disabled through table properties is unaffected.
  • The only behavior change is that an all-filtered merge bin produces no output instead of failing the commit.

Related code:

A candidate implementation and regression tests are available here:

DataDog@61d48d3

Spec reference:

Deleted manifest entries are informational and are not used when planning scans:

https://iceberg.apache.org/spec/#manifest-entry-fields

No specification change is required. This change only allows the existing historical-delete filtering behavior to produce no replacement manifest.

Willingness to contribute

  • I can contribute this improvement/feature independently
  • I would be willing to contribute this improvement/feature with guidance from the Iceberg community
  • I cannot contribute this improvement/feature at this time

Specifications

  • Table
  • View
  • REST
  • Puffin
  • Encryption
  • Other

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions