Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(server): support config set for some flags #1624

Merged
merged 11 commits into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions src/facade/dragonfly_listener.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#ifdef DFLY_USE_SSL
#include <openssl/ssl.h>
#endif

#include "base/flags.h"
#include "base/logging.h"
#include "facade/dragonfly_connection.h"
Expand All @@ -27,6 +26,7 @@ ABSL_FLAG(string, tls_cert_file, "", "cert file for tls connections");
ABSL_FLAG(string, tls_key_file, "", "key file for tls connections");
ABSL_FLAG(string, tls_ca_cert_file, "", "ca signed certificate to validate tls connections");
ABSL_FLAG(string, tls_ca_cert_dir, "", "ca signed certificates directory");
ABSL_FLAG(uint32_t, tcp_keepalive, 300, "the period in seconds used to send ACKs to client");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the description of this flag is not accurate.

  1. We start sending acks after the idle time that is bound to the value of this flag.,
  2. We give up on the connection if we have not got any ack response during the additional period that equals to the same value.

So it's worth to mention that it's a period of inactivity after which keep-alives are triggerred and the total time until the dead connection is closed is twice the specified value.

image


#if 0
enum TlsClientAuth {
Expand Down Expand Up @@ -99,21 +99,19 @@ SSL_CTX* CreateSslServerCntx() {
}
#endif

bool ConfigureKeepAlive(int fd, unsigned interval_sec) {
DCHECK_GT(interval_sec, 3u);

bool ConfigureKeepAlive(int fd) {
int val = 1;
if (setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val)) < 0)
return false;

val = interval_sec;
val = absl::GetFlag(FLAGS_tcp_keepalive);
if (setsockopt(fd, IPPROTO_TCP, TCP_KEEPIDLE, &val, sizeof(val)) < 0)
return false;

/* Send next probes after the specified interval. Note that we set the
* delay as interval / 3, as we send three probes before detecting
* an error (see the next setsockopt call). */
val = interval_sec / 3;
val = std::max(val / 3, 1);
if (setsockopt(fd, IPPROTO_TCP, TCP_KEEPINTVL, &val, sizeof(val)) < 0)
return false;

Expand Down Expand Up @@ -153,12 +151,11 @@ util::Connection* Listener::NewConnection(ProactorBase* proactor) {

error_code Listener::ConfigureServerSocket(int fd) {
int val = 1;
constexpr int kInterval = 300; // 300 seconds is ok to start checking for liveness.

if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val)) < 0) {
LOG(WARNING) << "Could not set reuse addr on socket " << SafeErrorMessage(errno);
}
bool success = ConfigureKeepAlive(fd, kInterval);
bool success = ConfigureKeepAlive(fd);

if (!success) {
int myerr = errno;
Expand Down
4 changes: 4 additions & 0 deletions src/server/config_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ ConfigRegistry& ConfigRegistry::Register(std::string_view name, WriteCb cb) {
return *this;
}

ConfigRegistry& ConfigRegistry::Register(std::string_view name) {
return Register(name, [](const absl::CommandLineFlag& flag) { return true; });
}

// Returns true if the value was updated.
bool ConfigRegistry::Set(std::string_view config_name, std::string_view value) {
unique_lock lk(mu_);
Expand Down
1 change: 1 addition & 0 deletions src/server/config_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class ConfigRegistry {
using WriteCb = std::function<bool(const absl::CommandLineFlag&)>;

ConfigRegistry& Register(std::string_view name, WriteCb cb);
ConfigRegistry& Register(std::string_view name);

// Returns true if the value was updated.
bool Set(std::string_view config_name, std::string_view value);
Expand Down
5 changes: 5 additions & 0 deletions src/server/main_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,11 @@ void Service::Init(util::AcceptServer* acceptor, std::vector<facade::Listener*>
return true;
});

config_registry.Register("dir");
config_registry.Register("requirepass");
config_registry.Register("masterauth");
config_registry.Register("tcp_keepalive");

pp_.Await([](uint32_t index, ProactorBase* pb) { ServerState::Init(index); });

uint32_t shard_num = GetFlag(FLAGS_num_shards);
Expand Down
2 changes: 1 addition & 1 deletion src/server/server_family.cc
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ GenericError ValidateFilename(const fs::path& filename, bool new_version) {

if (!filename.parent_path().empty() && !is_cloud_path) {
return {absl::StrCat("filename may not contain directory separators (Got \"", filename.c_str(),
"\")")};
"\"). dbfilename should specify the filename without the directory")};
}

if (!filename.has_extension()) {
Expand Down
Loading