Skip to content

[core] Continue FileStoreWrite cleanup after writer close failures - #10288

Open
PDGGK wants to merge 1 commit into
apache:masterfrom
PDGGK:fix-filestore-cleanup-after-close-failure
Open

PDGGK wants to merge 1 commit into
apache:masterfrom
PDGGK:fix-filestore-cleanup-after-close-failure

Conversation

@PDGGK

@PDGGK PDGGK commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Purpose

#9227 made AbstractFileStoreWrite.close() close every writer and shut down its executors before rethrowing the first writer failure. The subclasses above it still call super.close() first and release their own resources afterwards, so that rethrown failure skips:

  • MemoryFileStoreWrite: the writer buffer metric group
  • BaseAppendFileStoreWrite: the blob fetch metrics
  • KeyValueFileStoreWrite: the compact manager factory, whose close() invalidates the lookup file cache

These classes back both append-only and primary-key tables.

What changes

Close the parent and the subclass resource with IOUtils.closeAll, the helper #9227 uses. The first failure is rethrown with later ones attached as suppressed. An Error still propagates immediately.

Tests

AbstractFileStoreWriteCloseTest adds an append-only and a primary-key case in which a writer fails to close. Both fail on master because the outer cleanup never runs, and both pass with the fix. Reverting any one of the three classes fails its case. BucketedAppendFileStoreWriteTest and KeyValueFileStoreWriteTest pass; spotless and checkstyle are clean.

API and Format

No change.

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