From 0ddb88db451dad4a79a099a46184a297c19b41e5 Mon Sep 17 00:00:00 2001
From: Vartan Babayan <v.babayan@picodata.io>
Date: Fri, 27 Sep 2024 22:52:34 +0300
Subject: [PATCH] feat: support human numbers for vinyl cache and memory

---
 CHANGELOG.md                 |  2 +-
 src/config.rs                | 48 +++++++++++++++++++++++++-----------
 src/tarantool.rs             | 10 +++++---
 test/int/test_config_file.py | 16 ++++++------
 4 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a60795b6d7..8820be30dc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -24,7 +24,7 @@ Connection via `Pgproto` no longer requires additional manual step to change the
 - New `picodata connect` and `picodata expel` argument `--timeout` for specifying
 the timeout for address resolving operation.
 
-- Support human numbers to configure memtx.memory.
+- Support human numbers to configure memtx.memory, vinyl.memory and vinyl.cache parameters.
   Supported suffixes: K, M, G, T, 1K = 1024
   (e.g picodata run --memtx-memory 10G)
 
diff --git a/src/config.rs b/src/config.rs
index 815a6b4678..f782a14306 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -1119,7 +1119,25 @@ impl InstanceConfig {
     pub fn memtx_memory(&self) -> u64 {
         self.memtx
             .memory
-            .clone()
+            .as_ref()
+            .expect("is set in PicodataConfig::set_defaults_explicitly")
+            .into()
+    }
+
+    #[inline]
+    pub fn vinyl_memory(&self) -> u64 {
+        self.vinyl
+            .memory
+            .as_ref()
+            .expect("is set in PicodataConfig::set_defaults_explicitly")
+            .into()
+    }
+
+    #[inline]
+    pub fn vinyl_cache(&self) -> u64 {
+        self.vinyl
+            .cache
+            .as_ref()
             .expect("is set in PicodataConfig::set_defaults_explicitly")
             .into()
     }
@@ -1202,8 +1220,8 @@ impl FromStr for ByteSize {
     }
 }
 
-impl From<ByteSize> for u64 {
-    fn from(memory: ByteSize) -> Self {
+impl From<&ByteSize> for u64 {
+    fn from(memory: &ByteSize) -> Self {
         memory.as_u64().expect("should be valid ByteSize")
     }
 }
@@ -1280,14 +1298,14 @@ pub struct VinylSection {
     /// The maximum number of in-memory bytes that vinyl uses.
     ///
     /// Corresponds to `box.cfg.vinyl_memory`
-    #[introspection(config_default = 128 * 1024 * 1024)]
-    pub memory: Option<u64>,
+    #[introspection(config_default = "128M")]
+    pub memory: Option<ByteSize>,
 
     /// The cache size for the vinyl storage engine.
     ///
     /// Corresponds to `box.cfg.vinyl_cache`
-    #[introspection(config_default = 128 * 1024 * 1024)]
-    pub cache: Option<u64>,
+    #[introspection(config_default = "128M")]
+    pub cache: Option<ByteSize>,
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -2087,28 +2105,28 @@ instance:
     fn test_bytesize_from_str() {
         let res = |s| ByteSize::from_str(s).unwrap();
         assert_eq!(res("1024").to_string(), "1024B");
-        assert_eq!(u64::from(res("1024")), 1024);
+        assert_eq!(res("1024").as_u64().unwrap(), 1024);
 
         assert_eq!(res("256B").to_string(), "256B");
-        assert_eq!(u64::from(res("256B")), 256);
+        assert_eq!(res("256B").as_u64().unwrap(), 256);
 
         assert_eq!(res("64K").to_string(), "64K");
-        assert_eq!(u64::from(res("64K")), 65_536);
+        assert_eq!(res("64K").as_u64().unwrap(), 65_536);
 
         assert_eq!(res("949M").to_string(), "949M");
-        assert_eq!(u64::from(res("949M")), 995_098_624);
+        assert_eq!(res("949M").as_u64().unwrap(), 995_098_624);
 
         assert_eq!(res("1G").to_string(), "1G");
-        assert_eq!(u64::from(res("1G")), 1_073_741_824);
+        assert_eq!(res("1G").as_u64().unwrap(), 1_073_741_824);
 
         assert_eq!(res("10T").to_string(), "10T");
-        assert_eq!(u64::from(res("10T")), 10_995_116_277_760);
+        assert_eq!(res("10T").as_u64().unwrap(), 10_995_116_277_760);
 
         assert_eq!(res("0M").to_string(), "0M");
-        assert_eq!(u64::from(res("0M")), 0);
+        assert_eq!(res("0M").as_u64().unwrap(), 0);
 
         assert_eq!(res("185T").to_string(), "185T");
-        assert_eq!(u64::from(res("185T")), 203_409_651_138_560);
+        assert_eq!(res("185T").as_u64().unwrap(), 203_409_651_138_560);
 
         let e = |s| ByteSize::from_str(s).unwrap_err();
         assert_eq!(e(""), "cannot parse integer from empty string");
diff --git a/src/tarantool.rs b/src/tarantool.rs
index f7a80ee21f..a9765f63ba 100644
--- a/src/tarantool.rs
+++ b/src/tarantool.rs
@@ -234,8 +234,11 @@ impl Cfg {
         self.vinyl_dir = config.instance.data_dir();
 
         // FIXME: make the loop below work with default values
-        self.other_fields
-            .insert("memtx_memory".into(), config.instance.memtx_memory().into());
+        self.other_fields.extend([
+            ("memtx_memory".into(), config.instance.memtx_memory().into()),
+            ("vinyl_memory".into(), config.instance.vinyl_memory().into()),
+            ("vinyl_cache".into(), config.instance.vinyl_cache().into()),
+        ]);
 
         #[rustfmt::skip]
         const MAPPING: &[(&str, &str)] = &[
@@ -243,10 +246,9 @@ impl Cfg {
             ("checkpoint_count",            config_parameter_path!(instance.memtx.checkpoint_count)),
             ("checkpoint_interval",         config_parameter_path!(instance.memtx.checkpoint_interval)),
             ("log_format",                  config_parameter_path!(instance.log.format)),
-            ("vinyl_memory",                config_parameter_path!(instance.vinyl.memory)),
-            ("vinyl_cache",                 config_parameter_path!(instance.vinyl.cache)),
             ("net_msg_max",                 config_parameter_path!(instance.iproto.max_concurrent_messages)),
         ];
+
         for (box_field, picodata_field) in MAPPING {
             let value = config
                 .get_field_as_rmpv(picodata_field)
diff --git a/test/int/test_config_file.py b/test/int/test_config_file.py
index 530ab8975d..ed55ced536 100644
--- a/test/int/test_config_file.py
+++ b/test/int/test_config_file.py
@@ -109,8 +109,8 @@ instance:
                 checkpoint_interval=dict(value=3600, source="default"),
             ),
             vinyl=dict(
-                memory=dict(value=128 * 1024 * 1024, source="default"),
-                cache=dict(value=128 * 1024 * 1024, source="default"),
+                memory=dict(value="128M", source="default"),
+                cache=dict(value="128M", source="default"),
             ),
             iproto=dict(
                 max_concurrent_messages=dict(value=768, source="default"),
@@ -303,8 +303,8 @@ cluster:
     assert box_cfg["checkpoint_count"] == 2
     assert box_cfg["checkpoint_interval"] == 3600
 
-    assert box_cfg["vinyl_memory"] == 128 * 1024 * 1024
-    assert box_cfg["vinyl_cache"] == 128 * 1024 * 1024
+    assert box_cfg["vinyl_memory"] == 134217728
+    assert box_cfg["vinyl_cache"] == 134217728
     assert box_cfg["vinyl_read_threads"] == 1
     assert box_cfg["vinyl_write_threads"] == 4
     assert box_cfg["vinyl_defer_deletes"] == False  # noqa: E712
@@ -341,8 +341,8 @@ instance:
         checkpoint_interval: 1800
 
     vinyl:
-        memory: 0x8888888
-        cache: 0x4444444
+        memory: 600M
+        cache: 300M
 
     iproto:
         max_concurrent_messages: 0x600
@@ -384,8 +384,8 @@ instance:
     assert box_cfg["checkpoint_count"] == 8
     assert box_cfg["checkpoint_interval"] == 1800
 
-    assert box_cfg["vinyl_memory"] == 0x8888888
-    assert box_cfg["vinyl_cache"] == 0x4444444
+    assert box_cfg["vinyl_memory"] == 629145600
+    assert box_cfg["vinyl_cache"] == 314572800
 
     assert box_cfg["net_msg_max"] == 0x600
 
-- 
GitLab