hudi-agent commented on code in PR #18834:
URL: https://github.com/apache/hudi/pull/18834#discussion_r3297010931
##########
hudi-common/src/main/java/org/apache/hudi/common/table/view/SpillableMapBasedFileSystemView.java:
##########
@@ -225,14 +225,18 @@ protected void
removeReplacedFileIdsAtInstants(Set<String> instants) {
}
@Override
- public void close() {
+ protected void closeResources() throws Exception {
+ // Close ExternalSpillableMaps (which hold RocksDB handles) while the
writeLock is held
+ // by AbstractTableFileSystemView.close(). This prevents a race where a
concurrent reader
+ // holding readLock could be mid-call in RocksDBDAO.put() when the handles
are cleared,
+ // causing a NullPointerException at RocksDB.put(null_handle, ...).
closeFileGroupsMapIfPresent();
closePendingClusteringMapIfPresent();
closePendingCompactionMapIfPresent();
closePendingLogCompactionMapIfPresent();
closeBootstrapFileMapIfPresent();
closeReplaceInstantsMapIfPresent();
- super.close();
+ super.closeResources();
Review Comment:
🤖 The fix correctly closes the spillable maps under writeLock. One thing
worth double-checking: after `super.closeResources()` runs and
`AbstractTableFileSystemView.close()` releases the writeLock, control returns
to `HoodieTableFileSystemView.close()` which then nulls out
`partitionToFileGroupsMap`/`fgIdToPendingCompaction`/etc. and finally sets
`closed = true` — all outside the lock. A new reader arriving in that window
would see `isClosed()` returning false and non-null map references, but the
underlying RocksDB handles are already released, so a `storePartitionView →
RocksDBDAO.put` call could still hit the same NPE. The window is much smaller
than before, but is closing it (e.g., moving the field nulls + a `volatile`
`closed = true` into `closeResources()` while still under writeLock) something
you'd consider here, or is that intentionally left for a follow-up?
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
