Skip to content

Commit

Permalink
Handle empty file request on dedup store
Browse files Browse the repository at this point in the history
  • Loading branch information
WillDoItMyself committed Oct 7, 2024
1 parent 553f33c commit 7e6c796
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 0 deletions.
7 changes: 7 additions & 0 deletions nativelink-store/src/dedup_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ use tokio_util::codec::FramedRead;
use tokio_util::io::StreamReader;
use tracing::{event, Level};

use crate::cas_utils::is_zero_digest;

// NOTE: If these change update the comments in `stores.rs` to reflect
// the new defaults.
const DEFAULT_MIN_SIZE: u64 = 64 * 1024;
Expand Down Expand Up @@ -159,6 +161,11 @@ impl StoreDriver for DedupStore {
.iter()
.zip(results.iter_mut())
.map(|(key, result)| async move {
if is_zero_digest(key.borrow()) {
*result = Some(0);
return Ok(());
}

match self.has(key.borrow()).await {
Ok(maybe_size) => {
*result = maybe_size;
Expand Down
33 changes: 33 additions & 0 deletions nativelink-store/tests/dedup_store_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use nativelink_error::{Code, Error, ResultExt};
use nativelink_macro::nativelink_test;
use nativelink_store::cas_utils::ZERO_BYTE_DIGESTS;
use nativelink_store::dedup_store::DedupStore;
use nativelink_store::memory_store::MemoryStore;
use nativelink_util::common::DigestInfo;
Expand Down Expand Up @@ -393,3 +394,35 @@ async fn has_with_no_existing_index_returns_none_test() -> Result<(), Error> {
}
Ok(())
}

/// Ensure that when we run a `.has()` on a dedup store to check for empty blobs it will
/// properly return Some(0).
#[nativelink_test]
async fn has_with_zero_digest_returns_some_test() -> Result<(), Error> {
let index_store = MemoryStore::new(&nativelink_config::stores::MemoryStore::default());
let content_store = MemoryStore::new(&nativelink_config::stores::MemoryStore {
eviction_policy: Some(nativelink_config::stores::EvictionPolicy {
max_count: 10,
..Default::default()
}),
});

let store = DedupStore::new(
&make_default_config(),
Store::new(index_store.clone()),
Store::new(content_store.clone()),
)?;

let digest = ZERO_BYTE_DIGESTS[0];

{
let size_info = store.has(digest).await.err_tip(|| "Failed to run .has")?;
assert_eq!(
size_info,
Some(0),
"Expected Sone(0) to be returned, got {:?}",
size_info
);
}
Ok(())
}

0 comments on commit 7e6c796

Please sign in to comment.