Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 44 additions & 44 deletions docs/src/format/table/mem_wal.md

Large diffs are not rendered by default.

32 changes: 16 additions & 16 deletions java/lance-jni/src/mem_wal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use jni::objects::{JClass, JMap, JObject, JString, JValueGen};
use jni::sys::{jdouble, jint, jlong};
use lance::dataset::Dataset as LanceDataset;
use lance::dataset::mem_wal::scanner::{
FlushedGeneration, LsmDataSourceCollector, LsmPointLookupPlanner, LsmVectorSearchPlanner,
LsmDataSourceCollector, LsmPointLookupPlanner, LsmVectorSearchPlanner, SsTable,
parse_filter_expr as parse_lsm_filter_expr, write_pk_sidecar,
};
use lance::dataset::mem_wal::write::{MemTableStats, WriteStatsSnapshot};
Expand Down Expand Up @@ -204,10 +204,10 @@ fn inner_delete(env: &mut JNIEnv, this: JObject, stream_addr: jlong) -> Result<(
Ok(())
}

/// Test-support: write a primary-key dedup sidecar (`_pk_index/`) for a
/// flushed-generation dataset already staged at `gen_path`, mirroring what
/// production flush emits. Lets Java tests stage a *faithful* flushed
/// generation (dataset + sidecar); production always writes the sidecar during
/// Test-support: write a primary-key dedup sidecar (`_pk_index/`) for an
/// SSTable dataset already staged at `gen_path`, mirroring what
/// production flush emits. Lets Java tests stage a *faithful* SSTable
/// (dataset + sidecar); production always writes the sidecar during
/// flush, so a dataset-without-sidecar is not a state the system produces.
/// Mirrors the Python `_write_pk_sidecar` binding.
#[unsafe(no_mangle)]
Expand Down Expand Up @@ -1119,13 +1119,13 @@ fn read_shard_snapshots(env: &mut JNIEnv, list_obj: &JObject) -> Result<Vec<Shar
.with_spec_id(spec_id)
.with_current_generation(current_generation);

