Monitor command (#427)

feat(server): support monitor command - allowing user to debug commands
from all connections by using a connection as monitors for the this
(#344)

Signed-off-by: Boaz Sade <[email protected]>

docs/api_status.md

@@ -104,7 +104,7 @@ with respect to Memcached and Redis APIs.
   - [X] ZSCORE
 - [ ] Other
   - [ ] BGREWRITEAOF
-  - [ ] MONITOR
+  - [x] MONITOR
   - [ ] RANDOMKEY
 
 ### API 2

src/facade/conn_context.h

@@ -19,7 +19,8 @@ class ConnectionContext {
 
   // We won't have any virtual methods, probably. However, since we allocate a derived class,
   // we need to declare a virtual d-tor, so we could properly delete it from Connection code.
-  virtual ~ConnectionContext() {}
+  virtual ~ConnectionContext() {
+  }
 
   Connection* owner() {
     return owner_;
@@ -44,12 +45,12 @@ class ConnectionContext {
   }
 
   // connection state / properties.
-  bool async_dispatch: 1;  // whether this connection is currently handled by dispatch fiber.
-  bool conn_closing: 1;
-  bool req_auth: 1;
-  bool replica_conn: 1;
-  bool authenticated: 1;
-  bool force_dispatch: 1;   // whether we should route all requests to the dispatch fiber.
+  bool async_dispatch : 1;  // whether this connection is currently handled by dispatch fiber.
+  bool conn_closing : 1;
+  bool req_auth : 1;
+  bool replica_conn : 1;
+  bool authenticated : 1;
+  bool force_dispatch : 1;  // whether we should route all requests to the dispatch fiber.
 
  private:
   Connection* owner_;

src/facade/dragonfly_connection.cc

@@ -109,6 +109,8 @@ struct Connection::RequestDeleter {
 // Please note: The call to the Dtor is mandatory for this!!
 // This class contain types that don't have trivial destructed objects
 struct Connection::Request {
+  using MonitorMessage = std::string;
+
   struct PipelineMsg {
     absl::FixedArray<MutableSlice, 6> args;
 
@@ -122,26 +124,38 @@ struct Connection::Request {
   };
 
  private:
-  using MessagePayload = std::variant<PipelineMsg, PubMsgRecord>;
+  using MessagePayload = std::variant<PipelineMsg, PubMsgRecord, MonitorMessage>;
 
   Request(size_t nargs, size_t capacity) : payload(PipelineMsg{nargs, capacity}) {
   }
 
   Request(PubMsgRecord msg) : payload(std::move(msg)) {
   }
 
+  Request(MonitorMessage msg) : payload(std::move(msg)) {
+  }
+
   Request(const Request&) = delete;
 
  public:
   // Overload to create the a new pipeline message
   static RequestPtr New(mi_heap_t* heap, RespVec args, size_t capacity);
 
-  // overload to create a new pubsub message
+  // Overload to create a new pubsub message
   static RequestPtr New(const PubMessage& pub_msg, fibers_ext::BlockingCounter bc);
 
+  // Overload to create a new the monitor message
+  static RequestPtr New(MonitorMessage msg);
+
   MessagePayload payload;
 };
 
+Connection::RequestPtr Connection::Request::New(std::string msg) {
+  void* ptr = mi_malloc(sizeof(Request));
+  Request* req = new (ptr) Request(std::move(msg));
+  return Connection::RequestPtr{req, Connection::RequestDeleter{}};
+}
+
 Connection::RequestPtr Connection::Request::New(mi_heap_t* heap, RespVec args, size_t capacity) {
   constexpr auto kReqSz = sizeof(Request);
   void* ptr = mi_heap_malloc_small(heap, kReqSz);
@@ -159,6 +173,7 @@ Connection::RequestPtr Connection::Request::New(mi_heap_t* heap, RespVec args, s
     pipeline_msg.args[i] = MutableSlice(next, s);
     next += s;
   }
+
   return Connection::RequestPtr{req, Connection::RequestDeleter{}};
 }
 
@@ -491,8 +506,6 @@ auto Connection::ParseRedis() -> ParserStatus {
         service_->DispatchCommand(cmd_list, cc_.get());
         last_interaction_ = time(nullptr);
       } else {
-        VLOG(2) << "Dispatch async";
-
         // Dispatch via queue to speedup input reading.
         RequestPtr req = FromArgs(std::move(parse_args_), tlh);
 
@@ -655,19 +668,23 @@ auto Connection::IoLoop(util::FiberSocketBase* peer) -> variant<error_code, Pars
 
 struct Connection::DispatchOperations {
   DispatchOperations(SinkReplyBuilder* b, Connection* me)
-      : stats{me->service_->GetThreadLocalConnectionStats()}, builder{b},
-        empty{me->dispatch_q_.empty()}, self(me) {
+      : stats{me->service_->GetThreadLocalConnectionStats()}, builder{b}, self(me) {
   }
 
   void operator()(PubMsgRecord& msg);
   void operator()(Request::PipelineMsg& msg);
+  void operator()(const Request::MonitorMessage& msg);
 
   ConnectionStats* stats = nullptr;
   SinkReplyBuilder* builder = nullptr;
-  bool empty = false;
   Connection* self = nullptr;
 };
 
+void Connection::DispatchOperations::operator()(const Request::MonitorMessage& msg) {
+  RedisReplyBuilder* rbuilder = (RedisReplyBuilder*)builder;
+  rbuilder->SendSimpleString(msg);
+}
+
 void Connection::DispatchOperations::operator()(PubMsgRecord& msg) {
   RedisReplyBuilder* rbuilder = (RedisReplyBuilder*)builder;
   ++stats->async_writes_cnt;
@@ -690,7 +707,7 @@ void Connection::DispatchOperations::operator()(PubMsgRecord& msg) {
 
 void Connection::DispatchOperations::operator()(Request::PipelineMsg& msg) {
   ++stats->pipelined_cmd_cnt;
-
+  bool empty = self->dispatch_q_.empty();
   builder->SetBatchMode(!empty);
   self->cc_->async_dispatch = true;
   self->service_->DispatchCommand(CmdArgList{msg.args.data(), msg.args.size()}, self->cc_.get());
@@ -703,6 +720,9 @@ struct Connection::DispatchCleanup {
     msg.bc.Dec();
   }
 
+  void operator()(const Connection::Request::MonitorMessage&) const {
+  }
+
   void operator()(const Connection::Request::PipelineMsg&) const {
   }
 };
@@ -769,4 +789,26 @@ void RespToArgList(const RespVec& src, CmdArgVec* dest) {
   }
 }
 
+void Connection::SendMonitorMsg(std::string monitor_msg) {
+  DCHECK(cc_);
+
+  if (!cc_->conn_closing) {
+    RequestPtr req = Request::New(std::move(monitor_msg));
+    dispatch_q_.push_back(std::move(req));
+    if (dispatch_q_.size() == 1) {
+      evc_.notify();
+    }
+  }
+}
+
+std::string Connection::RemoteEndpointStr() const {
+  LinuxSocketBase* lsb = static_cast<LinuxSocketBase*>(socket_.get());
+  bool unix_socket = lsb->IsUDS();
+  std::string connection_str = unix_socket ? "unix:" : std::string{};
+
+  auto re = lsb->RemoteEndpoint();
+  absl::StrAppend(&connection_str, re.address().to_string(), ":", re.port());
+  return connection_str;
+}
+
 }  // namespace facade

src/facade/dragonfly_connection.h

@@ -59,8 +59,15 @@ class Connection : public util::Connection {
     std::string_view message;
   };
 
+  // this function is overriden at test_utils TestConnection
   virtual void SendMsgVecAsync(const PubMessage& pub_msg, util::fibers_ext::BlockingCounter bc);
 
+  // Please note, this accept the message by value, since we really want to
+  // create a new copy here, so that we would not need to "worry" about memory
+  // management, we are assuming that we would not have many copy for this, and that
+  // we would not need in this way to sync on the lifetime of the message
+  void SendMonitorMsg(std::string monitor_msg);
+
   void SetName(std::string_view name) {
     CopyCharBuf(name, sizeof(name_), name_);
   }
@@ -70,6 +77,7 @@ class Connection : public util::Connection {
   }
 
   std::string GetClientInfo() const;
+  std::string RemoteEndpointStr() const;
   uint32 GetClientId() const;
 
   void ShutdownSelf();

src/facade/facade.cc

@@ -63,7 +63,6 @@ string UnknownSubCmd(string_view subcmd, string_view cmd) {
                       cmd, " HELP.");
 }
 
-
 const char kSyntaxErr[] = "syntax error";
 const char kWrongTypeErr[] = "-WRONGTYPE Operation against a key holding the wrong kind of value";
 const char kKeyNotFoundErr[] = "no such key";

src/server/CMakeLists.txt

@@ -13,7 +13,7 @@ add_library(dfly_transaction db_slice.cc engine_shard_set.cc blocking_controller
 cxx_link(dfly_transaction uring_fiber_lib dfly_core strings_lib)
 
 add_library(dragonfly_lib  channel_slice.cc command_registry.cc
-            config_flags.cc conn_context.cc debugcmd.cc dflycmd.cc
+            config_flags.cc conn_context.cc debugcmd.cc server_state.cc dflycmd.cc
             generic_family.cc hset_family.cc json_family.cc
             list_family.cc main_service.cc memory_cmd.cc rdb_load.cc rdb_save.cc replica.cc
             snapshot.cc script_mgr.cc server_family.cc malloc_stats.cc

src/server/conn_context.cc

@@ -6,12 +6,40 @@
 
 #include "base/logging.h"
 #include "server/engine_shard_set.h"
+#include "server/server_family.h"
+#include "server/server_state.h"
+#include "src/facade/dragonfly_connection.h"
 #include "util/proactor_base.h"
 
 namespace dfly {
 
 using namespace std;
 
+void ConnectionContext::SendMonitorMsg(std::string msg) {
+  CHECK(owner());
+
+  owner()->SendMonitorMsg(std::move(msg));
+}
+
+void ConnectionContext::ChangeMonitor(bool start) {
+  // This will either remove or register a new connection
+  // at the "top level" thread --> ServerState context
+  // note that we are registering/removing this connection to the thread at which at run
+  // then notify all other threads that there is a change in the number of monitors
+  auto& my_monitors = ServerState::tlocal()->Monitors();
+  if (start) {
+    my_monitors.Add(this);
+  } else {
+    VLOG(1) << "connection " << owner()->GetClientInfo()
+            << " no longer needs to be monitored - removing 0x" << std::hex << (const void*)this;
+    my_monitors.Remove(this);
+  }
+  // Tell other threads that about the change in the number of connection that we monitor
+  shard_set->pool()->Await(
+      [start](auto*) { ServerState::tlocal()->Monitors().NotifyChangeCount(start); });
+  EnableMonitoring(start);
+}
+
 void ConnectionContext::ChangeSubscription(bool to_add, bool to_reply, CmdArgList args) {
   vector<unsigned> result(to_reply ? args.size() : 0, 0);
 

src/server/conn_context.h

@@ -32,7 +32,9 @@ struct ConnectionState {
     ExecInfo(ExecInfo&&) = delete;
 
     // Return true if ExecInfo is active (after MULTI)
-    bool IsActive() { return state != EXEC_INACTIVE; }
+    bool IsActive() {
+      return state != EXEC_INACTIVE;
+    }
 
     // Resets to blank state after EXEC or DISCARD
     void Clear();
@@ -117,17 +119,29 @@ class ConnectionContext : public facade::ConnectionContext {
     return conn_state.db_index;
   }
 
+  // Note that this is accepted by value for lifetime reasons
+  // we want to have our own copy since we are assuming that
+  // 1. there will be not to many connections that we in monitor state
+  // 2. we need to have for each of them each own copy for thread safe reasons
+  void SendMonitorMsg(std::string msg);
+
   void ChangeSubscription(bool to_add, bool to_reply, CmdArgList args);
   void ChangePSub(bool to_add, bool to_reply, CmdArgList args);
   void UnsubscribeAll(bool to_reply);
   void PUnsubscribeAll(bool to_reply);
+  void ChangeMonitor(bool start);  // either start or stop monitor on a given connection
 
   bool is_replicating = false;
+  bool monitor = false;  // when a monitor command is sent over a given connection, we need to aware
+                         // of it as a state for the connection
 
  private:
+  void EnableMonitoring(bool enable) {
+    force_dispatch = enable;  // required to support the monitoring
+    monitor = enable;
+  }
   void SendSubscriptionChangedResponse(std::string_view action,
-                                       std::optional<std::string_view> topic,
-                                       unsigned count);
+                                       std::optional<std::string_view> topic, unsigned count);
 };
 
 }  // namespace dfly