mirror of
https://github.com/lancedb/lancedb.git
synced 2026-09-21 20:45:57 +00:00
fix(node): preserve optimize cleanup timestamp (#4160)
<!-- lance-gatekeeper-fix:v1 agent=278cb095b2e5d69051442bc254d803a6 generation=1 --> ## Summary - pass the TypeScript `cleanupOlderThan` date to the native binding as an unchanged epoch timestamp - prune with Lance's absolute `before_timestamp` policy so dispatch and compaction time cannot move the cutoff - retain versions created after the supplied cutoff and document that behavior - add boundary and end-to-end regression coverage ## Root cause The TypeScript layer converted the absolute date into an elapsed duration before calling native optimize. Lance converted that duration back into a timestamp only after compaction, which silently advanced the requested cutoff and made the cleanup count depend on a millisecond timing boundary. ## Validation - `cargo fmt --all` - `cargo clippy --quiet --features remote --tests --examples -p lancedb -p lancedb-nodejs` - `pnpm build` - `pnpm lint` - `pnpm run docs` - `pnpm test __test__/table.test.ts --runInBand` (309 passed) Fixes #4159 --------- Co-authored-by: Gatefixer <313497061+lancedb-gatefixer[bot]@users.noreply.github.com> Co-authored-by: Xuanwo <github@xuanwo.io>
This commit is contained in:
co-authored by
Xuanwo
parent
37771fd4fc
commit
ffe94a65a1
@@ -53,6 +53,7 @@ import {
|
||||
Operator,
|
||||
instanceOfFullTextQuery,
|
||||
} from "../lancedb/query";
|
||||
import { LocalTable } from "../lancedb/table";
|
||||
|
||||
describe.each([arrow15, arrow16, arrow17, arrow18])(
|
||||
"Given a table",
|
||||
@@ -2789,7 +2790,7 @@ describe("when optimizing a dataset", () => {
|
||||
it("cleanups old versions", async () => {
|
||||
const stats = await table.optimize({ cleanupOlderThan: new Date() });
|
||||
expect(stats.prune.bytesRemoved).toBeGreaterThan(0);
|
||||
expect(stats.prune.oldVersionsRemoved).toBe(3);
|
||||
expect(stats.prune.oldVersionsRemoved).toBe(2);
|
||||
});
|
||||
|
||||
it("delete unverified", async () => {
|
||||
@@ -2810,6 +2811,24 @@ describe("when optimizing a dataset", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("passes cleanupOlderThan to the native binding as an absolute timestamp", async () => {
|
||||
const optimize = jest.fn().mockResolvedValue({
|
||||
compaction: {
|
||||
filesAdded: 0,
|
||||
filesRemoved: 0,
|
||||
fragmentsAdded: 0,
|
||||
fragmentsRemoved: 0,
|
||||
},
|
||||
prune: { bytesRemoved: 0, oldVersionsRemoved: 0 },
|
||||
});
|
||||
const table = new LocalTable({ optimize } as never);
|
||||
const cutoff = new Date("2020-01-02T03:04:05.678Z");
|
||||
|
||||
await table.optimize({ cleanupOlderThan: cutoff, deleteUnverified: true });
|
||||
|
||||
expect(optimize).toHaveBeenCalledWith(cutoff.getTime(), true);
|
||||
});
|
||||
|
||||
describe.each([arrow15, arrow16, arrow17, arrow18])(
|
||||
"when optimizing a dataset",
|
||||
// biome-ignore lint/suspicious/noExplicitAny: <explanation>
|
||||
|
||||
+3
-10
@@ -148,7 +148,8 @@ export interface OptimizeOptions {
|
||||
* olderThan.setDate(olderThan.getDate() - 1));
|
||||
* tbl.optimize({cleanupOlderThan: olderThan});
|
||||
*
|
||||
* // Delete all versions except the current version
|
||||
* // Delete versions committed before this point. Versions created by the
|
||||
* // optimize call itself are newer than the cutoff and will be retained.
|
||||
* tbl.optimize({cleanupOlderThan: new Date()});
|
||||
*/
|
||||
cleanupOlderThan: Date;
|
||||
@@ -1486,16 +1487,8 @@ export class LocalTable extends Table {
|
||||
}
|
||||
|
||||
async optimize(options?: Partial<OptimizeOptions>): Promise<OptimizeStats> {
|
||||
let cleanupOlderThanMs;
|
||||
if (
|
||||
options?.cleanupOlderThan !== undefined &&
|
||||
options?.cleanupOlderThan !== null
|
||||
) {
|
||||
cleanupOlderThanMs =
|
||||
new Date().getTime() - options.cleanupOlderThan.getTime();
|
||||
}
|
||||
return await this.inner.optimize(
|
||||
cleanupOlderThanMs,
|
||||
options?.cleanupOlderThan?.getTime(),
|
||||
options?.deleteUnverified,
|
||||
);
|
||||
}
|
||||
|
||||
+27
-23
@@ -7,7 +7,7 @@ use chrono::{DateTime, Utc};
|
||||
|
||||
use lancedb::ipc::{ipc_file_to_batches, ipc_file_to_schema};
|
||||
use lancedb::table::{
|
||||
AddDataMode, ColumnAlteration as LanceColumnAlteration, Duration,
|
||||
AddDataMode, ColumnAlteration as LanceColumnAlteration,
|
||||
FieldMetadataUpdate as LanceFieldMetadataUpdate, FtsToken as LanceDbFtsToken,
|
||||
NewColumnTransform, OptimizeAction, OptimizeOptions, Ref, Table as LanceDbTable,
|
||||
};
|
||||
@@ -677,22 +677,20 @@ impl Table {
|
||||
#[napi(catch_unwind)]
|
||||
pub async fn optimize(
|
||||
&self,
|
||||
older_than_ms: Option<i64>,
|
||||
before_timestamp_ms: Option<i64>,
|
||||
delete_unverified: Option<bool>,
|
||||
) -> napi::Result<OptimizeStats> {
|
||||
let inner = self.inner_ref()?;
|
||||
|
||||
let older_than = if let Some(ms) = older_than_ms {
|
||||
if ms == i64::MIN {
|
||||
return Err(napi::Error::from_reason(format!(
|
||||
"older_than_ms can not be {}",
|
||||
i32::MIN,
|
||||
)));
|
||||
}
|
||||
Duration::try_milliseconds(ms)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let before_timestamp = before_timestamp_ms
|
||||
.map(|ms| {
|
||||
DateTime::from_timestamp_millis(ms).ok_or_else(|| {
|
||||
napi::Error::from_reason(format!(
|
||||
"cleanupOlderThan timestamp is out of range: {ms}"
|
||||
))
|
||||
})
|
||||
})
|
||||
.transpose()?;
|
||||
|
||||
let compaction_stats = inner
|
||||
.optimize(OptimizeAction::Compact {
|
||||
@@ -703,16 +701,22 @@ impl Table {
|
||||
.default_error()?
|
||||
.compaction
|
||||
.unwrap();
|
||||
let prune_stats = inner
|
||||
.optimize(OptimizeAction::Prune {
|
||||
older_than,
|
||||
delete_unverified,
|
||||
error_if_tagged_old_versions: None,
|
||||
})
|
||||
.await
|
||||
.default_error()?
|
||||
.prune
|
||||
.unwrap();
|
||||
let prune_stats = if let Some(before_timestamp) = before_timestamp {
|
||||
inner
|
||||
.optimize_prune_before(before_timestamp, delete_unverified, None)
|
||||
.await
|
||||
} else {
|
||||
inner
|
||||
.optimize(OptimizeAction::Prune {
|
||||
older_than: None,
|
||||
delete_unverified,
|
||||
error_if_tagged_old_versions: None,
|
||||
})
|
||||
.await
|
||||
}
|
||||
.default_error()?
|
||||
.prune
|
||||
.unwrap();
|
||||
inner
|
||||
.optimize(lancedb::table::OptimizeAction::Index(
|
||||
OptimizeOptions::default(),
|
||||
|
||||
Reference in New Issue
Block a user