Repository: TraceMachina/nativelink (language: rust)

Issue:
Empty digests not handled properly - still
It's related to #498. Fix there was covering simple cases, still leaving a lot of to fix/cover, creating extra unpredictable issues when using advanced configs.

Example config:
```
"CAS_MAIN_STORE": {                                                                       
      "dedup": {              
        // Index store contains the references to the chunks of data and how to
        // reassemble them live. These will usually be <1% of the total size of
        // the object being indexed.
        "index_store": {
          "filesystem": {
            "content_path": ...
            "temp_path": ...
            "eviction_policy": { 
              // 1gb.
              "max_bytes": 10000000,
            }
          }
        },
        // This is where the actual content will be stored, but will be in small
        // files chunked into different sizes based on the "*_size" settings below.
        "content_store": {
          // Then apply a compression configuration to the individual file chunks.
          "compression": {
            "compression_algorithm": {
              "lz4": {}
            },
            "backend": {
              // Then take those compressed chunks and store them to the filesystem.
                "filesystem": {
                        "content_path": ...
                        "temp_path": ...
                                               
                "eviction_policy": { 
                // 100gb.
                "max_bytes": 10000000000,
                }      
                } 
              }
            }        
          }            
        },                         
        // The file will not be chunked into parts smaller than this (64k).
        "min_size": 65536,
        // The file will attempt to be chunked into about this size (128k).
        "normal_size": 131072,
        // No chunk should be larger than this size (256k).
        "max_size": 262144
      },
      "AC_MAIN_STORE": {
       "completeness_checking": {
         "backend": {
            "filesystem": {
            "content_path": ...
            "temp_path": ...
            "eviction_policy": {
              // 1gb.
              "max_bytes": 1000000,
            }
          }
       },
      "cas_store": {
       "ref_store": {
        "name": "CAS_MAIN_STORE"
       }
     }
   }
  }

```
  
Problem here is:
1. completeness_checking asks CAS if it has empty file
2. dedup with filesystem index_store asks filesystem if it has empty file
3. filesystem creates handles this request by creating empty file under dedup structures
4. dedup as next step verifies it can deserialize file created by step 3
5. deserialization fails with EOF error
6. completness_checking fails - returns not found
7. build starts instead of taking results from cache

Possible solutions:
1. extend and verify dedup (or even all) store layers if they can handle empty digest - for just dedup, as temporary workaround sound reasonable
2. but long term I think that should not be handled by stores itself, more closer to cas_server - basically before calling any store with request has/upload/get_part_with_resposone - maybe adding extra layer which will handle it or store_trait.rs? 


Discussion / hints:

Fix patch:
diff --git a/nativelink-store/src/dedup_store.rs b/nativelink-store/src/dedup_store.rs
index ebacad27d..dcd14f92d 100644
--- a/nativelink-store/src/dedup_store.rs
+++ b/nativelink-store/src/dedup_store.rs
@@ -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;
@@ -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;


Test patch:
diff --git a/nativelink-store/tests/dedup_store_test.rs b/nativelink-store/tests/dedup_store_test.rs
index a19c6a8ec..d2db4c912 100644
--- a/nativelink-store/tests/dedup_store_test.rs
+++ b/nativelink-store/tests/dedup_store_test.rs
@@ -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;
@@ -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(())
+}