diff --git a/python/python/lancedb/table.py b/python/python/lancedb/table.py index 0dc927b29..8068e73d2 100644 --- a/python/python/lancedb/table.py +++ b/python/python/lancedb/table.py @@ -134,6 +134,34 @@ _MODEL_BACKED_TOKENIZER_ERRORS = ( ) +def _optimize_cleanup_since_ms( + cleanup_older_than: Optional[timedelta], retrain: bool +) -> Optional[int]: + # Called directly by both the sync and async optimize so stacklevel=3 + # names the user's call site rather than the background event loop. + cleanup_since_ms: Optional[int] = None + if cleanup_older_than is not None: + cleanup_since_ms = round(cleanup_older_than.total_seconds() * 1000) + if cleanup_since_ms <= 0: + warnings.warn( + "optimize(cleanup_older_than=0) removes every version except " + "the latest. Any concurrent reader or writer still using an " + "older version will fail. Use a longer cleanup_older_than " + "unless no other process is working on this table.", + UserWarning, + stacklevel=3, + ) + + if retrain: + warnings.warn( + "The 'retrain' parameter is deprecated and will be removed in a " + "future version.", + DeprecationWarning, + stacklevel=3, + ) + return cleanup_since_ms + + def _add_unique_note(exception: BaseException, note: str) -> None: existing_notes = getattr(exception, "__notes__", ()) or () message = ( @@ -2217,6 +2245,13 @@ class Table(ABC): 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. + + .. warning:: + + Setting this to 0 deletes the data files of every older + version, so any other reader or writer still using an older + version of the table will fail. Only set it to 0 if no other + process is working on this 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 @@ -4423,6 +4458,13 @@ class LanceTable(Table): 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. + + .. warning:: + + Setting this to 0 deletes the data files of every older + version, so any other reader or writer still using an older + version of the table will fail. Only set it to 0 if no other + process is working on this 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 @@ -4447,10 +4489,9 @@ class LanceTable(Table): modification operations. """ LOOP.run( - self._table.optimize( - cleanup_older_than=cleanup_older_than, - delete_unverified=delete_unverified, - retrain=retrain, + self._table._do_optimize( + _optimize_cleanup_since_ms(cleanup_older_than, retrain), + delete_unverified, ) ) @@ -6958,6 +6999,13 @@ class AsyncTable: 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. + + .. warning:: + + Setting this to 0 deletes the data files of every older + version, so any other reader or writer still using an older + version of the table will fail. Only set it to 0 if no other + process is working on this 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 @@ -6981,20 +7029,14 @@ class AsyncTable: you have added or modified 100,000 or more records or run more than 20 data modification operations. """ - cleanup_since_ms: Optional[int] = None - if cleanup_older_than is not None: - cleanup_since_ms = round(cleanup_older_than.total_seconds() * 1000) - - if retrain: - import warnings - - warnings.warn( - "The 'retrain' parameter is deprecated and will be removed in a " - "future version.", - DeprecationWarning, - stacklevel=2, - ) + return await self._do_optimize( + _optimize_cleanup_since_ms(cleanup_older_than, retrain), + delete_unverified, + ) + async def _do_optimize( + self, cleanup_since_ms: Optional[int], delete_unverified: bool + ) -> OptimizeStats: return await self._inner.optimize( cleanup_since_ms=cleanup_since_ms, delete_unverified=delete_unverified, diff --git a/python/python/tests/test_table.py b/python/python/tests/test_table.py index 5c56fea38..fa1a494b0 100644 --- a/python/python/tests/test_table.py +++ b/python/python/tests/test_table.py @@ -4307,7 +4307,8 @@ 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)) + with pytest.warns(UserWarning, match="concurrent"): + stats = await table.optimize(cleanup_older_than=timedelta(seconds=0)) assert stats.prune.bytes_removed > 0 assert stats.prune.old_versions_removed == 3 @@ -4335,12 +4336,35 @@ async def test_optimize_delete_unverified(tmp_db_async: AsyncConnection, tmp_pat stats = await table.optimize(delete_unverified=False) assert stats.prune.old_versions_removed == 0 - stats = await table.optimize( - cleanup_older_than=timedelta(seconds=0), delete_unverified=True - ) + with pytest.warns(UserWarning, match="concurrent"): + stats = await table.optimize( + cleanup_older_than=timedelta(seconds=0), delete_unverified=True + ) assert stats.prune.old_versions_removed == 2 +@pytest.mark.asyncio +async def test_optimize_warns_on_zero_cleanup(mem_db_async: AsyncConnection): + table = await mem_db_async.create_table("test", data=[{"x": [1]}]) + with pytest.warns(UserWarning, match="concurrent"): + await table.optimize(cleanup_older_than=timedelta(0)) + + with warnings.catch_warnings(): + warnings.simplefilter("error") + await table.optimize(cleanup_older_than=timedelta(days=1)) + await table.optimize() + + +def test_optimize_warns_on_zero_cleanup_sync(mem_db: DBConnection): + table = mem_db.create_table("test", data=[{"x": [1]}]) + with warnings.catch_warnings(record=True) as seen: + warnings.simplefilter("default") + table.optimize(cleanup_older_than=timedelta(0)) + table.optimize(cleanup_older_than=timedelta(0)) + assert [w.filename for w in seen] == [__file__, __file__] + assert all("concurrent" in str(w.message) for w in seen) + + def test_replace_field_metadata(tmp_path): db = lancedb.connect(tmp_path) table = db.create_table("my_table", data=[{"x": 0}])