bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Hermetic linux sandbox (and Bazel-initiated file changes) creates spurious hashing on rebuild

Open jkeljo opened this issue 2 months ago • 23 comments

Description of the bug:

If you build something locally with the hermetic linux sandbox, then build it again without changing anything, Bazel will re-hash all of the input files to the action before realizing there is nothing to do. This appears to be because hardlinking the inputs into the sandbox touches ctime without updating Bazel's cache of file information, thus making Bazel think the file might have changed.

The same thing happens with MODULE.bazel.lock, though that may be an actual content change. In general, it seems like if Bazel modifies a file, it should update its own filesystem cache to know that it did so.

Which category does this issue belong to?

Local Execution

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Check out https://github.com/jkeljo/bazel-hermetic-sandbox-diffawareness-bug and follow the instructions in the README

Which operating system are you running Bazel on?

Ubuntu 20.04

What is the output of bazel info release?

release 8.4.2

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

[email protected]:jkeljo/bazel-hermetic-sandbox-diffawareness-bug.git
748c41a99f05bbe659d127d6f6edf153649e70ae

If this is a regression, please try to identify the Bazel commit where the bug was introduced with bazelisk --bisect.

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

jkeljo avatar Oct 10 '25 16:10 jkeljo

MODULE.bazel.lock is a different case since the content actually changes (we only write to the file if the content differs, precisely to avoid ctime bumps).

But hardlinks are particularly wasteful right now since we usually wouldn't expect the file to have changed. The only problem is that it seems difficult to tell whether it was really Bazel that caused the ctime update and not an external process.

An alternative would be to introduce a flag that caches digests by mtime/size/node id, but not by ctime.

fmeum avatar Oct 10 '25 16:10 fmeum

I dislike adding a flag that makes Bazel less correct, so it seems that this is a design limitation of Bazel.

Does anyone have other clever ideas?

meisterT avatar Oct 14 '25 13:10 meisterT

Would it be possible for the code that's doing the hardlinking to recognize that the file's ctime matched the cached value before the hardlink was added and update the cache afterwards?

jkeljo avatar Oct 14 '25 15:10 jkeljo

That sounds like a pretty good heuristic, but I don't think it's entirely safe: What if a file is modified while a hardlink to it is created? In that case Bazel wouldn't become aware of the changed contents.

Unless there's a filesystem function that creates a hardlink and atomically returns the new ctime, I don't think this can be made safe. But I would be glad to be convinced otherwise :⁠-⁠)

fmeum avatar Oct 14 '25 16:10 fmeum

If it's a content modification, it would also touch mtime and maybe size. It would have to be another operation that only affects ctime. It would presumably be a pretty short window, but yeah, there would be a window.

jkeljo avatar Oct 14 '25 18:10 jkeljo

We've had issues with moves of files extracted from tars (and thus fixed mtimes) in the past, which resulted in the addition of ctimes to digest caches. This situation is better since the modification would need to happen in the time it takes to link and stat.

fmeum avatar Oct 14 '25 19:10 fmeum

@oquenchil I have a new idea for this: Since https://github.com/bazelbuild/bazel/commit/ec90e05dee8690e89d925e59a590b51e9d6511f7, file copies can use CoW on supported file systems, which should be very fast. Perhaps we could just replace hardlinks with copies?

cc @tjgq

fmeum avatar Nov 04 '25 16:11 fmeum

@fmeum, can you elaborate on "moves of files extracted from tars"?

If we run stat on "path" before and after the following moves:

mv path newLocation
mv otherFile path

then I expect the change to be detected by the different inode numbers, regardless of whether otherFile has the same mtime.

But maybe I misunderstand what is meant by "moves of files"?

ulrfa avatar Nov 04 '25 18:11 ulrfa

The original reproducer for this was https://github.com/bazelbuild/bazel/commit/763f1d976ba679e5b7aebe63a978a4ab80fca310#diff-de81268ae69d5eff8ee8ac19cd3dd94b399828ec7f655a23445fded61e355798. I may very well have missed something back then, so please double-check (for example by dropping ctime from the cache key and keeping the integration tests).

fmeum avatar Nov 04 '25 18:11 fmeum

