Skip to content

fix(mem-wal): resolve writer shard spec identity - #8112

Open
u70b3 wants to merge 1 commit into
lance-format:mainfrom
u70b3:feat/memwal-shard-spec-id-allocation
Open

fix(mem-wal): resolve writer shard spec identity#8112
u70b3 wants to merge 1 commit into
lance-format:mainfrom
u70b3:feat/memwal-shard-spec-id-allocation

Conversation

@u70b3

@u70b3 u70b3 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • resolve the default dataset-writer shard_spec_id (0) to the MemWAL index's sole automatic sharding spec before creating or claiming a shard manifest
  • accept an explicit id only when it matches that sole spec, and require manually sharded indexes to keep id 0
  • reject metadata with multiple automatic specs because the current writer model supports at most one
  • cover the original fresh-shard bug at the public Dataset::mem_wal_writer boundary plus invalid manual, mismatched, and multiple-spec inputs

Scope

This PR fixes the current writer path only. MemWAL initialization already creates either no spec (manual sharding) or one automatic spec with id 1; the writer must record that same identity instead of its unresolved default 0.

It does not add monotonic allocation, spec revisions, activation semantics, metadata validation for hypothetical producers, or a file-format change. Those require a separate end-to-end design if a re-shard API is introduced.

This PR complements but is not a prerequisite for #7949: this PR resolves the table identity before a fresh manifest is created, while #7949 defensively rejects changing an identity already stored in a manifest.

Testing

  • cargo fmt --all -- --check
  • cargo test -p lance --lib test_writer_shard_spec_resolution_rejects_invalid_identity -- --nocapture
  • cargo test -p lance --lib test_mem_wal_writer_uses_automatic_sharding_spec -- --nocapture
  • cargo clippy --all --tests --benches -- -D warnings

@github-actions github-actions Bot added the enhancement New feature or request label Jul 31, 2026
@u70b3
u70b3 force-pushed the feat/memwal-shard-spec-id-allocation branch 5 times, most recently from a82f42f to 9c6f90d Compare August 2, 2026 08:12
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Important

This PR touches the Lance format specification.

Substantive changes to the format specification — the .proto definitions
and the spec docs under docs/src/format/ — require a PMC vote before merge.
Minor edits such as typo fixes, wording, or formatting are excluded; use your
judgment.

If this is a meaningful format change:

  • Start a vote following the Lance community voting process.
    Format specification modifications need 3 binding +1 votes (excluding the
    proposer), held on GitHub Discussions, with a minimum voting period of 1 week.
  • Once the vote passes, link the completed vote in this PR. It should not be
    merged until the vote is linked.

@github-actions github-actions Bot added the A-format On-disk format: protos and format spec docs label Aug 2, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate recommendation: maintainer decision required.

The fresh-shard writer fix is sound, but the PMC must choose whether the format should make activity implicit in the maximum append-only ID (simple, but commits future cutover and rollback semantics) or persist an explicit active ID (more metadata, but independent activation). This substantive format change requires the documented three binding +1 votes and one-week period; no completed vote is linked. Please link that vote before accepting the contract.

@u70b3
u70b3 force-pushed the feat/memwal-shard-spec-id-allocation branch from 9c6f90d to dbb9b85 Compare August 3, 2026 06:55

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate recommendation: maintainer decision required.

The fresh-shard writer fix is sound, but the PMC must choose whether the format should make activity implicit in the maximum append-only ID (simple, but commits future cutover and rollback semantics) or persist an explicit active ID (more metadata, but independent activation). This substantive format change requires the documented three binding +1 votes and one-week period; no completed vote is linked. Please link that vote before accepting the contract.

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

@u70b3
u70b3 force-pushed the feat/memwal-shard-spec-id-allocation branch from 0c3ae83 to 369c3ae Compare August 3, 2026 10:05
@u70b3 u70b3 changed the title feat(mem-wal): allocate monotonic sharding spec ids fix(mem-wal): resolve writer shard spec identity Aug 3, 2026
@github-actions github-actions Bot added the bug Something isn't working label Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-format On-disk format: protos and format spec docs bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant