Code review responses for non-integer resources requests

It4innovations · Oct 17, 2023 · e7aa4fb · e7aa4fb
1 parent 9d35af9
commit e7aa4fb
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 11 deletions.
diff --git a/crates/hyperqueue/src/common/parser2.rs b/crates/hyperqueue/src/common/parser2.rs
@@ -231,6 +231,7 @@ pub fn parse_u32() -> impl CharParser<u32> {
     })
 }
 
+/// Parse int32 or float with FRACTIONS_MAX_DIGITS precision (e.g. "2", "3.3", "0.0001")
 pub fn parse_resource_amount() -> impl CharParser<ResourceAmount> {
     parse_u32()
         .then(just('.').ignore_then(parse_integer_string()).or_not())
@@ -326,7 +327,13 @@ mod tests {
             parse_resource_amount().parse_text("0.346").unwrap(),
             ResourceAmount::new(0, 3460)
         );
-        assert!(parse_resource_amount().parse_text("0.12345").is_err());
+        let r = parse_resource_amount().parse_text("0.12345");
+        insta::assert_snapshot!(r.unwrap_err().to_string().as_str(), @r###"
+        Unexpected end of input found, expected something else:
+          0.12345
+          |
+          --- Resource precision exceeded
+        "###);
     }
 
     #[test]

diff --git a/crates/tako/src/internal/common/resources/allocation.rs b/crates/tako/src/internal/common/resources/allocation.rs
@@ -31,6 +31,12 @@ pub struct Allocation {
     pub resources: ResourceAllocations,
 }
 
+impl Default for Allocation {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl Allocation {
     pub fn new() -> Self {
         Allocation {

diff --git a/crates/tako/src/internal/common/resources/amount.rs b/crates/tako/src/internal/common/resources/amount.rs
@@ -5,7 +5,7 @@ pub type ResourceUnits = u32;
 pub type ResourceFractions = u32;
 
 pub const FRACTIONS_PER_UNIT: ResourceFractions = 10_000;
-pub const FRACTIONS_MAX_DIGITS: usize = 4; // = log10(FRACTIONS_PER_UNIT)
+pub const FRACTIONS_MAX_DIGITS: usize = FRACTIONS_PER_UNIT.ilog10() as usize;
 
 #[derive(
     Debug,

diff --git a/crates/tako/src/internal/tests/integration/test_resources.rs b/crates/tako/src/internal/tests/integration/test_resources.rs
@@ -90,10 +90,7 @@ async fn test_submit_2_sleeps_on_separated_2() {
             )
             .await;
 
-        let workers = handler
-            .start_workers(Default::default, 3)
-            .await
-            .unwrap();
+        let workers = handler.start_workers(Default::default, 3).await.unwrap();
         let worker_ids: Vec<WorkerId> = workers.iter().map(|x| x.id).collect();
 
         wait_for_task_start(&mut handler, 1).await;

diff --git a/crates/tako/src/internal/tests/integration/test_worker.rs b/crates/tako/src/internal/tests/integration/test_worker.rs
@@ -144,10 +144,7 @@ async fn test_panic_on_worker_lost() {
 #[tokio::test]
 async fn test_lost_worker_with_tasks_continue() {
     run_test(Default::default(), |mut handler| async move {
-        let _workers = handler
-            .start_workers(Default::default, 2)
-            .await
-            .unwrap();
+        let _workers = handler.start_workers(Default::default, 2).await.unwrap();
 
         let task_ids = handler
             .submit(GraphBuilder::singleton(simple_task(&["sleep", "1"], 1)))

diff --git a/crates/tako/src/internal/worker/resources/allocator.rs b/crates/tako/src/internal/worker/resources/allocator.rs
@@ -130,7 +130,9 @@ impl ResourceAllocator {
                 }
                 let (units, fractions) = amount.split();
                 if fractions > 0 {
-                    todo!() // Fractions and ForceCompact is not supported in the current version
+                    // Fractions and ForceCompact is not supported in the current version
+                    // It is checked in resource request validation, this code should be unreachable
+                    unreachable!()
                 }
                 let socket_count = (((units - 1) / pool.min_group_size()) as usize) + 1;
                 if free.n_groups() < socket_count {