PragmaTwice commented on code in PR #3378:
URL: https://github.com/apache/kvrocks/pull/3378#discussion_r2881735198
##########
src/server/server.cc:
##########
@@ -994,10 +995,87 @@ void Server::cron() {
}
CleanupExitedSlaves();
+
+ // CLIENT PAUSE timeout check
+ if (conn_pause_end_time_ != 0 && util::GetTimeStampMS() >=
conn_pause_end_time_) {
+ UnpauseConns();
+ }
+
recordInstantaneousMetrics();
}
}
+void Server::PauseConns(uint64_t end_time_ms, PauseMode mode) {
+ std::lock_guard<std::mutex> lock(conn_pause_mu_);
+ conn_pause_end_time_ = end_time_ms;
+ conn_pause_mode_ = mode;
+}
+
+void Server::UnpauseConns() {
+ std::vector<PausedConnEntry> paused_conns;
+ {
+ std::lock_guard<std::mutex> lock(conn_pause_mu_);
+ conn_pause_end_time_ = 0;
+ conn_pause_mode_ = PauseMode::kOff;
+ paused_conns.swap(paused_conns_);
+ }
+ // Resume via the worker so fd+id validation under conns_mu_ serializes with
FreeConnection.
+ for (auto &entry : paused_conns) {
+ entry.worker->UnpauseConnection(entry.fd, entry.id);
+ }
+}
+
+bool Server::PauseConnIfNeeded(redis::Connection *conn, const std::string
&cmd_name, uint64_t cmd_flags) {
+ if (conn_pause_end_time_.load() == 0) {
+ return false;
+ }
+ if (conn->GetClientType() & kTypeSlave) {
+ return false;
+ }
+ // CLIENT subcommands (PAUSE/UNPAUSE) are exempt to avoid deadlock.
+ if (util::EqualICase(cmd_name, "client")) {
+ return false;
+ }
+
+ // Re-read mode under the lock to avoid a TOCTOU race with PauseConns.
+ std::lock_guard<std::mutex> lock(conn_pause_mu_);
+ if (conn_pause_end_time_.load() == 0) {
+ return false;
+ }
+
+ auto mode = conn_pause_mode_.load();
+ bool should_pause = false;
+ if (mode == PauseMode::kAll) {
+ should_pause = true;
+ } else if (mode == PauseMode::kWrite) {
+ if (cmd_flags & redis::kCmdWrite) {
+ should_pause = true;
+ } else {
+ static const std::unordered_set<std::string> kWriteModeSpecialCmds = {
+ "eval", "evalsha", "publish", "pfcount", "wait",
+ };
Review Comment:
Could we move it to a global variable instead of local static variable?
Maybe just upon the `bool Server::PauseConnIfNeeded` definition.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
