Commit Graph
2 Commits
Author SHA1 Message Date
Dan RammerandClaude Opus 5 fe34eb3549 refactor!: rename LSM APIs to MemWAL and align SSTable/table shard naming (#4075)
## What

Renames the MemWAL surface in two passes, across Rust, Python,
TypeScript, and Java, including the wire format.

### 1. MemWAL APIs drop "LSM"

The write path these APIs drive is Lance's MemWAL; "LSM" described its
shape rather than naming it. Turning it on and off now reads as enabling
a feature rather than installing a spec.

| Before | After |
|---|---|
| `set_lsm_write_spec` | `enable_memwal` |
| `unset_lsm_write_spec` | `disable_memwal` |
| `get_lsm_write_spec` | `get_memwal_spec` |
| `flush_lsm` / `compact_lsm` / `checkpoint_lsm` | `flush_memwal` /
`compact_memwal` / `checkpoint_memwal` |
| `get_lsm_stats` | `get_memwal_stats` |
| `close_lsm_writers` | `close_memwal_writers` |
| `use_lsm` (query, merge insert) | `use_memwal` |
| `LsmWriteSpec` / `LsmStats` | `MemWalWriteSpec` / `MemWalStats` |
| Java `LanceDbTableLsm` | `LanceDbTableMemWal` |

TypeScript and Java use camelCase (`enableMemWal`, `getMemWalStats`,
...); the napi methods set `js_name` so the generated names keep the
`MemWal` casing. flush/compact/checkpoint keep a `_memwal` suffix so
`compact_memwal` is not confused with `compact_files` / `optimize`,
which compact the base table.

Internal names (`LsmMode`, `LsmScanner`, the `merge::lsm` / `query::lsm`
modules) are unchanged.

### 2. Stats use SSTable and table shard

Aligns the stats surface with the MemWAL naming settled upstream in
lance-format/lance#7943 and #7957, where the persisted unit became an
**SSTable**.

**Generation → SSTable.** A `GenerationStats` describes one flushed
MemTable, which is an SSTable. The generation *number* is kept — an
SSTable is identified by its generation — so only the noun moved.

**Bucket → table shard.** Each entry is one MemWAL shard. "Bucket" names
only the hash sharding *transform*, so it was already wrong for
`identity` and `year` sharding, which produce shards and no buckets at
all.

| Before | After |
|---|---|
| `GenerationStats` | `SsTableStats` |
| `BucketStats` | `TableShardStats` |
| `stats.buckets` | `stats.table_shards` |
| `BucketStats.generations` | `TableShardStats.sstables` |
| `include_generation_rows` | `include_sstable_rows` |
| `newest_generation` | `newest_sstable_generation` |
| `outstanding_generations` | `outstanding_sstables` |

Docs and comments follow suit: "seal" → "freeze", "L0" / "flushed
generations" → SSTables, "bucket" (meaning a shard) → table shard.

`MemWalWriteSpec::bucket`, `num_buckets`, and the `"Sealed"` shard
status value are **unchanged** — those name the sharding transform or a
server value, not the shard.

## Compatibility

Breaking, for an experimental API. The wire format changes with the
SDKs, so this must roll out together with the matching server change:

- Routes: `/set_lsm_write_spec/` → `/enable_memwal/`,
`/unset_lsm_write_spec/` → `/disable_memwal/`, `/get_lsm_write_spec/` →
`/get_memwal_spec/`, `/flush_lsm/` → `/flush_memwal/`, `/compact_lsm/` →
`/compact_memwal/`, `/get_lsm_stats/` → `/get_memwal_stats/`
- Response keys: `lsm_write_spec` → `memwal_spec`, `lsm_stats` →
`memwal_stats`, `buckets` → `table_shards`, `generations` → `sstables`
- Request fields: `include_generation_rows` → `include_sstable_rows`;
`use_lsm` → `use_memwal` on query and merge-insert bodies

## Validation

- Rust: `cargo test --features remote -p lancedb --lib --tests` — 1467
lib tests and all integration test targets pass; `cargo clippy` clean
- Python: rebuilt the extension; `test_remote_db`, `test_memwal_spec`,
`test_merge_insert_lsm`, `test_query`, `test_permutation`, `test_table`
— 655 passed; `ruff format` / `ruff check` clean
- TypeScript: `pnpm build`; `table`, `merge`, `query` tests — 593
passed; `biome check` clean; `docs/src/js` regenerated with `npm run
docs`
- Java: `mvn test` — 37 passed (Spotless applied with JDK 11; the pinned
google-java-format does not run on JDK 21)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-10-16 13:33:57 -05:00
Dan RammerandClaude Opus 5 f1c4967eeb feat: bring the MemWAL LSM surface to parity across the SDKs (#3962)
## Why

Four of the eight LSM methods are **remote-only in the core**. `impl
BaseTable for NativeTable` implements only
`set`/`unset`/`get_lsm_write_spec` and `close_lsm_writers`; `flush_lsm`,
`compact_lsm` and `get_lsm_stats` fall through to trait defaults
returning `NotSupported` (`rust/lancedb/src/table.rs:679,687,696`), and
`checkpoint_lsm` is built on all three.

That explains the state of the bindings: Node had bound the four that
work against a local table and stopped, so a Cloud user could install an
LSM write spec but had no way to observe fresh-tier state or drive a
checkpoint. Java had none of it at all.

| SDK | set/unset/get spec | closeWriters | flush | compact | getStats |
checkpoint |
|---|---|---|---|---|---|---|
| Rust core | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ |
| Python | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ |
| Node *(before)* | ✅ | ✅ | — | — | — | — |
| **Node (after)** | ✅ | ✅ | **new** | **new** | **new** | **new** |
| Java *(before)* | — | — | — | — | — | — |
| **Java (after)** | **new** | n/a | **new** | **new** | **new** |
**new** |

Go and C are separate repos and are out of scope here. `closeLsmWriters`
drains cached in-process shard writers, so it has no meaning for Java,
which is a pure REST client.

## Node

Adds napi bindings for `flushLsm`, `compactLsm`, `checkpointLsm` and
`getLsmStats`, plus typed `LsmStats` / `BucketStats` / `GenerationStats`
/ `MemtableStats` objects — typed rather than a JSON blob, matching the
existing `LsmWriteSpec` object in the same file, with `u64` cast to
`i64` per that file's convention.

Because these four are remote-only, the new tests assert each binding
reaches the core and surfaces `NotSupported` against a local table. That
covers the wiring; behavior against a real endpoint stays covered by the
mocked-endpoint tests in `rust/lancedb/src/remote/table.rs`.

## Python

No new methods. All eight are on `LanceTable`, `AsyncTable` and
`RemoteTable` — the last four landed on the sync `RemoteTable` in #3961,
which is merged into this branch.

What was missing here was reachability. `LsmWriteSpec` was importable
only from the private `lancedb._lancedb`, appearing in `table.py` solely
under `if TYPE_CHECKING:`, and `docs/src/python/python.md` had no
mention of it, which per the repo's docs guidance means it rendered
nowhere in the API reference. It is now `lancedb.LsmWriteSpec`, in
`__all__`, and documented.

## Java

Java reaches LanceDB purely over REST through the generated Lance
Namespace client, and these routes are not in that spec, so they are
issued through a small dedicated client rather than added to the spec.
That call is revisitable — LSM is one of four unspecified route families
alongside `multipart_write`, `page_cache/prewarm` and
`branches/diff|merge`. If those are ever regularized into the spec as a
group, `LanceDbTableLsm` is one file that gets deleted.

`LsmWriteSpec` here is deliberately **not**
`org.lance.memwal.InitializeMemWalParams`. That type defaults to
maintaining *no* indexes where a spec here defaults to maintaining
*every* index, and it cannot express the `null` that asks the server to
resolve the set:

| Value | On the wire | Meaning |
|---|---|---|
| unset (null) | `null` | Server resolves **every** maintainable index |
| `Collections.emptyList()` | `[]` | Maintain **none** |
| `Arrays.asList("id_idx")` | `["id_idx"]` | Exactly those |

A dedicated test pins null and `[]` as distinct on the wire, since
collapsing them is the failure mode that motivated a LanceDB-owned type.

`checkpointLsm` is ported from `rust/lancedb/src/table/checkpoint.rs`
with its constants and status semantics intact: 429/503 retried in place
against an 8-budget, 421 restarting from flush against a 3-budget, 5s
poll, and a target watermark fixed after the seal so it terminates under
write load.

`getLsmStats` returns typed `LsmStats` / `BucketStats` /
`GenerationStats` / `MemtableStats`, mirroring the Rust structs in
`rust/lancedb/src/table/lsm_stats.rs` and the objects Node exposes.
Decoding is strict — see below.

## Review feedback

Both gatekeeper findings were real. Each was reproduced against the
scripted test server first, and each fix ships with the reproducer as a
regression test.

**The transport was doubling every checkpoint retry budget.**
`HttpClients.createDefault()` installs Apache's default response retry
strategy, whose retryable-status list is exactly 429 and 503 — the two
statuses `isRetryable` owns. A 429 held against `flush_lsm` issued
**18** wire requests where the loop intends 9, and `compact_lsm` was
retried in place despite the loop being built to fall through to a fresh
stats poll instead. Timing confirmed the mechanism: that run took 25.4s
≈ 16.3s of the loop's own backoff plus 9 × the transport's 1s retry
interval.

Automatic retries are now disabled, so the checkpoint loop is the sole
owner of the 421/429/503 transitions. A side effect worth noting:
`testCheckpointRetriesRetryableStatusInPlace` was passing on a
transport-absorbed 429 and never reaching `issue()`'s retry branch at
all. It now exercises the real path.

**Stats decoding failed open.** `getLsmStats` read the response with
Jackson's `path()`, which yields a missing node that iterates as an
empty array — making "malformed" indistinguishable from "no buckets",
which is indistinguishable from "drained". Four separate payloads made
`checkpointLsm()` report convergence for a checkpoint that never ran:

| Response | Before | Now |
|---|---|---|
| `{"lsm_stats": null}` or absent key | disabled ✓ | disabled ✓ |
| `{"lsm_stats": {}}` | **reported success** | `IllegalStateException` |
| empty response body | **reported success** | `IllegalStateException` |
| bucket missing required fields | **reported success** |
`IllegalStateException` |

The empty-body row is the one to weight: a proxy 200 with no body is a
realistic production event, and it silently reported a checkpoint that
never happened.

Decoding is now strict and fails closed, matching the serde contract on
the Rust side exactly. One deliberate deviation from the review comment,
which asked that *only* explicit JSON `null` count as disabled: Rust has
`#[serde(default)]` on `lsm_stats`, so an **absent key** decodes to
`None` there too. Java now matches that. It is an absent-or-malformed
**`buckets`** that fails closed, which is the case the comment was
actually protecting.

## Testing

- Java: **33 passing** (8 existing + 25 LSM) against a scripted
`com.sun.net.httpserver.HttpServer` — no new test dependency. Wire
assertions mirror `rust/lancedb/src/remote/table.rs:6581-6748`;
checkpoint tests cover convergence, not piling onto a latched bucket,
421 restart-from-flush, 429 retry-in-place, terminal-status propagation,
reissue exhaustion, the exact wire-request count against the retry
budget, and five malformed stats payloads.
- Node: **19 LSM tests passing**; `cargo check`, `npm run build`, `npm
run tsc`, `npm run lint`, `npm run docs` all clean.
- Python: `ruff format --check` and `ruff check` clean.
- Java formatting: `./mvnw -pl lancedb-core spotless:apply` and
`spotless:check` both clean under a JDK 11 toolchain.

## Note: spotless needs a pre-16 JDK

`./mvnw spotless:apply` fails on JDK 16+ with
`JCTree$JCImport.getQualifiedIdentifier()` — google-java-format 1.7,
pinned at `java/pom.xml:34`, predates JDK 16's compiler API change.
**This is pre-existing** and reproduces on a pristine `main` checkout.

It is not a blocker, just a toolchain requirement. Spotless was run
against these sources under JDK 11 and both `spotless:apply` and
`spotless:check` pass on the whole module:

```shell
JAVA_HOME=/path/to/jdk11 ./mvnw -pl lancedb-core spotless:apply
```

Bumping the plugin so it works on modern JDKs is still worth doing, but
separately from this PR.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-19 11:44:46 -05:00