-
Notifications
You must be signed in to change notification settings - Fork 153
UDF Checkpoints cleanup #1454
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UDF Checkpoints cleanup #1454
Changes from 39 commits
7f876e8
a5c4572
70a44a6
f4c848b
d8a337a
d7b5ed9
8752a9a
862fe28
b599429
3804b0c
7c05e0d
20346e7
e0242c9
8fd41af
630e37b
b31d44a
b5bb8cd
a5f0fcd
92590f7
3c2211d
181ea2e
88c2648
c0f46cb
14e473b
d68d746
8e0339f
08c9ec4
bb50da7
bd7d978
4ddee8f
5b80a87
9a6c71f
a2d6b34
3621cde
437b63c
76125d0
c644aa1
5f6f183
91f7da5
709873c
e180338
aaf43f9
1488fab
d7f3a50
8a2ec11
4379d11
27033e5
d73d55d
b15f1c9
fd5019e
e4e6de9
1e7f941
6a5c140
f914b7f
91aa89c
452ae72
2375237
55b3846
87a51f3
43835f5
911a3dc
ad2907d
0b3e092
e909118
7bbe619
fa5053b
140e56b
54c4493
e9f48f5
38fa81d
8ab52ae
ced14b4
0f88905
6f7e06f
af784f2
7703eb5
4a7b4ca
a95a764
8876a3b
13f3552
157a437
c9d3bb0
2227fe1
f459d60
f93b49d
a2a9a98
631cdb3
49e7641
f50058f
d11fb5d
96f9de9
ec98372
c77858a
9a51f9c
298bcf3
aa11f80
ee464fb
e2ab50b
9cb16c7
3685dca
eba46b5
e58d742
1feac6d
13c6aa0
a878df6
c61a13b
d88d68a
abccfbc
809e9a3
ab6799f
fa00047
da8fd5b
115ea69
334f5fb
cb18ee4
d1f83f1
79655e7
b93f328
4352423
2504498
97fe6ae
26f1eef
5049e96
985415d
31fe6dd
f117751
15751ba
85305b0
cafeadf
a022bc1
27781df
05cf600
af0dc7f
b108dc8
24c3894
eb0e03a
9d93aad
e8ec502
3bcfa18
4dd9cd4
8fdfea2
34402f4
8ea8b12
acf79c8
585d685
adb828e
797b6cd
505f304
d702b21
8df4905
659cc1c
b90a9d6
3431c10
19093a3
6803345
8c57340
d25b5af
7a44193
e267deb
bee6e0f
0144c1a
c457bf9
15650eb
495f189
8e6b2e5
56c6b78
8b16be4
88048de
e903715
79c94f2
a83fafe
1d77ac2
0de0d3f
8b8a8d3
0f61ea7
25def9c
5a70e41
12a771c
41847c9
6279013
85e0c42
3e6601c
a9358d1
22ebd7d
1a04a7c
5998b8e
e83884b
6e18191
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |||||||||
| from collections.abc import Callable, Iterable, Iterator, Mapping, Sequence | ||||||||||
| from copy import copy | ||||||||||
| from dataclasses import dataclass | ||||||||||
| from datetime import datetime, timedelta, timezone | ||||||||||
| from functools import cached_property, reduce | ||||||||||
| from threading import Thread | ||||||||||
| from typing import IO, TYPE_CHECKING, Any, NoReturn | ||||||||||
|
|
@@ -22,6 +23,7 @@ | |||||||||
| from tqdm.auto import tqdm | ||||||||||
|
|
||||||||||
| from datachain.cache import Cache | ||||||||||
| from datachain.checkpoint import Checkpoint | ||||||||||
| from datachain.client import Client | ||||||||||
| from datachain.dataset import ( | ||||||||||
| DATASET_PREFIX, | ||||||||||
|
|
@@ -2039,3 +2041,66 @@ def index( | |||||||||
| client_config=client_config or self.client_config, | ||||||||||
| only_index=True, | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| def _remove_checkpoint(self, checkpoint: Checkpoint) -> None: | ||||||||||
| """ | ||||||||||
| Remove a checkpoint and its associated job-specific UDF tables. | ||||||||||
|
|
||||||||||
| Since tables are now job-scoped, this removes only the tables | ||||||||||
| belonging to this specific checkpoint's job. | ||||||||||
|
|
||||||||||
| Args: | ||||||||||
| checkpoint: The checkpoint object to remove. | ||||||||||
| """ | ||||||||||
| # Remove the checkpoint from metastore first | ||||||||||
| self.metastore.remove_checkpoint(checkpoint) | ||||||||||
|
|
||||||||||
| # Remove job-specific tables for this checkpoint | ||||||||||
| # Table patterns: udf_{job_id}_{hash}_{suffix} | ||||||||||
| # where suffix can be: input, output, output_partial, processed | ||||||||||
| job_id_sanitized = checkpoint.job_id.replace("-", "") | ||||||||||
| table_prefix = f"udf_{job_id_sanitized}_{checkpoint.hash}_" | ||||||||||
| matching_tables = self.warehouse.db.list_tables(prefix=table_prefix) | ||||||||||
|
|
||||||||||
| if matching_tables: | ||||||||||
|
Comment on lines
+2058
to
+2060
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (code-quality): Use named expression to simplify assignment and conditional (
Suggested change
|
||||||||||
| self.warehouse.cleanup_tables(matching_tables) | ||||||||||
|
|
||||||||||
| def cleanup_checkpoints(self, ttl_seconds: int | None = None) -> None: | ||||||||||
| """ | ||||||||||
| Clean up outdated checkpoints and their associated UDF tables. | ||||||||||
|
|
||||||||||
| Uses optimized branch pruning: removes outdated checkpoints if no | ||||||||||
| descendants have active (non-outdated) checkpoints that depend on them. | ||||||||||
|
|
||||||||||
| This prevents accumulation of checkpoints while ensuring that ancestor | ||||||||||
| tables are preserved when descendants still need them. | ||||||||||
|
|
||||||||||
| Args: | ||||||||||
| ttl_seconds: Time-to-live in seconds. Checkpoints older than this | ||||||||||
| are considered outdated. If None, uses CHECKPOINT_TTL | ||||||||||
| environment variable or default. | ||||||||||
| """ | ||||||||||
| if ttl_seconds is None: | ||||||||||
| ttl_seconds = int(os.environ.get("CHECKPOINT_TTL", str(TTL_INT))) | ||||||||||
|
|
||||||||||
| ttl_threshold = datetime.now(timezone.utc) - timedelta(seconds=ttl_seconds) | ||||||||||
|
|
||||||||||
| # Cache descendant check results per job_id to avoid redundant checks | ||||||||||
| has_active_descendants_cache: dict[str, bool] = {} | ||||||||||
|
|
||||||||||
| # For each outdated checkpoint, check if it's safe to remove | ||||||||||
| for ch in self.metastore.list_checkpoints(created_before=ttl_threshold): | ||||||||||
| # Check once per job_id if descendants have active checkpoints (cached) | ||||||||||
| if ch.job_id not in has_active_descendants_cache: | ||||||||||
| has_active_descendants_cache[ch.job_id] = any( | ||||||||||
| list( | ||||||||||
| self.metastore.list_checkpoints( | ||||||||||
| desc_id, created_after=ttl_threshold | ||||||||||
| ) | ||||||||||
| ) | ||||||||||
| for desc_id in self.metastore.get_descendant_job_ids(ch.job_id) | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| # If no active descendants, remove the checkpoint | ||||||||||
| if not has_active_descendants_cache[ch.job_id]: | ||||||||||
| self._remove_checkpoint(ch) | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,12 +12,18 @@ def clear_cache(catalog: "Catalog"): | |||||||||||||||
|
|
||||||||||||||||
| def garbage_collect(catalog: "Catalog"): | ||||||||||||||||
| temp_tables = catalog.get_temp_table_names() | ||||||||||||||||
| if not temp_tables: | ||||||||||||||||
| print("Nothing to clean up.") | ||||||||||||||||
| else: | ||||||||||||||||
| print(f"Garbage collecting {len(temp_tables)} tables.") | ||||||||||||||||
| has_tables = bool(temp_tables) | ||||||||||||||||
|
|
||||||||||||||||
| if has_tables: | ||||||||||||||||
| print(f"Garbage collecting {len(temp_tables)} temporary tables.") | ||||||||||||||||
| catalog.cleanup_tables(temp_tables) | ||||||||||||||||
|
|
||||||||||||||||
| print("Cleaning up outdated checkpoints.") | ||||||||||||||||
| catalog.cleanup_checkpoints() | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (bug_risk): Consider handling and reporting errors from cleanup_checkpoints. Wrap catalog.cleanup_checkpoints() in a try/except block to prevent abrupt termination and provide user-friendly error reporting.
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| if not has_tables: | ||||||||||||||||
| print("No temporary tables to clean up.") | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def completion(shell: str) -> str: | ||||||||||||||||
| from datachain.cli import get_parser | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Sanitizing job_id by removing hyphens may not be sufficient for table name safety.
Consider validating job_id to exclude all invalid characters and reserved words to ensure table names remain safe and compliant.
Suggested implementation: