feat: Extensible segment components via plugin trait#2993
Conversation
Introduce a SegmentPlugin / PluginWriter trait system that lets custom data structures participate in the segment lifecycle (write, serialize, merge, GC) through the same interface as the built-in components. The built-ins (field norms, postings, fast fields, store) are reimplemented as plugins. Highlights: - PluginWriter::add_document hook: write-path extensions (and the built-ins) receive every document with no by-name wiring in SegmentWriter. The hook takes a concrete &TantivyDocument; Document::into_tantivy_document converts custom document types once at the boundary. - Per-segment plugin extensions are persisted and reopen fails closed when a required plugin is not registered. - GC file listing and SegmentSpaceUsage are derived from the registered plugins rather than a hardcoded component enum. - Custom data is read back via the existing public APIs (SegmentReader::open_read + Collector); no read-side plugin hook is needed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e9898a325
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let custom_plugins = if let Some(first_segment) = segments.first() { | ||
| first_segment.index().custom_plugins().to_vec() |
There was a problem hiding this comment.
Validate plugins before cross-index merges
When merge_indices/merge_filtered_segments is called with a source index that was reopened without re-registering its persisted custom plugins, or with later source indices that have plugins not present on the first index, this copies only the first segment index's live custom_plugins. The merge then omits those custom components and writes output metadata without their extensions, so plugin data is silently lost instead of failing closed; this path bypasses the writer-time PluginCheckedIndex guard.
Useful? React with 👍 / 👎.
| let persisted_custom_extensions: HashSet<&str> = metas | ||
| .persisted_custom_extensions | ||
| .iter() | ||
| .map(String::as_str) | ||
| .collect(); |
There was a problem hiding this comment.
Preserve custom plugin registration order
For indexes with multiple custom plugins where a later plugin depends on an earlier plugin's serialized output, converting the persisted extension list to a HashSet drops the creation-time order. Reopening the same index with the same extensions registered in a different order passes validation, but all_plugins() serializes and merges in the new registration order, so dependent plugins can run before their prerequisites; the persisted order should be validated instead of only set membership.
Useful? React with 👍 / 👎.
| /// Context provided to [`SegmentPlugin::merge`]. | ||
| pub struct PluginMergeContext<'a> { | ||
| pub readers: &'a [SegmentReader], | ||
| pub doc_id_mapping: &'a SegmentDocIdMapping, |
There was a problem hiding this comment.
Expose merge doc-id mapping to plugins
When an out-of-crate SegmentPlugin::merge needs to merge per-document data after deletes or index sorting, this context exposes SegmentDocIdMapping but the address iterator and backing fields remain pub(crate). External plugins therefore cannot determine which source doc corresponds to each output doc, so a correct merge either fails to compile or has to ignore deletes/sort remapping and corrupts the custom component.
Useful? React with 👍 / 👎.
| for plugin in self.index.all_plugins() { | ||
| let plugin_usage = plugin | ||
| .space_usage(self) | ||
| .map_err(|err| io::Error::other(err.to_string()))?; | ||
| components.extend(plugin_usage); |
There was a problem hiding this comment.
Include persisted custom files in space usage
When a custom-plugin index is opened read-only without re-registering its plugin, this iterates only the live registered plugins, even though registration is documented as required only before write/merge/GC. In that common read-only context, space_usage() silently omits the persisted custom component files from both the component map and total byte count; it should at least account for IndexMeta::persisted_custom_extensions with basic file sizes when the plugin is absent.
Useful? React with 👍 / 👎.
| let previous_meta = index.load_metas()?; | ||
| let index_meta = IndexMeta { | ||
| index_settings: index.settings().clone(), | ||
| persisted_custom_extensions: previous_meta.persisted_custom_extensions.clone(), |
There was a problem hiding this comment.
Validate plugins for single-segment writers
When SingleSegmentIndexWriter::new is used with an Index::open handle whose custom plugin has not been re-registered, SegmentWriter creates only the built-in writers, but this finalize path still copies the persisted custom extensions into meta.json. The published segment then claims custom component files that were never written, so later plugin-aware merges or reads of that component fail instead of the path failing closed like IndexWriter does.
Useful? React with 👍 / 👎.
| self.index_document(&document)?; | ||
| let doc_writer = self.segment_serializer.get_store_writer(); | ||
| doc_writer.store(&document, &self.schema)?; | ||
| let schema = self.schema.clone(); | ||
| for writer in &mut self.plugin_writers { | ||
| writer.add_document(doc_id, &document, &schema)?; |
There was a problem hiding this comment.
Call plugin hooks before mutating postings
If a later PluginWriter::add_document fails for a document, for example the fast-field plugin rejects an invalid fast-field value or a custom plugin validates the document, postings and field norms have already been updated while max_doc is not advanced. Callers that continue with the same SegmentWriter (notably the public single-segment writer) will reuse the same doc id for the next document and serialize inconsistent component lengths; the built-in fast-field validation used to run before index_document mutated postings.
Useful? React with 👍 / 👎.
| /// store (`<uuid>.store.temp`) and the delete bitset (`<uuid>.<opstamp>.del`). | ||
| /// Both spellings of each are reserved — the bare [`SegmentComponent`] name and the | ||
| /// on-disk extension — so a custom plugin cannot claim either and contend for the file. | ||
| const RESERVED_NON_PLUGIN_EXTENSIONS: &[&str] = &["temp", "store.temp", "del"]; |
There was a problem hiding this comment.
Reject delete-bitset extension patterns
When a custom plugin declares an extension such as 42.del, this guard allows it even though delete bitsets are stored as <uuid>.<opstamp>.del. If a segment later needs a delete file at that opstamp, advance_deletes and the plugin target the same path, so deletes or merges fail with FileAlreadyExists instead of safely coexisting; numeric *.del extensions should be reserved along with bare del.
Useful? React with 👍 / 👎.
fulmicoton
left a comment
There was a problem hiding this comment.
Main comment:
You went for a 1 file per plugin design, which forced you to isolate indexing aside.
I think we could have one InvertedIndex plugin which covers several files.
What do you think?
fulmicoton
left a comment
There was a problem hiding this comment.
The inverted index stuff, due to their interactions, end up being pulled apart.
Should we group them (termdict, postings, positions, fieldnorms) as a single plugin maybe?
| fn add_document( | ||
| &mut self, | ||
| _doc_id: DocId, | ||
| _doc: &TantivyDocument, |
There was a problem hiding this comment.
You removed the point of having a Document trait.
I was not much involved with its introduction. It was mainly lead by @ChillFish8 . I can check if we rely on it on Quickwit side. Can you check with @ChillFish8 if this is still in use in his projects?
If not, we need to also assess removing the Document trait entirely.
There was a problem hiding this comment.
I use a custom built system now, however, I believe some other users make use of this trait going of some previous discussions in the discord.
I think I would lean towards a solution which can still support custom document types, although I think current design of that custom doc system isn't perfect and might want some changes to make it more ergonomic.
There was a problem hiding this comment.
Good point. We don't make use of custom documents so I forgot to flag this when putting up this PR.
Given the current pull-based Document approach, we'd need a pretty significant refactor to make this generic for all plugins. Perhaps a middle ground could be: bring back the generic add_document for the builtin plugins (inverted index, fast field, store), but make it so that TantivyDocuments are materialized for custom plugins (and we document that as a perf limitation).
Thoughts?
There was a problem hiding this comment.
Okay, let me know what you think of this...
Builtin plugins index the documents generically (like what they do today). Custom plugins get an erased ErasedDocument which has an ErasedValue that mirrors ReferenceValue, and can be downcast to a custom Document.
fulmicoton
left a comment
There was a problem hiding this comment.
This PR (silently) removes the purpose of the Document trait. This might be ok (well not mentioning in the PR description is not ok) but we need to investigate.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e0c5865dc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// Opens one of the component files for reading. | ||
| pub fn open_read(&self, component: SegmentComponent) -> Result<FileSlice, OpenReadError> { | ||
| let path = self.relative_path(component); | ||
| self.index.directory().open_read(&path) |
There was a problem hiding this comment.
Keep custom component files alive for held readers
When a Searcher/SegmentReader is kept across a reload and the writer's garbage collection removes old segment files, this lazy path reopens the component by filename even though built-in segment data is already opened and cached for exactly this lifetime guarantee. A custom plugin query that later calls open_read(Custom(...)) on the still-valid old reader will get FileDoesNotExist after ordinary merges/GC; the reader needs to hold custom component file slices (or otherwise protect them from GC) when it is opened.
Useful? React with 👍 / 👎.
6e0c586 to
5755d0a
Compare
- Drop gratuitous Option wrappers in FastFieldsPluginWriter; serialize takes &mut self - Replace PluginCheckedIndex newtype with Index::validate_plugins; add Index::custom_extensions - Fail closed when merging indices whose source plugin sets differ - Avoid one-letter closure variable names in plugin code - Unify fieldnorm + postings into one InvertedIndexPlugin (in the index module); serialize consumes self, dropping the separate close() pass - Index built-in writers (inverted index, fast fields, store) generically so any Document type is indexed zero-copy; custom plugins receive the document type-erased via ErasedDocument instead of a materialized TantivyDocument
5755d0a to
1d90bf8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d90bf875e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for plugin in self.index.all_plugins() { | ||
| let plugin_usage = plugin | ||
| .space_usage(self) | ||
| .map_err(|err| io::Error::other(err.to_string()))?; |
There was a problem hiding this comment.
Keep space usage on held readers self-contained
When a Searcher is held across a reload/merge and the writer's GC removes the old segment files, this new dispatch calls built-in plugin space_usage implementations that reopen component files by path (for example termdict/postings/fast fields) instead of using the FileSlices already cached in SegmentReader. The old searcher is still valid for search, but searcher.space_usage() can now fail with FileDoesNotExist or undercount positions after ordinary GC; the previous implementation computed usage from the cached composites/readers.
Useful? React with 👍 / 👎.
Motivation
Today every segment component — postings, fast fields, field norms, store — is hardcoded into several places. Adding a new per-segment data structure means forking Tantivy and editing each of those sites.
This PR introduces a
SegmentPlugintrait that lets a custom component participate in the full segment lifecycle — write, serialize, merge, garbage collection, space usage — through the same interface the built-ins use, without touching Tantivy internals. The four built-in components are themselves reimplemented as plugins.We (ParadeDB) plan on using this trait for 1) additional segment metadata for partitioning 2) custom vector index.
Plugin Trait
Two traits. The first is the
SegmentPluginfactory:A
SegmentPluginowns one or more file extensions and knows how to (a) build a writer for the indexing path and (b) merge itself across segments.The second trait is the segment writer:
The write path no longer has any by-name wiring:
SegmentWriterhands every document to every plugin writer'sadd_document, andfinalize()callsserializethen close on each.Key Design Decisions
The index — not the segment — owns the plugin set. The set of custom plugins is recorded once, at index creation, in
IndexMeta.#[serde(default)]makes this backward compatible.Plugins are registered, like tokenizers — and re-registration is enforced fail-closed. Plugins are not serialized; they're re-attached on every
Index::openviaregister_plugin, exactly like custom tokenizers. To prevent consumers from accidentally forgetting to register a plugin, we validate the registered plugin set against the persisted set when the index is first used for a write/merge/GC operation.Registration order is the write/merge order. Built-ins come first (field norms → postings → fast fields → store), then custom plugins.
The read side needs no plugin hook. Custom component data is read back through the existing public surface
SegmentReader::open_read.Backwards compatibility — behavior for existing indexes is unchanged.