Unify configured predicate keys and materialized predicate indexes
- Dominant language
- Rust
- Stars
- 244
- Forks
- 29
- Avg merge
- 2d 41m
- Merged PRs (30d)
- 19
Description
## Problem
`PredicateIndices` currently tracks predicate configuration and materialized index data separately:
```rust
struct PredicateIndices {
inner: HashMap,
allowed_predicates: HashSet,
}
```
The fields have different meanings:
- `allowed_predicates` contains every predicate key configured for the store.
- `inner` contains indexes materialized from metadata encountered in store entries.
This permits valid states such as:
```text
allowed_predicates = {"country"}
inner = {}
```
That state occurs when `country` is configured as a predicate index but no inserted entry has contained `country` metadata yet.
Maintaining two collections duplicates predicate keys, increases memory usage, and makes configuration checks less obvious. It also creates the possibility of the collections becoming inconsistent.
## Proposed Direction
Investigate making `inner` the single source of truth:
```rust
struct PredicateIndices {
inner: HashMap,
}
```
Under this model, `inner` must contain one entry for every configured predicate key, even when its `PredicateIndex` contains no metadata-value buckets.
The invariant would become:
> A key exists in `inner` if and only if that predicate index is configured for the store.
This would allow:
- `inner.is_empty()` to determine whether predicate indexing is configured.
- `inner.keys()` to provide the configured predicate list.
- predicate creation and deletion to update one collection.
- ingestion to look up the configured index directly.
## Required Changes
- Initialize `inner` with an empty `PredicateIndex` for every predicate configured during store creation.
- Replace `allowed_predicates` membership checks with `inner` lookups.
- Return `inner` keys from `current_predicates()`.
- Remove predicate configuration directly from `inner`.
- Update dynamic predicate-index creation to establish the new index before populating it.
- Remove `allowed_predicates`.
- Update size accounting, serialization, restoration, and relevant tests.
## Concurrency Constraint
Creating a predicate index for an existing store must not lose concurrent writes.
A potentially unsafe order is:
1. Read existing store entries.
2. Build the predicate index.
3. Insert the completed index into `inner`.
A write between steps 1 and 3 would not appear in the store snapshot and would not see the new index.
The preferred order is:
1. Atomically insert or claim the empty predicate index in `inner`.
2. Read the existing store entries.
3. Populate the claimed index.
With this order:
- writes completed before the index insertion are included in the store scan;
- writes after the insertion update the visible index directly;
- overlapping additions are deduplicated by `HashSet`.
Concurrent create, drop, and recreate operations for the same predicate key also require explicit tests.
## Persistence Compatibility
`PredicateIndices` currently derives serialization with both `inner` and `allowed_predicates`. Removing a field changes the persisted state-machine/snapshot shape.
The implementation must determine whether to:
- add a migration from the current representation;
- introduce a versioned predicate-index representation; or
- defer the format change until a permitted compatibility boundary.
Existing snapshots must not silently restore with missing predicate configuration.
## Performance Considerations
The current model lazily creates a `PredicateIndex` only after matching metadata is encountered. A unified model may eagerly create a Papaya map for every configured predicate key.
Benchmarks should compare:
- store creation with configured but unused predicate keys;
- first ingestion containing a configured key;
- normal indexed ingestion;
- dynamic predicate-index creation over an existing store;
- configuration listing and deletion;
- memory usage for configured but unused indexes.
A lazy value representation may be considered if eager `PredicateIndex` allocation is materially expensive, provided it preserves the single-source-of-truth invariant.
## Acceptance Criteria
- `inner` is the sole source of truth for configured predicate keys.
- No duplicate `allowed_predicates` collection remains.
- Empty but configured predicate indexes remain visible through store information APIs.
- Concurrent writes cannot be omitted while creating an index over existing data.
- Concurrent create, drop, and recreate behavior is covered by tests.
- Snapshot compatibility is explicitly handled and tested.
- Benchmarks show no material regression in predicate ingestion or index management.
Contributor guide
Research direction
Start at the PredicateIndices implementation and trace store creation, ingestion, dynamic creation and deletion, size accounting, serialization, and restoration. Review the existing concurrency and snapshot tests, then add coverage for concurrent create/drop/recreate behavior, persistence compatibility, and the required benchmarks; done means the stated invariant and acceptance criteria hold without omitted writes or lost configuration.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- databases
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100