Make max_bytes_per_stream optional in config (#474)

Defaults "max_bytes_per_stream" to 64Kib in config file.
TraceMachina · Dec 12, 2023 · a01a552 · a01a552
1 parent 931e721
commit a01a552
Show file tree

Hide file tree

Showing 10 changed files with 24 additions and 33 deletions.
diff --git a/deployment-examples/docker-compose/local-storage-cas.json b/deployment-examples/docker-compose/local-storage-cas.json
@@ -56,9 +56,7 @@
       "bytestream": {
         "cas_stores": {
           "main": "CAS_MAIN_STORE",
-        },
-        // According to https://github.com/grpc/grpc.github.io/issues/371 16KiB - 64KiB is optimal.
-        "max_bytes_per_stream": 64000, // 64kb.
+        }
       }
     }
   }, {
@@ -91,9 +89,7 @@
       "bytestream": {
         "cas_stores": {
           "main": "CAS_MAIN_STORE",
-        },
-        // According to https://github.com/grpc/grpc.github.io/issues/371 16KiB - 64KiB is optimal.
-        "max_bytes_per_stream": 64000, // 64kb.
+        }
       }
     }
   }]

diff --git a/deployment-examples/terraform/AWS/scripts/cas.json b/deployment-examples/terraform/AWS/scripts/cas.json
@@ -140,9 +140,7 @@
       "bytestream": {
         "cas_stores": {
           "main": "CAS_S3_STORE",
-        },
-        // According to https://github.com/grpc/grpc.github.io/issues/371 16KiB - 64KiB is optimal.
-        "max_bytes_per_stream": 64000, // 64kb.
+        }
       }
     }
   }]

diff --git a/deployment-examples/terraform/GCP/module/scripts/browser_proxy.json b/deployment-examples/terraform/GCP/module/scripts/browser_proxy.json
@@ -61,9 +61,7 @@
       "bytestream": {
         "cas_stores": {
           "main": "CAS_STORE"
-        },
-        // According to https://github.com/grpc/grpc.github.io/issues/371 16KiB - 64KiB is optimal.
-        "max_bytes_per_stream": 64000 // 64kb.
+        }
       }
     }
   }]

diff --git a/deployment-examples/terraform/GCP/module/scripts/cas.json b/deployment-examples/terraform/GCP/module/scripts/cas.json
@@ -107,9 +107,7 @@
       "bytestream": {
         "cas_stores": {
           "main": "CAS_STORE"
-        },
-        // According to https://github.com/grpc/grpc.github.io/issues/371 16KiB - 64KiB is optimal.
-        "max_bytes_per_stream": 64000 // 64kb.
+        }
       }
     }
   }, {
@@ -139,9 +137,7 @@
       "bytestream": {
         "cas_stores": {
           "main": "CAS_STORE"
-        },
-        // According to https://github.com/grpc/grpc.github.io/issues/371 16KiB - 64KiB is optimal.
-        "max_bytes_per_stream": 64000 // 64kb.
+        }
       }
     }
   }]

diff --git a/nativelink-config/README.md b/nativelink-config/README.md
@@ -53,10 +53,7 @@ A very basic configuration that is a pure in-memory store is:
       "bytestream": {
         "cas_stores": {
           "main": "CAS_MAIN_STORE",
-        },
-        // According to https://github.com/grpc/grpc.github.io/issues/371
-        // 16KiB - 64KiB is optimal.
-        "max_bytes_per_stream": 64000, // 64kb.
+        }
       }
     }
   }]

diff --git a/nativelink-config/examples/basic_cas.json b/nativelink-config/examples/basic_cas.json
@@ -119,9 +119,7 @@
       "bytestream": {
         "cas_stores": {
           "main": "WORKER_FAST_SLOW_STORE",
-        },
-        // According to https://github.com/grpc/grpc.github.io/issues/371 16KiB - 64KiB is optimal.
-        "max_bytes_per_stream": 64000, // 64kb.
+        }
       }
     }
   }, {

diff --git a/nativelink-config/examples/filesystem_cas.json b/nativelink-config/examples/filesystem_cas.json
@@ -142,9 +142,7 @@
       "bytestream": {
         "cas_stores": {
           "main": "CAS_MAIN_STORE",
-        },
-        // According to https://github.com/grpc/grpc.github.io/issues/371 16KiB - 64KiB is optimal.
-        "max_bytes_per_stream": 64000, // 64kb.
+        }
       }
     }
   }, {

diff --git a/nativelink-config/examples/s3_backend_with_local_fast_cas.json b/nativelink-config/examples/s3_backend_with_local_fast_cas.json
@@ -157,9 +157,7 @@
       "bytestream": {
         "cas_stores": {
           "main": "CAS_MAIN_STORE",
-        },
-        // According to https://github.com/grpc/grpc.github.io/issues/371 16KiB - 64KiB is optimal.
-        "max_bytes_per_stream": 64000, // 64kb.
+        }
       }
     }
   }]

diff --git a/nativelink-config/src/cas_server.rs b/nativelink-config/src/cas_server.rs
@@ -120,7 +120,11 @@ pub struct ByteStreamConfig {
     pub cas_stores: HashMap<InstanceName, StoreRefName>,
 
     /// Max number of bytes to send on each grpc stream chunk.
-    #[serde(deserialize_with = "convert_numeric_with_shellexpand")]
+    /// According to <https://github.com/grpc/grpc.github.io/issues/371>
+    /// 16KiB - 64KiB is optimal.
+    ///
+    /// Defaults: 64KiB
+    #[serde(default, deserialize_with = "convert_numeric_with_shellexpand")]
     pub max_bytes_per_stream: usize,
 
     /// In the event a client disconnects while uploading a blob, we will hold

diff --git a/nativelink-service/src/bytestream_server.rs b/nativelink-service/src/bytestream_server.rs
@@ -45,6 +45,9 @@ use tracing::{enabled, error, info, Level};
 /// If this value changes update the documentation in the config definition.
 const DEFAULT_PERSIST_STREAM_ON_DISCONNECT_TIMEOUT: Duration = Duration::from_secs(60);
 
+/// If this value changes update the documentation in the config definition.
+const DEFAULT_MAX_BYTES_PER_STREAM: usize = 64 * 1024;
+
 type ReadStream = Pin<Box<dyn Stream<Item = Result<ReadResponse, Status>> + Send + 'static>>;
 type StoreUpdateFuture = Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'static>>;
 
@@ -170,9 +173,14 @@ impl ByteStreamServer {
                 .ok_or_else(|| make_input_err!("'cas_store': '{}' does not exist", store_name))?;
             stores.insert(instance_name.to_string(), store);
         }
+        let max_bytes_per_stream = if config.max_bytes_per_stream == 0 {
+            DEFAULT_MAX_BYTES_PER_STREAM
+        } else {
+            config.max_bytes_per_stream
+        };
         Ok(ByteStreamServer {
             stores,
-            max_bytes_per_stream: config.max_bytes_per_stream,
+            max_bytes_per_stream,
             active_uploads: Arc::new(Mutex::new(HashMap::new())),
             sleep_fn,
         })