Fix bug in redis store when zero data stored but data does not exist #1304
Conversation
There was a problem hiding this comment.
Reviewable status: 0 of 1 LGTMs obtained, and 0 of 2 files reviewed, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Local / ubuntu-22.04, NativeLink.com Cloud / Remote Cache (Legacy Dockerfile Test), Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (22.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, vale, windows-2022 / stable (waiting on @adam-singer)
Note: This is unlikely to have been observed from the RBE API. We are supposed to check if an item exists even if it has zero bytes stored in it, but since redis is quite tricky to do this, we were only checking existance if it was a zero byte digest or if the length was zero bytes. In this case we might have a zero-byte proto that is actually valid that does not follow the CAS rules (hash of contents equals key). This is in prep for a Redis Scheduler. fixes TraceMachina#1286
allada
force-pushed
the
fix-redis-get-part-bug
branch
from
eff9e0e to
8f26eb7
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 1 LGTMs obtained, and 0 of 2 files reviewed, and pending CI: pre-commit-checks (waiting on @adam-singer)
nativelink-store/src/redis_store.rs line 118 at r1 (raw file):
) -> Result<Self, Error> { let client_pool = builder .set_performance_config(PerformanceConfig {
fyi: Gunna sneak this in here, it is not really related to the other PRs, but in theory could happen.
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 1 of 1 LGTMs obtained, and all files reviewed
Note: This is unlikely to have been observed from the RBE API.
We are supposed to check if an item exists even if it has zero bytes stored in it, but since redis is quite tricky to do this, we were only checking existance if it was a zero byte digest or if the length was zero bytes. In this case we might have a zero-byte proto that is actually valid that does not follow the CAS rules (hash of contents equals key).
This is in prep for a Redis Scheduler.
fixes #1286
This change is