Skip to content

【wip】[common] Keep the shared _temporary directory while other writers stage there#8862

Closed
sundapeng wants to merge 1 commit into
apache:masterfrom
sundapeng:fix/shared-temporary-staging-cleanup
Closed

【wip】[common] Keep the shared _temporary directory while other writers stage there#8862
sundapeng wants to merge 1 commit into
apache:masterfrom
sundapeng:fix/shared-temporary-staging-cleanup

Conversation

@sundapeng

@sundapeng sundapeng commented Jul 27, 2026

Copy link
Copy Markdown
Member

Purpose

RenamingTwoPhaseOutputStream stages a file at <target dir>/_temporary/.tmp.<uuid> and renames it into place on commit. TempFileCommitter#clean then deleted tempPath.getParent() — the whole _temporary directory — after committing a single file.

That directory is not private to one stream. It is the staging directory of the target directory, shared with everything else writing there, so deleting it outright throws away work that is still in flight:

  • two concurrent writers into the same directory delete each other's pending files, and the loser fails at rename with Failed to rename ...;
  • a Hadoop FileOutputCommitter job staging into the same directory loses its task output.

clean() now deletes this stream's own temporary file, and removes _temporary only once nothing is staged there any more. In the ordinary single-writer case the directory is already empty at that point and is removed exactly as before, so nothing is left behind.

Brief change log

  • RenamingTwoPhaseOutputStream.TempFileCommitter#clean: delete the temporary file, and the staging directory only when it is empty.

Tests

  • RenamingTwoPhaseOutputStreamTest#testCleanKeepsTheSharedStagingDirectory: another writer's pending file, and the directory holding it, survive clean(). Fails without the change.
  • RenamingTwoPhaseOutputStreamTest#testCleanRemovesTheStagingDirectoryOnceEmpty: an uncontended commit still leaves no _temporary behind.
  • mvn -pl paimon-common test -Dtest='RenamingTwoPhaseOutputStreamTest,MultiPartUploadTwoPhaseOutputStreamTest'

API and Format

  • This PR does not change any storage format
  • RenamingTwoPhaseOutputStream is @Public, but no signature changes and an uncontended commit behaves exactly as before; only the case where something else is staging in the same directory differs.

Documentation

  • No documentation change needed

…ge there

TempFileCommitter#clean deleted the whole '_temporary' directory of the target
directory after committing one file. That directory is shared with every other
writer: two concurrent writers into the same directory wiped each other's
pending files, and a Hadoop FileOutputCommitter job staging there lost its
output. Delete this stream's own temporary file, and the directory itself only
once nothing is staged there any more.
@sundapeng sundapeng changed the title [common] Keep the shared _temporary directory while other writers stage there 【wip】[common] Keep the shared _temporary directory while other writers stage there Jul 27, 2026
@sundapeng

Copy link
Copy Markdown
Member Author

Folded into #8861.

Reviewing this on its own surfaced why the split was wrong: keeping a non-empty _temporary alive is only safe once the read path stops treating what is inside it as data. On master that path still filters by leaf file name, so this change alone would let another job's uncommitted files reach query results — and duplicate those rows once that job commits. The two halves are one invariant (a staging directory belongs to whoever is staging there), and Apache PRs have no enforced merge order, so they now ship as one.

@sundapeng sundapeng closed this Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant