RFC: mononoke: add support for string tunables

Summary:
Just as for ints and bools, let's add support for strings.
A few notes:
1) we don't have AtomicString, so I decided to use ArcSwap<String>.
However to make code generation simpler (see tunables-derive) I've added a type
alias for that
2) Return type is Arc<String> to avoid unnecessary copies.Another option that we might have here is to put the whole Tunables structure inside ArcSwap, and changing `tunables()` to return Arc<MononookeTunables>
instead of &'static MononokeTunables. However that's a bigger change to make.

Reviewed By: markbt

Differential Revision: D21592390

fbshipit-source-id: 6d3cf340b13f7aef9adb2b1b99ed2bf260033285

eden/mononoke/tunables/Cargo.toml

@@ -13,6 +13,7 @@ cached_config = { git = "https://github.com/facebookexperimental/rust-shed.git",
 fbinit = { git = "https://github.com/facebookexperimental/rust-shed.git", branch = "master" }
 fbthrift = { git = "https://github.com/facebook/fbthrift.git", branch = "master" }
 anyhow = "1.0"
+arc-swap = "0.4"
 futures = { version = "0.3", features = ["async-await", "compat"] }
 once_cell = "1.2"
 serde_json = "1.0"

eden/mononoke/tunables/src/lib.rs

@@ -10,6 +10,7 @@ use std::thread;
 use std::time::Duration;
 
 use anyhow::Result;
+use arc_swap::ArcSwap;
 use cached_config::ConfigHandle;
 use once_cell::sync::OnceCell;
 use slog::{debug, warn, Logger};
@@ -25,6 +26,9 @@ pub fn tunables() -> &'static MononokeTunables {
     TUNABLES.get_or_init(MononokeTunables::default)
 }
 
+// This type exists to simplify code generation in tunables-derive
+pub type TunableString = ArcSwap<String>;
+
 #[derive(Tunables, Default, Debug)]
 pub struct MononokeTunables {
     mutation_advertise_for_infinitepush: AtomicBool,
@@ -69,6 +73,7 @@ fn update_tunables(new_tunables: Arc<TunablesStruct>) -> Result<()> {
     let tunables = tunables();
     tunables.update_bools(&new_tunables.killswitches);
     tunables.update_ints(&new_tunables.ints);
+    tunables.update_strings(&new_tunables.strings);
 
     Ok(())
 }
@@ -83,6 +88,7 @@ mod test {
     struct TestTunables {
         boolean: AtomicBool,
         num: AtomicI64,
+        string: TunableString,
     }
 
     #[derive(Tunables, Default)]
@@ -96,6 +102,7 @@ mod test {
 
         empty.update_bools(&bools);
         empty.update_ints(&ints);
+        empty.update_strings(&HashMap::new());
     }
 
     #[test]
@@ -130,4 +137,15 @@ mod test {
         test.update_ints(&d);
         assert_eq!(test.get_num(), 0);
     }
+
+    #[test]
+    fn update_string() {
+        let mut d = HashMap::new();
+        d.insert("string".to_string(), "value".to_string());
+
+        let test = TestTunables::default();
+        assert_eq!(test.get_string().as_str(), "");
+        test.update_strings(&d);
+        assert_eq!(test.get_string().as_str(), "value");
+    }
 }

eden/mononoke/tunables/tunables-derive/lib.rs

@@ -18,6 +18,7 @@ const STRUCT_FIELD_MSG: &str = "Only implemented for named fields of a struct";
 enum TunableType {
     Bool,
     I64,
+    String,
 }
 
 #[proc_macro_derive(Tunables)]
@@ -45,10 +46,19 @@ pub fn derive_tunables(input: proc_macro::TokenStream) -> proc_macro::TokenStrea
 }
 
 impl TunableType {
-    fn external_type(&self) -> Ident {
+    fn external_type(&self) -> TokenStream {
+        match self {
+            Self::Bool => quote! { bool },
+            Self::I64 => quote! { i64 },
+            Self::String => quote! { Arc<String> },
+        }
+    }
+
+    fn input_type(&self) -> Ident {
         match self {
             Self::Bool => quote::format_ident!("{}", "bool"),
             Self::I64 => quote::format_ident!("{}", "i64"),
+            Self::String => quote::format_ident!("{}", "String"),
         }
     }
 
@@ -56,9 +66,20 @@ impl TunableType {
         let method = quote::format_ident!("get_{}", name);
         let external_type = self.external_type();
 
-        quote! {
-            pub fn #method(&self) -> #external_type {
-                return self.#name.load(std::sync::atomic::Ordering::Relaxed)
+        match &self {
+            Self::Bool | Self::I64 => {
+                quote! {
+                    pub fn #method(&self) -> #external_type {
+                        return self.#name.load(std::sync::atomic::Ordering::Relaxed)
+                    }
+                }
+            }
+            Self::String => {
+                quote! {
+                    pub fn #method(&self) -> #external_type {
+                        self.#name.load_full()
+                    }
+                }
             }
         }
     }
@@ -90,11 +111,17 @@ where
     ));
 
     methods.extend(generate_updater_method(
-        names_and_types,
+        names_and_types.clone(),
         TunableType::I64,
         quote::format_ident!("update_ints"),
     ));
 
+    methods.extend(generate_updater_method(
+        names_and_types,
+        TunableType::String,
+        quote::format_ident!("update_strings"),
+    ));
+
     methods
 }
 
@@ -108,21 +135,37 @@ where
 {
     let names = names_and_types.filter(|(_, t)| *t == ty).map(|(n, _)| n);
 
-    let type_ident = ty.external_type();
+    let type_ident = ty.input_type();
     let mut names = names.peekable();
     let mut body = TokenStream::new();
 
     if names.peek().is_some() {
-        body.extend(
-            quote! {
-                for (name, val) in tunables.iter() {
-                    match name.as_ref() {
-                        #(stringify!(#names) => self.#names.store(*val, std::sync::atomic::Ordering::Relaxed), )*
-                        _ => {}
+        match ty {
+            TunableType::I64 | TunableType::Bool => {
+                body.extend(
+                    quote! {
+                        for (name, val) in tunables.iter() {
+                            match name.as_ref() {
+                                #(stringify!(#names) => self.#names.store(*val, std::sync::atomic::Ordering::Relaxed), )*
+                                _ => {}
+                            }
+                        }
                     }
-                }
+                );
             }
-        )
+            TunableType::String => {
+                body.extend(quote! {
+                    for (name, val) in tunables {
+                        match name.as_ref() {
+                            #(stringify!(#names) => {
+                                self.#names.swap(Arc::new(val.clone()));
+                            }, )*
+                            _ => {}
+                        }
+                    }
+                });
+            }
+        }
     }
 
     quote! {
@@ -154,6 +197,11 @@ fn resolve_type(ty: Type) -> TunableType {
             match &ident.to_string()[..] {
                 "AtomicBool" => return TunableType::Bool,
                 "AtomicI64" => return TunableType::I64,
+                // TunableString is a type alias of ArcSwap<String>.
+                // p.path.get_ident() returns None for ArcSwap<String>
+                // and it makes it harder to parse it.
+                // We use TunableString as a workaround
+                "TunableString" => return TunableType::String,
                 _ => unimplemented!("{}", UNIMPLEMENTED_MSG),
             }
         }