[MINOR] Make HoodieTable AutoCloseable and close per-cycle tables in BaseHoodieTableServiceClient#19042
Open
Davis-Zhang-Onehouse wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19042 +/- ##
=============================================
- Coverage 68.78% 51.04% -17.74%
+ Complexity 29136 20833 -8303
=============================================
Files 2515 2457 -58
Lines 139938 133008 -6930
Branches 17187 15628 -1559
=============================================
- Hits 96260 67898 -28362
- Misses 35902 59726 +23824
+ Partials 7776 5384 -2392
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
255eaf2 to
005f406
Compare
…tem views A HoodieTable created per table-service work unit on the driver lazily builds a FileSystemViewManager. For a SPILLABLE_DISK view the file-group store spills to an on-disk BitCaskDiskMap. HoodieTable had no close() and the per-cycle tables created in BaseHoodieTableServiceClient were never closed, so their cached views' on-disk maps lingered until JVM exit. - HoodieTable now implements AutoCloseable; close() releases the metadata reader and, for locally-managed views (SPILLABLE_DISK / MEMORY / EMBEDDED_KV), the FileSystemViewManager. For REMOTE_ONLY/REMOTE_FIRST views close() leaves the view manager alone: that view talks to an embedded timeline server shared with the write client and later table-service cycles, so tearing it down here would break them. - Every create-use-drop HoodieTable in BaseHoodieTableServiceClient is wrapped in try-with-resources / try-finally (preCommit, logCompact, compact, cluster, scheduleTableServiceInternal, scheduleCleaning, purgePendingClustering, clean, rollbackFailedIndexingCommits, rollback, rollbackFailedBootstrap). - SpillableMapBasedFileSystemView drains its ExternalSpillableMaps from closeResources() (under the AbstractTableFileSystemView writeLock) instead of close(), so the on-disk maps are released while still referenced and without racing a concurrent reader. - Adds TestHoodieTableResourceLifecycle verifying the on-disk BitCaskDiskMap is released by HoodieTable.close(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
005f406 to
839de89
Compare
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change Logs
A
HoodieTablecreated per table-service work unit on the driver lazily builds aFileSystemViewManager. For aSPILLABLE_DISKview the file-group store spills to an on-diskBitCaskDiskMap(which registers a JVM shutdown hook).HoodieTablehad noclose(), and the per-cycle tables created inBaseHoodieTableServiceClientwere never closed, so their cached views' on-disk maps lingered until JVM exit.HoodieTablenow implementsAutoCloseable;close()releases theFileSystemViewManagerand the table metadata reader (idempotent, never throws).HoodieTableinBaseHoodieTableServiceClientis wrapped in try-with-resources / try-finally:preCommit,logCompact,compact,cluster,scheduleTableServiceInternal,scheduleCleaning,purgePendingClustering,clean,rollbackFailedIndexingCommits,rollback,rollbackFailedBootstrap. (commitCompaction/commitLogCompactionare left alone — they receive anOption<HoodieTable>that may be caller-owned.)SpillableMapBasedFileSystemViewnow drains itsExternalSpillableMaps fromcloseResources()— under theAbstractTableFileSystemViewwriteLock— instead of fromclose()before the lock. This releases the on-disk maps while still referenced and without racing a concurrent reader.TestHoodieTableResourceLifecycleverifying thatHoodieTable.close()releases the spillable view's on-diskBitCaskDiskMap.Impact
Promptly frees per-cycle driver-side file-system view resources instead of leaving them until JVM exit. No behavior change to table-service results.
Risk level: low
Lifecycle change covered by existing file-system-view close tests plus the new test.
Companion internal PR
This is kept code-consistent with the Onehouse-internal companion (onehouseinc/hudi-internal#1959). The internal repo additionally needs the
closeResources()ordering fix as an actual bug fix (itsSpillableMapBasedFileSystemViewoverridescloseResources()and was callingsuperfirst, nulling the map fields before draining them); here the same drain-before-super ordering is applied for the writeLock benefit.Documentation Update
None.
Contributor's checklist