let flushed_list = env
.call_method(&obj, "flushedGenerations", "()Ljava/util/List;", &[])?
let sstable_list = env
.call_method(&obj, "sstables", "()Ljava/util/List;", &[])?
.l()?;
for flushed in import_vec(env, &flushed_list)? {
let generation = env.get_u64_from_method(&flushed, "generation")?;
let path = env.get_string_from_method(&flushed, "path")?;
snapshot = snapshot.with_flushed_generation(generation, path);
for sstable in import_vec(env, &sstable_list)? {
let generation = env.get_u64_from_method(&sstable, "generation")?;
let path = env.get_string_from_method(&sstable, "path")?;
snapshot = snapshot.with_sstable(generation, path);
}
Ok(snapshot)
})
Expand All @@ -1137,12 +1137,12 @@ fn shard_snapshot_from_manifest(manifest: ShardManifest) -> ShardSnapshot {
shard_id: manifest.shard_id,
spec_id: manifest.shard_spec_id,
current_generation: manifest.current_generation,
flushed_generations: manifest
.flushed_generations
sstables: manifest
.sstables
.into_iter()
.map(|generation| FlushedGeneration {
generation: generation.generation,
path: generation.path,
.map(|sstable| SsTable {
generation: sstable.generation,
path: sstable.path,
})
.collect(),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ public class LsmPointLookupPlanner implements AutoCloseable {

/**
* @param dataset the base dataset
* @param shardSnapshots shard snapshots specifying the flushed generations to include
* @param shardSnapshots shard snapshots specifying the SSTables to include
*/
public LsmPointLookupPlanner(Dataset dataset, List<ShardSnapshot> shardSnapshots) {
this(dataset, shardSnapshots, null);
}

/**
* @param dataset the base dataset
* @param shardSnapshots shard snapshots specifying the flushed generations to include
* @param shardSnapshots shard snapshots specifying the SSTables to include
* @param pkColumns primary key column names; inferred from schema metadata when {@code null}
*/
public LsmPointLookupPlanner(
Expand Down
6 changes: 3 additions & 3 deletions java/src/main/java/org/lance/memwal/LsmScanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
* LSM-aware scanner covering all MemWAL data levels.
*
* <p>Results are deduplicated by primary key, always returning the newest version of each row
* across the base table, flushed MemTables, and (when created from a {@link ShardWriter}) the
* active MemTable.
* across the base table, SSTables, and (when created from a {@link ShardWriter}) the active
* MemTable.
*
* <p>The builder methods ({@link #project}, {@link #filter}, {@link #limit}, {@link
* #withRowAddress}, {@link #withMemtableGen}) mutate this scanner and return it for chaining.
Expand All @@ -55,7 +55,7 @@ private LsmScanner() {}
* read-your-writes consistency.
*
* @param dataset the base dataset to scan
* @param shardSnapshots shard snapshots specifying the flushed generations to include
* @param shardSnapshots shard snapshots specifying the SSTables to include
* @return an LSM scanner
*/
public static LsmScanner fromSnapshots(Dataset dataset, List<ShardSnapshot> shardSnapshots) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class LsmVectorSearchPlanner implements AutoCloseable {

/**
* @param dataset the base dataset
* @param shardSnapshots shard snapshots specifying the flushed generations to include
* @param shardSnapshots shard snapshots specifying the SSTables to include
* @param vectorColumn name of the {@code FixedSizeList<float32>} vector column
*/
public LsmVectorSearchPlanner(
Expand All @@ -54,7 +54,7 @@ public LsmVectorSearchPlanner(

/**
* @param dataset the base dataset
* @param shardSnapshots shard snapshots specifying the flushed generations to include
* @param shardSnapshots shard snapshots specifying the SSTables to include
* @param vectorColumn name of the {@code FixedSizeList<float32>} vector column
* @param pkColumns primary key column names; inferred from schema metadata when {@code null}
* @param distanceType distance metric, one of {@code "l2"}, {@code "cosine"}, {@code "dot"},
Expand All @@ -71,7 +71,7 @@ public LsmVectorSearchPlanner(

/**
* @param dataset the base dataset
* @param shardSnapshots shard snapshots specifying the flushed generations to include
* @param shardSnapshots shard snapshots specifying the SSTables to include
* @param vectorColumn name of the {@code FixedSizeList<float32>} vector column
* @param pkColumns primary key column names; inferred from schema metadata when {@code null}
* @param distanceType distance metric, one of {@code "l2"}, {@code "cosine"}, {@code "dot"},
Expand Down
4 changes: 2 additions & 2 deletions java/src/main/java/org/lance/memwal/MergedGeneration.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import com.google.common.base.Preconditions;

/**
* Identifies a flushed MemWAL generation that has been merged into the base table.
* Identifies an SSTable that has been merged into the base table.
*
* <p>Pass a list of these to {@link org.lance.merge.MergeInsertParams#markGenerationsAsMerged} so
* Lance knows which generations are now part of the base table.
Expand All @@ -28,7 +28,7 @@ public class MergedGeneration {

/**
* @param shardId UUID string for the write shard
* @param generation generation number from {@link ShardSnapshot#flushedGenerations()}
* @param generation generation number from {@link ShardSnapshot#sstables()}
*/
public MergedGeneration(String shardId, long generation) {
Preconditions.checkNotNull(shardId, "shardId must not be null");
Expand Down
18 changes: 9 additions & 9 deletions java/src/main/java/org/lance/memwal/ShardSnapshot.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@
* Snapshot of a MemWAL shard's state, used when constructing scanners and planners.
*
* <p>The builder methods ({@link #withSpecId}, {@link #withCurrentGeneration}, {@link
* #withFlushedGeneration}) mutate this instance and return it for chaining.
* #withSsTable}) mutate this instance and return it for chaining.
*/
public class ShardSnapshot {
private final String shardId;
private int specId = 0;
private long currentGeneration = 0;
private final List<FlushedGeneration> flushedGenerations = new ArrayList<>();
private final List<SsTable> sstables = new ArrayList<>();

/**
* @param shardId UUID string for the write shard
Expand All @@ -52,10 +52,10 @@ public ShardSnapshot withCurrentGeneration(long currentGeneration) {
return this;
}

/** Add a flushed generation with its storage path. */
public ShardSnapshot withFlushedGeneration(long generation, String path) {
/** Add an SSTable with its storage path. */
public ShardSnapshot withSsTable(long generation, String path) {
Preconditions.checkNotNull(path, "path must not be null");
this.flushedGenerations.add(new FlushedGeneration(generation, path));
this.sstables.add(new SsTable(generation, path));
return this;
}

Expand All @@ -74,9 +74,9 @@ public long currentGeneration() {
return currentGeneration;
}

/** The flushed generations included in this snapshot. */
public List<FlushedGeneration> flushedGenerations() {
return Collections.unmodifiableList(flushedGenerations);
/** The SSTables included in this snapshot. */
public List<SsTable> sstables() {
return Collections.unmodifiableList(sstables);
}

@Override
Expand All @@ -85,7 +85,7 @@ public String toString() {
.add("shardId", shardId)
.add("specId", specId)
.add("currentGeneration", currentGeneration)
.add("flushedGenerations", flushedGenerations)
.add("sstables", sstables)
.toString();
}
}
5 changes: 2 additions & 3 deletions java/src/main/java/org/lance/memwal/ShardWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,8 @@ public MemTableStats memtableStats() {
/**
* Create an LSM scanner that includes this writer's active MemTable.
*
* <p>The scanner covers the base table, the given flushed generations, and the current active
* MemTable, providing read-your-writes consistency. This writer's own shard is included
* automatically.
* <p>The scanner covers the base table, the given SSTables, and the current active MemTable,
* providing read-your-writes consistency. This writer's own shard is included automatically.
*
* @param shardSnapshots snapshots of other shards to include
* @return an LSM scanner
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,22 @@

import com.google.common.base.MoreObjects;

/** A flushed MemWAL generation and the storage path of its Lance files. */
public class FlushedGeneration {
/** An SSTable and the storage path of its Lance files. */
public class SsTable {
private final long generation;
private final String path;

public FlushedGeneration(long generation, String path) {
public SsTable(long generation, String path) {
this.generation = generation;
this.path = path;
}

/** The generation number of this flushed MemTable. */
/** The generation number of this SSTable. */
public long generation() {
return generation;
}

/** The storage path of the flushed Lance files. */
/** The storage path of the SSTable Lance files. */
public String path() {
return path;
Comment on lines +28 to 35

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.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files:"
git ls-files | rg '(^|/)(SsTable|ShardSnapshot)\.java$|(^|/)mem_wal\.rs$' || true

echo
echo "Java accessors:"
for f in java/src/main/java/org/lance/memwal/SsTable.java java/src/main/java/org/lance/memwal/ShardSnapshot.java; do
  echo "--- $f"
  wc -l "$f"
  sed -n '1,140p' "$f" | cat -n
done

echo
echo "JNI reference:"
if [ -f java/lance-jni/src/mem_wal.rs ]; then
  wc -l java/lance-jni/src/mem_wal.rs
  sed -n '1070,1145p' java/lance-jni/src/mem_wal.rs | cat -n
fi

echo
echo "Search for SsTable/ShardSnapshot reflective method names:"
rg -n "SsTable|ShardSnapshot|generation|path|sstables" java/lance-jni/src/mem_wal.rs java/src/main/java/org/lance/memwal | head -200

Repository: lance-format/lance

Length of output: 17662


Use JavaBean getters for the new public accessors.

The public Java API currently exposes bare accessor methods, which violates the Java binding guideline. Rename them and update the JNI reflective calls plus inline Javadoc references:

  • SsTable.java#L29-L34: rename generation() and path() to getGeneration() and getPath().
  • ShardSnapshot.java#L78: rename sstables() to getSstables().
  • mem_wal.rs#L1117-L1127: update JNI method names and descriptors to the new getters.
  • Update any Javadoc references to ShardSnapshot#sstables().
📍 Affects 2 files
  • java/src/main/java/org/lance/memwal/SsTable.java#L28-L35 (this comment)
  • java/src/main/java/org/lance/memwal/ShardSnapshot.java#L77-L79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@java/src/main/java/org/lance/memwal/SsTable.java` around lines 28 - 35,
Rename SsTable.generation() and path() to JavaBean getters getGeneration() and
getPath(), and rename ShardSnapshot.sstables() to getSstables(). Update JNI
reflective lookups in mem_wal.rs to use the new method names and matching
descriptors, and revise all inline Javadoc references from
ShardSnapshot#sstables() to ShardSnapshot#getSstables(). For
java/src/main/java/org/lance/memwal/SsTable.java lines 28-35 and
java/src/main/java/org/lance/memwal/ShardSnapshot.java lines 77-79, apply the
accessor renames; mem_wal.rs lines 1117-1127 requires the JNI updates.

Source: Coding guidelines

}
Expand Down
28 changes: 14 additions & 14 deletions java/src/test/java/org/lance/memwal/MemWalTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,12 @@ private static Dataset writeAppendOnlyDataset(
}

/**
* Stage a <em>faithful</em> flushed generation at {@code genPath}: the Lance dataset plus its
* primary-key dedup sidecar ({@code _pk_index/}), mirroring what production flush emits. The LSM
* scanner's cross-generation block-list opens the sidecar, so a dataset alone (no sidecar) is not
* a state production produces. Mirrors the Python {@code _write_flushed_gen} test helper.
* Stage a <em>faithful</em> SSTable at {@code genPath}: the Lance dataset plus its primary-key
* dedup sidecar ({@code _pk_index/}), mirroring what production flush emits. The LSM scanner's
* cross-generation block-list opens the sidecar, so a dataset alone (no sidecar) is not a state
* production produces. Mirrors the Python {@code _write_sstable} test helper.
*/
private static void writeFlushedGen(
private static void writeSsTable(
BufferAllocator allocator, String genPath, long[] ids, String prefix) throws Exception {
writeLookupDataset(allocator, genPath, ids, prefix).close();
try (VectorSchemaRoot root = lookupRoot(allocator, ids, prefix);
Expand All @@ -177,8 +177,8 @@ private static void writeFlushedGen(
}

/**
* Test-support native: write the primary-key dedup sidecar for a flushed-generation dataset
* already staged at {@code genPath}. See {@link #writeFlushedGen}.
* Test-support native: write the primary-key dedup sidecar for an SSTable dataset already staged
* at {@code genPath}. See {@link #writeSsTable}.
*/
private static native void nativeWritePkSidecar(
String genPath, long streamAddress, List<String> pkColumns);
Expand Down Expand Up @@ -451,20 +451,20 @@ void testLsmScannerFromSnapshots(@TempDir Path tempDir) throws Exception {
Dataset dataset = writeLookupDataset(allocator, basePath, new long[] {1, 2, 3}, "base")) {
dataset.initializeMemWal(new InitializeMemWalParams());

// Flushed generation overwrites id=2.
// SSTable overwrites id=2.
String genPath = basePath + "/_mem_wal/" + shardId + "/gen_1";
writeFlushedGen(allocator, genPath, new long[] {2}, "gen1");
writeSsTable(allocator, genPath, new long[] {2}, "gen1");

ShardSnapshot snapshot =
new ShardSnapshot(shardId).withFlushedGeneration(1, "gen_1").withCurrentGeneration(2);
new ShardSnapshot(shardId).withSsTable(1, "gen_1").withCurrentGeneration(2);

try (LsmScanner scanner =
LsmScanner.fromSnapshots(dataset, Collections.singletonList(snapshot));
ArrowReader reader = scanner.scanBatches()) {
Map<Long, String> byId = readByName(reader);
assertEquals(3, byId.size(), "Expected 3 deduplicated rows");
assertEquals("base_1", byId.get(1L));
assertEquals("gen1_2", byId.get(2L), "Flushed generation must win over base");
assertEquals("gen1_2", byId.get(2L), "SSTable must win over base");
assertEquals("base_3", byId.get(3L));
}

Expand All @@ -487,14 +487,14 @@ void testPointLookup(@TempDir Path tempDir) throws Exception {
dataset.initializeMemWal(new InitializeMemWalParams());

String genPath = basePath + "/_mem_wal/" + shardId + "/gen_1";
writeFlushedGen(allocator, genPath, new long[] {2}, "gen1");
writeSsTable(allocator, genPath, new long[] {2}, "gen1");

ShardSnapshot snapshot =
new ShardSnapshot(shardId).withFlushedGeneration(1, "gen_1").withCurrentGeneration(2);
new ShardSnapshot(shardId).withSsTable(1, "gen_1").withCurrentGeneration(2);

try (LsmPointLookupPlanner planner =
new LsmPointLookupPlanner(dataset, Collections.singletonList(snapshot))) {
// id=2 must resolve to the flushed-generation value.
// id=2 must resolve to the SSTable value.
assertEquals("gen1_2", lookup(planner, allocator, 2L));
// id=1 only exists in the base table.
assertEquals("base_1", lookup(planner, allocator, 1L));
Expand Down
12 changes: 6 additions & 6 deletions protos/table.proto
Original file line number Diff line number Diff line change
Expand Up @@ -648,8 +648,8 @@ message ShardManifest {
// Field 7 removed: merged_generation moved to MemWalIndexDetails.merged_generations
// which is the authoritative source for merge progress.

// List of flushed MemTable generations and their directory paths.
repeated FlushedGeneration flushed_generations = 8;
// List of SSTables (flushed MemTable generations) and their directory paths.
repeated SsTable sstables = 8;

// Lifecycle status. Default ACTIVE; SEALED marks an in-flight drop
// (drop-table 2PC). A SEALED manifest refuses claims at claim_epoch.
Expand All @@ -666,9 +666,9 @@ message ShardFieldEntry {
bytes value = 2;
}

// A flushed MemTable generation and its storage location.
message FlushedGeneration {
// Generation number.
// An SSTable: a flushed MemTable generation, stored as a Lance dataset.
message SsTable {
// Generation number identifying this SSTable.
uint64 generation = 1;

// Directory name relative to the shard directory.
Expand All @@ -685,7 +685,7 @@ message MergedGeneration {
}

// Tracks which merged generation a base table index has been rebuilt to cover.
// Used to determine whether to read from flushed MemTable indexes or base table.
// Used to determine whether to read from SSTable indexes or base table.
message IndexCatchupProgress {
// Name of the base table index (must match an entry in maintained_indexes).
string index_name = 1;
Expand Down
2 changes: 1 addition & 1 deletion python/python/lance/lance/__init__.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ class _ShardSnapshot:
def __init__(self, shard_id: str) -> None: ...
def with_spec_id(self, spec_id: int) -> Self: ...
def with_current_generation(self, generation: int) -> Self: ...
def with_flushed_generation(self, generation: int, path: str) -> Self: ...
def with_sstable(self, generation: int, path: str) -> Self: ...

class _ShardWriter:
shard_id: str
Expand Down
Loading
Loading