Add GarnetRecordDisposer with per-record dispose on page eviction#1689
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new per-record disposal hook that runs during page eviction, enabling applications to clean up external resources while the evicted record is still intact in memory.
Changes:
- Extend Tsavorite record disposer APIs with
DisposeRecord(ref LogRecord, DisposeReason)and plumb it throughIStoreFunctions/StoreFunctions. - Add
DisposeRecordsInRangeForEviction(startAddress, endAddress)and invoke it fromAllocatorBase.OnPagesClosedWorkerwhenDisposeOnPageEvictionis enabled. - Introduce
GarnetRecordDisposerand update Garnet server/tests/global usings to use it.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Garnet.test/GarnetObjectTests.cs | Update store creation to pass GarnetRecordDisposer into StoreFunctions.Create. |
| libs/storage/Tsavorite/cs/src/core/Index/StoreFunctions/StoreFunctions.cs | Add DisposeRecord delegation to the configured IRecordDisposer. |
| libs/storage/Tsavorite/cs/src/core/Index/StoreFunctions/IStoreFunctions.cs | Add DisposeRecord(ref LogRecord, DisposeReason) to store-functions interface. |
| libs/storage/Tsavorite/cs/src/core/Index/StoreFunctions/IRecordDisposer.cs | Add default no-op DisposeRecord callback for per-record cleanup. |
| libs/storage/Tsavorite/cs/src/core/Allocator/TsavoriteLogAllocator.cs | Add stub DisposeRecordsInRangeForEviction implementation. |
| libs/storage/Tsavorite/cs/src/core/Allocator/SpanByteAllocator.cs | Add (currently empty) DisposeRecordsInRangeForEviction implementation. |
| libs/storage/Tsavorite/cs/src/core/Allocator/ObjectAllocatorImpl.cs | Implement eviction-range iteration that calls storeFunctions.DisposeRecord per record. |
| libs/storage/Tsavorite/cs/src/core/Allocator/ObjectAllocator.cs | Expose DisposeRecordsInRangeForEviction via allocator wrapper. |
| libs/storage/Tsavorite/cs/src/core/Allocator/IAllocator.cs | Extend allocator interface with DisposeRecordsInRangeForEviction. |
| libs/storage/Tsavorite/cs/src/core/Allocator/AllocatorBase.cs | Invoke eviction disposal scan in OnPagesClosedWorker when enabled. |
| libs/server/Storage/Functions/GarnetRecordDisposer.cs | Add new Garnet-specific record disposer (currently no-op / disabled for eviction). |
| libs/host/GarnetServer.cs | Pass GarnetRecordDisposer into store-functions creation. |
| libs/GlobalUsings.cs | Switch Garnet store function/allocator aliases from DefaultRecordDisposer to GarnetRecordDisposer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add IRecordDisposer.DisposeRecord(ref LogRecord, DisposeReason) callback that allows applications to clean up external resources when records are disposed during page eviction. Add DisposeRecordsInRangeForEviction to ObjectAllocatorImpl, which iterates records on evicted pages (skipping page header, null, and closed records) and calls storeFunctions.DisposeRecord for each, giving the application access to the full record while data is still intact. Wired into AllocatorBase.OnPagesClosedWorker after the existing eviction observer and before FreePage, gated by DisposeOnPageEviction. Note: ObjectAllocatorImpl.DisposeRecord (the existing internal method for delete, compaction, CAS failure, etc.) is unchanged. The new storeFunctions.DisposeRecord is only called from the eviction range scan. Add GarnetRecordDisposer replacing DefaultRecordDisposer. Sets DisposeOnPageEviction=false and DisposeRecord as a no-op for now. Consuming features (e.g. RangeIndex) will set DisposeOnPageEviction=true and add record type handlers. Changes: - IRecordDisposer: add DisposeRecord(ref LogRecord, DisposeReason) with default no-op - IStoreFunctions, StoreFunctions: add DisposeRecord delegation - ObjectAllocatorImpl: add DisposeRecordsInRangeForEviction - AllocatorBase.OnPagesClosedWorker: call DisposeRecordsInRangeForEviction when DisposeOnPageEviction is true - IAllocator, ObjectAllocator, SpanByteAllocator, TsavoriteLogAllocator: add DisposeRecordsInRangeForEviction - GarnetRecordDisposer: new struct replacing DefaultRecordDisposer - GlobalUsings.cs: DefaultRecordDisposer -> GarnetRecordDisposer - GarnetServer.cs: pass GarnetRecordDisposer to StoreFunctions.Create Co-authored-by: Copilot <[email protected]>
badrishc
force-pushed
the
badrishc/garnet-record-disposer
branch
from
cae25bb to
cd646e7
Compare
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Add IRecordDisposer.DisposeRecord(ref LogRecord, DisposeReason) callback that allows applications to clean up external resources when records are disposed during page eviction.
Add DisposeRecordsInRangeForEviction to ObjectAllocatorImpl, which iterates records on evicted pages (skipping page header, null, and closed records) and calls storeFunctions.DisposeRecord for each, giving the application access to the full record while data is still intact. Wired into AllocatorBase.OnPagesClosedWorker after the existing eviction observer and before FreePage, gated by DisposeOnPageEviction.
Note: ObjectAllocatorImpl.DisposeRecord (the existing internal method for delete, compaction, CAS failure, etc.) is unchanged. The new storeFunctions.DisposeRecord is only called from the eviction range scan.
Add GarnetRecordDisposer replacing DefaultRecordDisposer. Sets DisposeOnPageEviction=false and DisposeRecord as a no-op for now. Consuming features (e.g. RangeIndex) will set DisposeOnPageEviction=true and add record type handlers.
Changes: