[simonchrz]

Cached metadata was stored at _metadata/<ecosystem>/<key>/metadata. Cache keys are case-sensitive, but with file:// storage on a case-insensitive filesystem (APFS, NTFS), keys that differ only in case shared one file. github.com/Sirupsen/logrus and github.com/sirupsen/logrus got separate database rows, but whichever was cached last was served for both. The same applies to legacy mixed-case npm names, Maven, gem and CRAN. Artifacts weren't affected because their paths carry a fetch ID.

The path now uses a SHA-256 of the exact key: _metadata/<ecosystem>/<sha256>/metadata. This also avoids keys that are Windows reserved names, end in a dot, or contain /. Keys that are already SHA-256 hex digests (APK, generic, Helm, Homebrew, Swift, container and Go sumdb keys) keep their path, so nothing changes for them. Everything else reads the path from the cache row, so eviction, size accounting and reclaim keep working unchanged.

For other keys, an existing row still points at the old path, and on an affected filesystem its blob may hold another key's document. The first lookup of such a row hashes its blob and compares it with the row's content_digest. If they match, the blob is this key's document: it is copied to the new path and the row moved there, so it keeps being served, including as a fallback when the upstream is down. If they don't match, or the row has no digest, the row is cleared and the document fetched again. Once a row has moved off its old path, that old blob is queued in pending_deletes. Reclaim skips it while a row still points at it. On a case-insensitive filesystem a key differing only in case that still reads its own old path (the same file) can lose its blob that way, and is then fetched again.

Tests: TestMetadataKeysDifferingInCaseCachedApart caches two keys that differ only in case on file storage, and fails without the fix on macOS. TestMetadataStoragePathKeepsCaseApart checks that the two paths differ even when compared case-insensitively, so it fails on any OS. TestLegacyMetadataMatchingBlobServedWhenUpstreamDown and TestLegacyMetadataMatchingBlobServedFromCache cover an old row whose blob matches, TestLegacyMetadataMismatchedBlobNotServed and TestLegacyMetadataPathRefetched one whose blob doesn't match or has no digest, and TestLegacyMetadataCaseVariantRefetchedAfterReclaim that, on storage that folds case, a case variant whose shared old blob was reclaimed is fetched again rather than served.

[andrew]

dropLegacyMetadataEntry ignores every row still at its old path, so on upgrade every deployment drops its whole metadata cache for unhashed keys (npm, PyPI, gem, Go, Maven and the rest), including Linux and S3 deployments where keys never collided. Until each document is fetched again, it is neither served fresh nor used as a stale fallback, so an instance upgraded while its upstream is unreachable serves nothing it had cached.

The row already has content_digest. Instead of dropping a legacy row, please hash its blob and compare: when it matches, the blob is this key's document and can be served, or moved to the hashed path. Only a mismatch, where another key's document overwrote it, needs dropping. Please add a test that a legacy row with a matching blob is still served when the upstream is down.

[simonchrz]

Thanks, good catch. A legacy row's blob is now hashed once and compared with its content_digest; when it matches, the blob is copied to the hashed path and the row moved there, so it's served as before, including as a stale fallback. Only a mismatch or a row without a digest is dropped and fetched again. TestLegacyMetadataMatchingBlobServedWhenUpstreamDown covers the upstream-down case.

[andrew]

Thanks, keeping legacy rows whose blob matches the digest is what I was after, and the upstream-down test covers it.

The case-variant lookup is more than this needs. MetadataCaseVariants filters on lower(name) = lower(?), which no index covers, so each legacy row's first lookup after upgrading scans every metadata_cache row in its ecosystem. Reclaim already makes the check it's there for: GetDuePendingDeletes skips any queued path a metadata_cache row still points at. So migrateLegacyMetadataEntry can just QueuePendingDelete(legacy) for its own row once the row has been moved or forgotten, and queueLegacyMetadataBlobs and MetadataCaseVariants can go.

On case-insensitive file storage that means a case variant still at its legacy path can lose its blob to reclaim and fetch the document again. That's fine: those keys were already overwriting each other's documents, and on case-sensitive storage the exact path match is right. Please change TestLegacyMetadataSharedPathKeptForCaseVariant to check that such a variant is fetched again rather than served.

The doc comment on migrateLegacyMetadataEntry walks through every branch, which the code already shows. Please cut it down to two or three lines on why the blob is checked against the digest.

[simonchrz]

Done in 25f62da. MetadataCaseVariants and queueLegacyMetadataBlobs are gone: once a row has moved or been forgotten, migrateLegacyMetadataEntry queues just its own legacy blob and leaves the rest to reclaim. The case-variant test is now TestLegacyMetadataCaseVariantRefetchedAfterReclaim, which runs on storage that folds case and checks that the variant is fetched again after reclaim, and the doc comment is down to three lines.