@ulrfa See https://github.com/bazelbuild/bazel/pull/27527, in which I dropped ctime from the cache key. One test is failing with that.

fmeum avatar Nov 04 '25 18:11 fmeum

Thanks @fmeum, interesting, I take a look.

ulrfa avatar Nov 05 '25 06:11 ulrfa

Two additional remarks regarding the inclusion of ctime in the cache key:

  • I think we might be missing a realistic scenario where not taking the ctime into account would make a difference (the integration test is somewhat contrived): AIUI, both tar and unzip (in their default configuration) extract onto a preexisting file by deleting and recreating it, so the inode would change. Another interesting situation is what happens when you check out a previous version of the file from source control; it appears that git overwrites an existing file without recreating it, but does not restore the mtime (so, from Bazel's perspective, the mtime would also change). In order to remove ctime from the cache key, I think we'd have to convince ourselves that other popular archivers and VCSes are sound in this regard.

  • WindowsFileSystem.stat() implements getChangeTime() but not getNodeId(), so it's also possible that the issue motivating the addition of ctime only existed on Windows (and could also be fixed by implementing getNodeId()).

Replacing hardlinks with copies is also an interesting proposal, though it's not immediately obvious to me that the performance would be comparable (even assuming copy-on-write).

tjgq avatar Nov 05 '25 14:11 tjgq

It was a mystery to me how the inode number could stay the same after tar called unlinkat:

openat(4, "template1", O_WRONLY|O_CREAT|O_EXCL|O_NOCTTY|O_NONBLOCK|O_CLOEXEC, 0644) = -1 EEXIST (File exists)
unlinkat(4, "template1", 0)             = 0
openat(4, "template1", O_WRONLY|O_CREAT|O_EXCL|O_NOCTTY|O_NONBLOCK|O_CLOEXEC, 0644) = 5
write(5, "test : delta\n", 13)          = 13
utimensat(5, NULL, [UTIME_OMIT, {tv_sec=0, tv_nsec=0}], 0) = 0

But now I realize that filesystems can reuse inode numbers from recently unlinked files:

$ touch hello && stat hello && rm -f hello && touch hello && stat hello

File: hello
Size: 0          Blocks: 0          IO Block: 65536  regular empty file
Device: 29h/41d  Inode: 61456208    Links: 1
Access: 2025-11-06 15:56:46.530199000 +0100
Modify: 2025-11-06 15:56:46.530199000 +0100
Change: 2025-11-06 15:56:46.530199000 +0100
 Birth: -
File: hello
Size: 0          Blocks: 0          IO Block: 65536  regular empty file
Device: 29h/41d  Inode: 61456208    Links: 1
Access: 2025-11-06 15:56:46.542165000 +0100
Modify: 2025-11-06 15:56:46.542165000 +0100
Change: 2025-11-06 15:56:46.542165000 +0100
 Birth: -

Perhaps different filesystems have different behavior. I tried the command above with XFS, NFS, and tmpfs, and often got the same inode number again.

I need to think more about this...

ulrfa avatar Nov 06 '25 15:11 ulrfa

The ctime value changes not only when sandboxes create hard links but also when those links are removed. It seems challenging to update the cache in DigestUtils atomically with ctime changes on disk, especially when multiple sandboxes are managed concurrently and share common input files. It might even be impossible if the running actions perform operations in their sandbox that affect the ctime of input files.

Therefore, I am thinking about relying on mtime instead of ctime, and having Bazel ensure that the mtimes are trustworthy for detecting change for all outputs produced by actions and repository rules, including cases when archives with fake mtimes are extracted.

It won’t prevent users, outside of bazel, from manipulating mtime but I believe the risk for that is minimal. And in most cases size or inode would still change. And if there are rare such cases, they probably manipulate by setting mtime=0, and for that special value we could fallback to ctime and lose performance but keep the correctness.

Some VCS set mtime to checkout time and others to commit time, but I'm not aware of any VCS that change file content without also changing mtime, so I believe the ctime check is unnecessary for VCS use cases.

Here is an experiment: #27685

Do you think something like that can work?

ulrfa avatar Nov 16 '25 19:11 ulrfa

This would make stale builds very rare in practice, but it would also become quite hard to explain what kind of file changes are tracked and which aren't.

I would prefer a solution that replaces hardlinks with reflinks (automatically used by FileSystemUtils.copyFile if supported), but I haven't benchmarked it yet.

fmeum avatar Nov 16 '25 22:11 fmeum

This would make stale builds very rare in practice, but it would also become quite hard to explain what kind of file changes are tracked and which aren't.

I would prefer a solution that replaces hardlinks with reflinks (automatically used by FileSystemUtils.copyFile if supported), but I haven't benchmarked it yet.

fmeum avatar Nov 16 '25 22:11 fmeum

but it would also become quite hard to explain what kind of file changes are tracked and which aren't.

What if explained as the following?

  • Files written by Bazel (outputs from actions and repository rules) shall have an mtime corresponding to when they are written to disk.
  • Bazel can reliably detect file changes based on mtime.
  • Bazel can also detect file changes based on other file attributes, but these are not as reliable.

ulrfa avatar Dec 01 '25 10:12 ulrfa

I wonder why Bazel has explicit code to manipulate mtime when archives are extracted? (e.g. via repository_ctx.extract) Wouldn’t it make more sense for Bazel to avoid faking mtime when extracting files? It is good practice to create reproducible archives by storing mtime as 0 inside the archive, but why using those faked mtimes when extracting?

ulrfa avatar Dec 01 '25 10:12 ulrfa

I would prefer a solution that replaces hardlinks with reflinks (automatically used by FileSystemUtils.copyFile if supported), but I haven't benchmarked it yet.

If your benchmark of copy-on-write will show performance on par with hard links, then I agree that it would be an attractive alternative for people using file systems that support copy-on-write.

However, I still think we (also?) need a hard-link–based solution for people who do not have such file systems. Do you agree? (My organization does not have copy-on-write.)

ulrfa avatar Dec 01 '25 10:12 ulrfa

I wonder why Bazel has explicit code to manipulate mtime when archives are extracted?

I don't think Bazel does that, see https://github.com/bazelbuild/bazel/blob/92de49c53bd00bfd0eea28f1ad510f057ffbe6bf/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/ZipDecompressor.java#L172, https://github.com/bazelbuild/bazel/blob/92de49c53bd00bfd0eea28f1ad510f057ffbe6bf/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/SevenZDecompressor.java#L140 and https://github.com/bazelbuild/bazel/blob/92de49c53bd00bfd0eea28f1ad510f057ffbe6bf/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/CompressedTarFunction.java#L145.

fmeum avatar Dec 01 '25 12:12 fmeum

However, I still think we (also?) need a hard-link–based solution for people who do not have such file systems. Do you agree? (My organization does not have copy-on-write.)

I agree that hardlink based sandboxes should stay around even if CoW is favored going forward. Since incremental correctness is by far the most important quality of Bazel, I'm not sure whether invalidation schemes with known (even if rare) failure modes are justified, especially if there is a performant alternative that only requires some additional setup. But let me benchmark this first, maybe CoW isn't the solution we are looking for.

fmeum avatar Dec 01 '25 12:12 fmeum

I don't think Bazel does that, see

bazel/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/ZipDecompressor.java

Line 172 in 92de49c

outputPath.setLastModifiedTime(entry.getTime()); ...

Maybe we have different interpretations of the word “manipulation,” because those exact lines are what I meant by “explicit code to manipulate mtime ...”

Question: Are there use cases where it is important that those lines call setLastModifiedTime()? Instead of simply going along with the automatic mtime set by the underlying filesystem for the moment the repository rule executes.

Consider extracting an archive as part of an action. When action outputs are downloaded from a remote cache, Bazel doesn’t call setLastModifiedTime() on the extracted files, even if the archive contains an mtime. Would getting similar result also for archives extracted by repository rules be acceptable?

ulrfa avatar Dec 01 '25 16:12 ulrfa

Yeah, most repo rules probably don't care about the modification times. But since repo rules are the one place where you can freely run arbitrary tools, it's hard to say for certain that this wouldn't break someone's repo rule. We could ask in Bazel Slack.

fmeum avatar Dec 01 '25 21:12 fmeum