SAI-5884: Unload unused Fields|DocValues Producers #38
Conversation
there are still a few tests that fail, but they fail in known ways hard to address, but none are alarming.
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on October 18. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
magibney
force-pushed
the
resource-unloading
branch
from
131fba1 to
a1d1185
Compare
otherwise dead reference analysis GC parent object may cause decRef, making resource to eligible for unloading before we've incRef'd for the new "child" object Saw this issue during merge on shard split
magibney
force-pushed
the
resource-unloading
branch
from
5769564 to
b14f3e1
Compare
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on November 18. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
this may yet not be enough for callback-oriented methods such as PointTree.visitDocValues(). If we continue to hit errors, we'll simply have to inc/decRef at beginning/end of such methods. But for now we know we have a problem in the way we're wrapping the raw readers, since the wrapped readers are very explicitly unaware of being wrapped, so if we track the wrapper refs, they become GC'able well before the raw instances that do the actual work. In any case we'll proceed iteratively here so that we have a better sense of where the problem is, and where it's been fixed.
also fix some javadocs, etc.
This avoids the need for GC "Reference Handler" thread to do full-on ReferenceQueue tracking, and allows us to more efficiently handle the majority of refs, which live only very briefly.
from derivative objects, indirectly ensure reachability of the top-level object to prevent collection while there are outstanding derivative refs
magibney
force-pushed
the
resource-unloading
branch
from
42ff907 to
9f9aeda
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
lucene/core/src/java/org/apache/lucene/codecs/perfield/PerFieldPostingsFormat.java
Show resolved
Hide resolved
| u.exec.schedule( | ||
| maybeUnloadTask(u, type, u.reporter), | ||
| KEEP_ALIVE_NANOS + INITIAL_NANOS, | ||
| TimeUnit.NANOSECONDS); |
There was a problem hiding this comment.
Bug: NullPointerException when executor is null
The code calls u.exec.schedule() without null-checking exec. If AbstractUnloadHelper.onCreation() returns null (when initialized with a null executor), subsequent attempts to schedule unload tasks will throw NullPointerException, which is not caught by the RejectedExecutionException handler. This occurs in the reopen lambdas for pointsReader, fieldsProducer, and docValuesProducer, causing resource loading to fail unexpectedly.
| } else { | ||
| dvp = | ||
| Unloader.docValuesProducer( | ||
| () -> format.fieldsProducer(srs), srs.segmentInfo.dir, srs); |
There was a problem hiding this comment.
Bug: Incorrect directory access in PerFieldDocValuesFormat
The code accesses srs.segmentInfo.dir to pass a Directory to Unloader.docValuesProducer(), but SegmentInfo does not have a dir property. This should be srs.directory instead, matching the usage pattern in PerFieldPostingsFormat.java line 334. This will cause an AttributeError or similar runtime exception.
Note
Introduces Unloader to dynamically unload/reload codec resources (fields/docvalues/points) with directory-coordinated executors, updates codecs to use it, and adjusts tests/build to support and stress this behavior.
index.Unloaderwith scheduling, reference tracking, andUnloadHelper/UnloadAwareAPIs.UnloadingFieldsProducer,UnloadingDocValuesProducer,UnloadingPointsReaderenabling on-demand reload.PerFieldPostingsFormatandPerFieldDocValuesFormatto wrap producers viaUnloader(bypass whenUnloadAware#disableUnload).Unloaderfor points inSegmentCoreReaders.store.UnloaderCoordinationPointand implement inFSDirectoryandByteBuffersDirectory(per-directory executor management, shutdown on close).lucene.unload.*) with defaults in Gradle.TestUnloader; update various tests to close resources, incRef segment files, or temporarily disable unloading where necessary.Written by Cursor Bugbot for commit 0c25238. This will update automatically on new commits. Configure here.