Skip to content

Fix FSDirectory resource leak when IndexWriter construction fails in HNSW vector index#18964

Merged
Jackie-Jiang merged 3 commits into
apache:masterfrom
Akanksha-kedia:fix/hnsw-directory-resource-leak
Jul 16, 2026
Merged

Fix FSDirectory resource leak when IndexWriter construction fails in HNSW vector index#18964
Jackie-Jiang merged 3 commits into
apache:masterfrom
Akanksha-kedia:fix/hnsw-directory-resource-leak

Conversation

@Akanksha-kedia

@Akanksha-kedia Akanksha-kedia commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Both HnswVectorIndexCreator and MutableVectorIndex open an FSDirectory in their constructors and then immediately construct an IndexWriter over it. If IndexWriter construction throws — e.g. due to a locked or corrupt index directory, a codec initialization failure, or an OutOfMemoryError — the already-opened FSDirectory is silently leaked: the catch block wraps and rethrows the exception without ever calling close() on the directory.

On servers that repeatedly create or reload vector indexes under error conditions, this accumulates unclosed file descriptors until the OS limit is hit, causing new index creations to fail with "Too many open files".

Root cause

// Both files — same pattern
_indexDirectory = FSDirectory.open(indexFile.toPath());  // succeeds
_indexWriter = new IndexWriter(_indexDirectory, ...);     // throws → _indexDirectory leaks

Fix

Use local variables so the directory is accessible in the catch block. Close the directory (or writer, if construction succeeded) before rethrowing. In MutableVectorIndex, also delete the temporary index directory so stale getTempDirectory() subdirectories don't accumulate on failure.

Directory indexDirectory = null;
IndexWriter indexWriter;
try {
    indexDirectory = FSDirectory.open(indexFile.toPath());
    indexWriter = new IndexWriter(indexDirectory, ...);
} catch (Exception e) {
    if (indexDirectory != null) {
        try { indexDirectory.close(); } catch (IOException ce) { e.addSuppressed(ce); }
    }
    throw new RuntimeException("...", e);
}
_indexDirectory = indexDirectory;
_indexWriter = indexWriter;

Files changed

  • HnswVectorIndexCreator.java — offline segment generation path
  • MutableVectorIndex.java — realtime ingestion path (also cleans up temp dir on failure)

Test plan

  • Existing unit/integration tests pass
  • ./mvnw checkstyle:check -pl pinot-segment-local — 0 violations

cc @xiangfu0 @raghavyadav01 @Jackie-Jiang

@Jackie-Jiang Jackie-Jiang added the bug Something is not working as expected label Jul 10, 2026
@xiangfu0

Copy link
Copy Markdown
Contributor

please rebase

@Akanksha-kedia

Copy link
Copy Markdown
Contributor Author

@xiangfu0 please take a look when you get a chance

In both HnswVectorIndexCreator and MutableVectorIndex, the constructor
opens an FSDirectory then constructs an IndexWriter. If IndexWriter
throws (e.g. due to a corrupt or locked index directory, codec init
failure, or OOM), the already-opened FSDirectory was silently leaked:
the catch block only wrapped and rethrew the exception without closing
the directory.

On systems that create many vector indexes (or under repeated reload
failures), this exhausts file descriptors over time.

Fix: use local variables for the Directory and IndexWriter so they are
accessible in the catch block. Close the directory (or the writer, which
owns the directory when construction succeeds) before rethrowing.
For MutableVectorIndex also delete the temporary index directory so it
does not accumulate stale temp files on failure.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@Akanksha-kedia
Akanksha-kedia force-pushed the fix/hnsw-directory-resource-leak branch from 5ee7efe to 9402863 Compare July 12, 2026 09:45
@Akanksha-kedia

Copy link
Copy Markdown
Contributor Author

@xiangfu0 please take a look when you get a chance

@codecov-commenter

codecov-commenter commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.05%. Comparing base (a2a27a1) to head (769ac5c).

Files with missing lines Patch % Lines
...local/realtime/impl/vector/MutableVectorIndex.java 50.00% 11 Missing ⚠️
...nt/creator/impl/vector/HnswVectorIndexCreator.java 50.00% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18964      +/-   ##
============================================
- Coverage     65.05%   65.05%   -0.01%     
  Complexity     1403     1403              
============================================
  Files          3399     3399              
  Lines        212808   212832      +24     
  Branches      33568    33571       +3     
============================================
+ Hits         138443   138451       +8     
- Misses        63223    63236      +13     
- Partials      11142    11145       +3     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 65.05% <50.00%> (-0.01%) ⬇️
temurin 65.05% <50.00%> (-0.01%) ⬇️
unittests 65.04% <50.00%> (-0.01%) ⬇️
unittests1 56.97% <15.62%> (-0.02%) ⬇️
unittests2 37.39% <50.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Akanksha-kedia

Copy link
Copy Markdown
Contributor Author

@xiangfu0 @shounakmk219 would appreciate a review when you get a chance. This PR fixes a file-descriptor leak in HnswVectorIndexCreator: if IndexWriter construction throws after FSDirectory is already open, the directory was previously never closed. The fix uses local variables with a try-catch so the directory is always closed on failure.

@Akanksha-kedia

Copy link
Copy Markdown
Contributor Author

@xiangfu0 @gortiz — would appreciate a review when you get a chance 🙏

@Jackie-Jiang Jackie-Jiang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@Jackie-Jiang
Jackie-Jiang merged commit 1216618 into apache:master Jul 16, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something is not working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants