Only write to AOF commands that are not readonly

chowc · Aug 9, 2015 · f9911ab · f9911ab
1 parent 76df31b
commit f9911ab
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 18 deletions.
diff --git a/command/src/lib.rs b/command/src/lib.rs
@@ -2045,7 +2045,7 @@ mod flags {
  * key_step: step to get all the keys from first to last argument. For instance
  *           in MSET the step is two since arguments are key,val,key,val,...
    */
-fn command_properties(command_name: &String) -> (i64, u64, i64, i64, i64) {
+fn command_properties(command_name: &str) -> (i64, u64, i64, i64, i64) {
     use flags::*;
     let wm = WRITE | DENYOOM;
     let wf = WRITE | FAST;
@@ -2054,7 +2054,7 @@ fn command_properties(command_name: &String) -> (i64, u64, i64, i64, i64) {
     let ls = LOADING | STALE;
     let ars = READONLY | ADMIN | NOSCRIPT;
     let sr = READONLY | SORT_FOR_SCRIPT;
-    match &**command_name {
+    match command_name {
         "get" => (2, fr, 1, 1, 1),
         "set" => (3, wm, 1, 1, 1),
         "setnx" => (3, wmf, 1, 1, 1),
@@ -2229,11 +2229,23 @@ fn command_properties(command_name: &String) -> (i64, u64, i64, i64, i64) {
     }
 }
 
+fn command_has_flags(command_name: &str, flags: u64) -> bool {
+    (command_properties(command_name).1 & flags) == flags
+}
+
+#[test]
+fn command_has_flags_test() {
+    assert!(command_has_flags("set", flags::WRITE));
+    assert!(command_has_flags("setnx", flags::WRITE | flags::FAST));
+    assert!(!command_has_flags("append", flags::READONLY));
+}
+
 fn execute_command(
         parser: &mut ParsedCommand,
         db: &mut Database,
         client: &mut Client,
         log: &mut bool,
+        write: &mut bool,
         ) -> Result<Response, ResponseError> {
     if parser.argv.len() == 0 {
         return Err(ResponseError::NoReply);
@@ -2242,6 +2254,9 @@ fn execute_command(
         Some(c) => c,
         None => return Ok(Response::Error("unknown command".to_owned())),
     };
+
+    *write = !command_has_flags(command_name, flags::READONLY);
+
     if db.config.requirepass.is_none() {
         client.auth = true;
     }
@@ -2395,10 +2410,11 @@ pub fn command(
         client: &mut Client,
         ) -> Result<Response, ResponseError> {
     let mut log = true;
-    let r = execute_command(&mut parser, db, client, &mut log);
+    let mut write = false;
+    let r = execute_command(&mut parser, db, client, &mut log, &mut write);
     // TODO: only log if there's anyone listening
     if log {
-        db.log_command(client.dbindex, &parser);
+        db.log_command(client.dbindex, &parser, write);
     }
     r
 }

diff --git a/database/src/lib.rs b/database/src/lib.rs
@@ -2096,24 +2096,26 @@ impl Database {
         self.monitor_senders.push(sender);
     }
 
-    pub fn log_command(&mut self, dbindex: usize, command: &ParsedCommand) {
+    pub fn log_command(&mut self, dbindex: usize, command: &ParsedCommand, write: bool) {
         // FIXME: unnecessary free/alloc?
         let bcommand = format!("{:?}", command);
         let tmp = self.monitor_senders.drain(RangeFull).filter(|s| s.send(bcommand.clone()).is_ok()).collect::<Vec<_>>();
         self.monitor_senders = tmp;
-        let mut err = false;
-        match self.aof_writer {
-            Some(ref mut w) => match w.write(dbindex, command) {
-                Ok(_) => (),
-                Err(e) => {
-                    log!(self.config.logger, Warning, "Error writing aof {:?}; stopped writing", e);
-                    err = true;
+        if write {
+            let mut err = false;
+            match self.aof_writer {
+                Some(ref mut w) => match w.write(dbindex, command) {
+                    Ok(_) => (),
+                    Err(e) => {
+                        log!(self.config.logger, Warning, "Error writing aof {:?}; stopped writing", e);
+                        err = true;
+                    },
                 },
-            },
-            None => (),
-        }
-        if err {
-            self.aof_writer = None;
+                None => (),
+            }
+            if err {
+                self.aof_writer = None;
+            }
         }
     }
 }
@@ -3522,7 +3524,7 @@ mod test_command {
         let (tx, rx) = channel();
         database.monitor_add(tx.clone());
         database.monitor_add(tx.clone());
-        database.log_command(0, &ParsedCommand::new(b"1", vec![Argument {pos: 0, len: 1}]));
+        database.log_command(0, &ParsedCommand::new(b"1", vec![Argument {pos: 0, len: 1}]), true);
         assert_eq!(rx.try_recv().unwrap(), "\"1\" ".to_owned());
         assert_eq!(rx.try_recv().unwrap(), "\"1\" ".to_owned());
         assert!(rx.try_recv().is_err())