Fix forward index reader context lifecycle

[xiangfu0]

Extracted from #18229 as an independent correctness prerequisite.

ForwardIndexHandler creates ForwardIndexReaderContext instances and PinotSegmentColumnReader wrappers while rewriting or collecting forward-index statistics. These resources can own direct buffers, but several paths did not close them. This change uses try-with-resources across those paths and preserves the existing reader and creator ownership boundaries.

Tests:

• ForwardIndexHandlerContextTest: closes the context exactly once on success and read failure
• pinot-segment-local Spotless, Checkstyle, license format, and license check

This PR does not add codec-pipeline behavior and can merge independently of the codec stack.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 68.51064% with 74 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.10%. Comparing base (b6cd43b) to head (003a430).

Files with missing lines	Patch %	Lines
...ocal/segment/index/loader/ForwardIndexHandler.java	68.51%	71 Missing and 3 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #19281      +/-   ##
============================================
+ Coverage     67.09%   67.10%   +0.01%     
  Complexity     1424     1424              
============================================
  Files          3459     3459              
  Lines        219789   219790       +1     
  Branches      35007    35008       +1     
============================================
+ Hits         147457   147485      +28     
+ Misses        60543    60511      -32     
- Partials      11789    11794       +5     

Flag	Coverage Δ
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-25	67.10% <68.51%> (+0.01%)	⬆️
lane-a	100.00% <ø> (ø)
lane-b	0.00% <ø> (ø)
temurin	67.10% <68.51%> (+0.01%)	⬆️
unittests	67.09% <68.51%> (+0.01%)	⬆️
unittests1	57.72% <15.31%> (-0.01%)	⬇️
unittests2	39.21% <68.51%> (+0.02%)	⬆️

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.

[Copilot]

Pull request overview

Fixes forward-index reader context leaks during index rewrites and statistics collection.

Changes:

• Closes reader contexts and column-reader wrappers via try-with-resources.
• Adds success and failure lifecycle tests while preserving reader/creator ownership.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
ForwardIndexHandler.java	Ensures deterministic cleanup across rewrite and statistics paths.
ForwardIndexHandlerContextTest.java	Verifies owned contexts close without closing externally owned resources.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/ForwardIndexHandlerContextTest.java:39

• /// is not a JavaDoc construct in Java (it’s just a regular line comment), so IDEs and doc tooling won’t treat this as class documentation, and the bracketed references won’t resolve. Prefer a proper JavaDoc block (/** ... */) above the class (or convert to // if you intentionally want a non-doc comment).

/// Unit tests for the resource lifecycle of [ForwardIndexReaderContext] instances created inside
/// [ForwardIndexHandler] rewrite helpers. Verifies that a helper closes the context it creates on both the success
/// and the failure path, without closing the reader or creator it does not own. Single-threaded; no shared state.
public class ForwardIndexHandlerContextTest {

pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/ForwardIndexHandlerContextTest.java:86

• This test hard-depends on reflective access to a private method (name string + setAccessible(true)), which is brittle and can be restricted under stronger JVM encapsulation settings. Consider exposing a small package-private helper (or a @VisibleForTesting-annotated method) that exercises the same resource-ownership behavior without reflection, so the test remains stable across refactors and JVM access changes.

    Method method = ForwardIndexHandler.class.getDeclaredMethod("forwardIndexReadDictWriteDictHelper",
        ForwardIndexReader.class, ForwardIndexCreator.class, int.class);
    method.setAccessible(true);
    return (long) method.invoke(null, reader, creator, 1);