mirror of
https://github.com/lancedb/lancedb.git
synced 2026-08-18 12:08:35 +00:00
fix: reject unsafe version cleanup windows
This commit is contained in:
@@ -117,6 +117,23 @@ _MODEL_BACKED_TOKENIZER_ERRORS = (
|
||||
"Failed to initialize default tokenizer",
|
||||
)
|
||||
|
||||
_MIN_SAFE_CLEANUP_AGE = timedelta(minutes=10)
|
||||
|
||||
|
||||
def _validate_cleanup_options(
|
||||
older_than: Optional[timedelta], delete_unverified: bool
|
||||
) -> None:
|
||||
if (
|
||||
older_than is not None
|
||||
and older_than < _MIN_SAFE_CLEANUP_AGE
|
||||
and not delete_unverified
|
||||
):
|
||||
raise ValueError(
|
||||
"cleanup age must be at least 10 minutes unless delete_unverified is "
|
||||
"true; short cleanup windows can remove a manifest still needed by an "
|
||||
"in-progress write"
|
||||
)
|
||||
|
||||
|
||||
def _add_unique_note(exception: BaseException, note: str) -> None:
|
||||
existing_notes = getattr(exception, "__notes__", ()) or ()
|
||||
@@ -1767,7 +1784,9 @@ class Table(ABC):
|
||||
----------
|
||||
older_than: timedelta, default None
|
||||
The minimum age of the version to delete. If None, then this defaults
|
||||
to two weeks.
|
||||
to two weeks. This must be longer than the longest expected write.
|
||||
Values shorter than 10 minutes require `delete_unverified=True` and
|
||||
are only safe when no other process can write to the dataset.
|
||||
delete_unverified: bool, default False
|
||||
Because they may be part of an in-progress transaction, files newer
|
||||
than 7 days old are not deleted by default. If you are sure that
|
||||
@@ -1835,9 +1854,11 @@ class Table(ABC):
|
||||
Parameters
|
||||
----------
|
||||
cleanup_older_than: timedelta, optional default 7 days
|
||||
All files belonging to versions older than this will be removed. Set
|
||||
to 0 days to remove all versions except the latest. The latest version
|
||||
is never removed.
|
||||
All files belonging to versions older than this will be removed. The
|
||||
latest version is never removed. This must be longer than the longest
|
||||
expected write. Values shorter than 10 minutes require
|
||||
`delete_unverified=True` and are only safe when no other process can
|
||||
write to the dataset.
|
||||
delete_unverified: bool, default False
|
||||
Files leftover from a failed transaction may appear to be part of an
|
||||
in-progress operation (e.g. appending new data) and these files will not
|
||||
@@ -3782,7 +3803,9 @@ class LanceTable(Table):
|
||||
----------
|
||||
older_than: timedelta, default None
|
||||
The minimum age of the version to delete. If None, then this defaults
|
||||
to two weeks.
|
||||
to two weeks. This must be longer than the longest expected write.
|
||||
Values shorter than 10 minutes require `delete_unverified=True` and
|
||||
are only safe when no other process can write to the dataset.
|
||||
delete_unverified: bool, default False
|
||||
Because they may be part of an in-progress transaction, files newer
|
||||
than 7 days old are not deleted by default. If you are sure that
|
||||
@@ -3795,6 +3818,7 @@ class LanceTable(Table):
|
||||
The stats of the cleanup operation, including how many bytes were
|
||||
freed.
|
||||
"""
|
||||
_validate_cleanup_options(older_than, delete_unverified)
|
||||
return self.to_lance().cleanup_old_versions(
|
||||
older_than, delete_unverified=delete_unverified
|
||||
)
|
||||
@@ -3840,9 +3864,11 @@ class LanceTable(Table):
|
||||
Parameters
|
||||
----------
|
||||
cleanup_older_than: timedelta, optional default 7 days
|
||||
All files belonging to versions older than this will be removed. Set
|
||||
to 0 days to remove all versions except the latest. The latest version
|
||||
is never removed.
|
||||
All files belonging to versions older than this will be removed. The
|
||||
latest version is never removed. This must be longer than the longest
|
||||
expected write. Values shorter than 10 minutes require
|
||||
`delete_unverified=True` and are only safe when no other process can
|
||||
write to the dataset.
|
||||
delete_unverified: bool, default False
|
||||
Files leftover from a failed transaction may appear to be part of an
|
||||
in-progress operation (e.g. appending new data) and these files will not
|
||||
@@ -6034,9 +6060,11 @@ class AsyncTable:
|
||||
Parameters
|
||||
----------
|
||||
cleanup_older_than: timedelta, optional default 7 days
|
||||
All files belonging to versions older than this will be removed. Set
|
||||
to 0 days to remove all versions except the latest. The latest version
|
||||
is never removed.
|
||||
All files belonging to versions older than this will be removed. The
|
||||
latest version is never removed. This must be longer than the longest
|
||||
expected write. Values shorter than 10 minutes require
|
||||
`delete_unverified=True` and are only safe when no other process can
|
||||
write to the dataset.
|
||||
delete_unverified: bool, default False
|
||||
Files leftover from a failed transaction may appear to be part of an
|
||||
in-progress operation (e.g. appending new data) and these files will not
|
||||
|
||||
@@ -2958,6 +2958,9 @@ def test_compact_cleanup(tmp_db: DBConnection):
|
||||
stats = table.cleanup_old_versions()
|
||||
assert stats.bytes_removed == 0
|
||||
|
||||
with pytest.raises(ValueError, match="at least 10 minutes"):
|
||||
table.cleanup_old_versions(older_than=timedelta(0))
|
||||
|
||||
stats = table.cleanup_old_versions(older_than=timedelta(0), delete_unverified=True)
|
||||
assert stats.bytes_removed > 0
|
||||
assert table.version == 4
|
||||
@@ -3374,7 +3377,14 @@ async def test_optimize(mem_db_async: AsyncConnection):
|
||||
assert stats.prune.bytes_removed == 0
|
||||
assert stats.prune.old_versions_removed == 0
|
||||
|
||||
stats = await table.optimize(cleanup_older_than=timedelta(seconds=0))
|
||||
version_before_rejected_cleanup = await table.version()
|
||||
with pytest.raises(ValueError, match="at least 10 minutes"):
|
||||
await table.optimize(cleanup_older_than=timedelta(seconds=0))
|
||||
assert await table.version() == version_before_rejected_cleanup
|
||||
|
||||
stats = await table.optimize(
|
||||
cleanup_older_than=timedelta(seconds=0), delete_unverified=True
|
||||
)
|
||||
assert stats.prune.bytes_removed > 0
|
||||
assert stats.prune.old_versions_removed == 3
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ use arrow::{
|
||||
};
|
||||
use lancedb::blob::{BlobFile, BlobRangeRequest};
|
||||
use lancedb::index::scalar::FtsIndexBuilder;
|
||||
use lancedb::table::optimize::validate_cleanup_options;
|
||||
use lancedb::table::{
|
||||
AddDataMode, ColumnAlteration, Duration, FieldMetadataUpdate, FtsToken as LanceDbFtsToken,
|
||||
NewColumnTransform, OptimizeAction, OptimizeOptions, Ref, Table as LanceDbTable,
|
||||
@@ -1214,6 +1215,7 @@ impl Table {
|
||||
} else {
|
||||
None
|
||||
};
|
||||
validate_cleanup_options(older_than, delete_unverified).infer_error()?;
|
||||
future_into_py(self_.py(), async move {
|
||||
let compaction_stats = inner
|
||||
.optimize(OptimizeAction::Compact {
|
||||
|
||||
Reference in New Issue
Block a user