Skip to content

add cache key debuginfo lookup#6061

Merged
tonistiigi merged 3 commits into
moby:masterfrom
tonistiigi:cache-debuginfo-db
Jul 11, 2025
Merged

add cache key debuginfo lookup#6061
tonistiigi merged 3 commits into
moby:masterfrom
tonistiigi:cache-debuginfo-db

Conversation

@tonistiigi
Copy link
Copy Markdown
Member

This allows opt-in to cache key debug database on
daemon startup.

If enabled, all cache keys generated by builds are saved into this database together with the plaintexts of the original data so a reverse lookup can be performed later to compare two checksums and find out their original difference. If checksum contains other checksums internally then these are saved as well. For storage constraints, the plaintext of file content is not saved but the metadata portion can be still looked up.

This allows opt-in to cache key debug database on
daemon startup.

If enabled, all cache keys generated by builds are
saved into this database together with the plaintexts
of the original data so a reverse lookup can be performed
later to compare two checksums and find out their original
difference. If checksum contains other checksums internally
then these are saved as well. For storage constraints, the
plaintext of file content is not saved but the metadata
portion can be still looked up.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi force-pushed the cache-debuginfo-db branch from 99ffe37 to 4c9d94f Compare July 1, 2025 06:29
@tonistiigi tonistiigi marked this pull request as ready for review July 3, 2025 00:27
@tonistiigi tonistiigi marked this pull request as draft July 8, 2025 00:34
These endpoints show the contents of current boltdb
cache database together with debug plaintexts if
they have been saved.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi marked this pull request as ready for review July 8, 2025 07:06
@tonistiigi
Copy link
Copy Markdown
Member Author

Fixed a bug where random: prefix got lost in some cachekeys and added a new commit with support for printing out cache chains from boltdb.

Comment thread cmd/buildkitd/main.go
Usage: "list of directories to scan for CDI spec files",
},
cli.BoolFlag{
Name: "save-cache-debug",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Name: "save-cache-debug",
Name: "debug-save-cache",

If more debug-* flags are to come

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--debug-save-cache would mean something about storing the cache itself. Atm it means "save-cache-debuginfo". If we want --debug prefix, then it should be smth like --debug-save-cache-plaintexts

Comment thread util/cachedigest/digest.go Outdated
Comment on lines +19 to +21
TypeStringArray Type = "string-array"
TypeDigestArray Type = "digest-array"
TypeFileList Type = "file-list"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we be consistent? Maybe -list instead of array?

Suggested change
TypeStringArray Type = "string-array"
TypeDigestArray Type = "digest-array"
TypeFileList Type = "file-list"
TypeStringList Type = "string-list"
TypeDigestList Type = "digest-list"
TypeFileList Type = "file-list"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can change this but logically these are different. File-list is a special format for file headers and checksum per file, not delimiter-separated.

Comment thread util/cachedigest/db.go
Comment on lines +89 to +91
if d.db == nil {
return "", nil, errors.WithStack(ErrNotFound)
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ErrNotFound might be misleading if db not set. Maybe ErrNoDB would be more accurate?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modeled so that DB can be called all the time. In the future, there may be other implementations. So the internals of whether the plaintext is not found because it was not saved for a specific record or for the whole database shouldn't matter. Note that because lookup and store happen at different times, it may be that one of them had DB configured and another one did not.

@crazy-max
Copy link
Copy Markdown
Member

Fixed a bug where random: prefix got lost in some cachekeys

Is it related to https://github.com/moby/buildkit/pull/6061/files#diff-d44a0b2b1649ed2262e65d607ac40667f9e287a289484f2b26e22c96297f9ebaR43 ?

@tonistiigi
Copy link
Copy Markdown
Member Author

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi merged commit 0506df9 into moby:master Jul 11, 2025
168 of 169 checks passed
@crazy-max crazy-max added this to the v0.24.0 milestone Aug 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants