Skip to content

feat: add optimize_indices for incremental index maintenance - #74

Open
everySympathy wants to merge 3 commits into
daft-engine:mainfrom
everySympathy:feat/optimize-indices
Open

everySympathy wants to merge 3 commits into
daft-engine:mainfrom
everySympathy:feat/optimize-indices

Conversation

@everySympathy

@everySympathy everySympathy commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Appended data is not indexed automatically: queries stay correct (uncovered fragments fall back to scans) but slow down as the unindexed share grows. This PR adds daft_lance.optimize_indices(), which restores index health on the dataset's latest version:

  • newly appended fragments are indexed,
  • small segments are merged (num_indices_to_merge),
  • stale fragment IDs left inside mixed segments by deletes are healed as part of a commit that indexes or merges new data,
  • it commits no new version when there is no new data to index and no segments to merge.

This closes the loop opened by #73: its PR notes said stale segments "are healed by optimize_indices" and "segment compaction is optimize_indices' job" — this is that function.

Design: delegate to pylance, like lance-ray

lance-ray's optimize_indices is a thin wrapper over pylance's DatasetOptimizer.optimize_indices, and lance-spark has no separate index-optimize command at all (its OPTIMIZE is data compaction; index coverage is handled by Lance core). Lance core owns the delta-index semantics, so this PR delegates too rather than reimplementing them on Daft workers:

User request → resolve dataset (uri | namespace) → pylance optimize_indices → manifest
  • The heavy lifting (scanning uncovered fragments, merging deltas) runs in Lance core in the coordinator process, the same execution shape lance-ray ships.
  • A full distributed rebuild stays where it already is: create_scalar_index(..., replace=True).
  • No version/asof parameters, matching lance-ray: pylance plans from the pinned manifest while committing on latest, so pinning a maintenance op would silently leave newer fragments unindexed (verified experimentally) — a health-restoration API belongs on the latest version.

Checks kept vs. checks not duplicated

The only validation between the public API and the pylance call is for our parameter:

  • indices=[] raises; unknown names raise listing the available indexes (pylance silently ignores unknown index_names, so without this check a typo'd filter would be a silent no-op); duplicates are ignored, matching fragment_ids semantics.
  • num_indices_to_merge and everything about index internals are Lance's: pylance already rejects invalid values, so no pre-check duplicates its gates.

Returns

OptimizeIndicesStats — versions of the dataset's latest snapshot immediately before and after the call, wall-clock duration, changed, and per-index counts of segments and live covered fragments (stale IDs left by deletes do not count as coverage; an index absent from the after-snapshot — e.g. dropped by a concurrent writer — reports zeros). OptimizeIndicesStats is the only top-level stats export; the per-index OptimizedIndexStats stays importable from daft_lance.lance_scalar_index.

Datasets whose legacy manifest describe_indices cannot parse fall back to list_indices names with unknown counts — the same degradation create_scalar_index ships for legacy indexes.

Verified on pylance 11.0.0 (all with indexes committed by our segment workflow)

Behavior Result
Coverage extension INVERTED + BTREE segment indexes: coverage extended over appended fragments in one version bump
Segment merging 4 segments → 1 with num_indices_to_merge=4
Stale-id healing mixed segment \{0,1,2,3\} with fragment 0 fully deleted → coverage [1,2,3,4] after optimize (healing rides along with the commit that indexes new data)
No new data optimize commits nothing; stale-only coverage and a delete-all'd index stay as-is
No-op full-coverage index and index-free dataset: no new version, no error
Query correctness filtered scans identical to an unindexed reference before/after optimize

Tests

New tests/io/lance/test_lance_optimize_indices.py (13 tests): coverage extension with query equivalence against an unindexed reference, segment merging, no-op commits no version, stale-fragment healing after delete, no-heal-without-new-data, delete-all no-op with live-only coverage counts, indices filter targeting one of two indexes, duplicate-name dedupe, unknown/empty indices raising with the manifest untouched, legacy-manifest fallback, dir-namespace entry, and stats shape. Full suite: 426 passed, 9 skipped, 2 xfailed, 2 xpassed. ruff format + ruff check + mypy --strict clean.

wangzheyan added 3 commits September 21, 2026 12:26
Delegates to pylance's DatasetOptimizer.optimize_indices (the same choice
lance-ray makes) to index newly appended fragments, merge small segments,
and heal stale coverage left by deletes, committing at most one new version
and none when there is nothing to do. The indices name filter is our
parameter and validated deterministically (empty list and unknown names
raise); everything else belongs to Lance. Returns OptimizeIndicesStats with
versions, duration, and per-index segment/coverage counts.
pylance's DatasetOptimizer.optimize_indices is declared as
def optimize_indices(self, **kwargs) with no annotations, so mypy
--strict rejects the call from typed code. Add the targeted
type-ignore with the error code, matching the existing pattern for
untyped pylance APIs (e.g. compact_files in lance_compaction.py).
- Drop version/asof: pylance plans from the pinned manifest while
  committing on latest, so pinning a maintenance op silently leaves newer
  fragments unindexed and corrupted the stats; lance-ray's optimize_indices
  has neither parameter.
- Sample both stat snapshots from the dataset's latest version (the
  before-baseline no longer mixes the handle's snapshot with open_latest),
  so changed/version_* no longer report concurrent or pinned commits as
  this run's work.
- Count only live fragments as coverage, so stale IDs left by deletes no
  longer masquerade as coverage (also makes healing visible in the stats).
- Ignore duplicate names in indices, matching fragment_ids semantics.
- Fall back to list_indices when describe_indices cannot parse a legacy
  manifest, mirroring _existing_index_names.
- Precise docs: healing rides along with commits that index or merge new
  data; a no-new-data run commits nothing; a delete-all leaves the index
  and its stale ids as-is. README section moved after the 0.5.0 note,
  doctest examples marked +SKIP.
- Un-export OptimizedIndexStats (still importable from the module) to keep
  one top-level stats name.

4 new tests pin these behaviors (13 total; suite 426 passed).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant