aicam commented on code in PR #8666:
URL: https://github.com/apache/texera/pull/8666#discussion_r4099607492
##########
bin/mounter/mounter.py:
##########
@@ -237,6 +240,14 @@ def do_mount(cuid, repo, commit, jwt, file_service_base):
"--endpoint", file_service_base,
"--memory-limit", "512",
"-o", "ro,allow_other",
+ ]
+ if CACHE_ROOT:
+ # Keyed by repository and commit, not by computing unit: a commit is
immutable, so
+ # every unit on this node reads one cache rather than each new unit
starting cold.
+ cache_dir = os.path.join(CACHE_ROOT, repo, commit)
+ os.makedirs(cache_dir, exist_ok=True)
Review Comment:
Good catch — yes it does. `os.makedirs` creates every missing parent, so
with a cache root that is not there it would have created the whole chain
rather than reporting it.
That is worse than untidy here. The chart mounts `cacheRoot` as a volume, so
a missing root means the volume is missing — and creating it puts the cache in
the container's own writable layer, where it is lost on every restart and can
quietly fill the node's disk with hundreds of GB.
Fixed in f8ea626: the root is now required to exist and the mount is refused
if it is not, naming the likely cause. Only the repository and commit levels
beneath it are created, which is what `makedirs` is actually wanted for. Added
a test that a missing root raises and is not created; the happy-path test now
creates the root first, as the volume would.
Worth noting for the record: `repo` and `commit` are already put through
`validated_segment` a few lines above, so neither can escape the root.
I did consider falling back to an uncached mount instead of refusing.
Refusing seemed better for an opt-in setting — an operator who sets `cacheRoot`
wants caching, and silently not caching returns them to the slow path this PR
exists to fix. Happy to switch it if you would rather a cache misconfiguration
never blocked data access.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
