This is an automated email from the ASF dual-hosted git repository.
twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/incubator-kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new c09cb8c4 Support CLI options for kvrocks server (#1037)
c09cb8c4 is described below
commit c09cb8c4054efadd3958046ee5f92b88a477eabf
Author: Twice <[email protected]>
AuthorDate: Mon Oct 24 23:29:14 2022 +0800
Support CLI options for kvrocks server (#1037)
* Support CLI options for kvrocks server
* read from stdin
* fix usage
* fix
* remove header
---
src/config/config.cc | 62 +++++++++++--------
src/config/config.h | 11 +++-
src/config/config_type.h | 8 +--
src/main.cc | 71 +++++++++++++---------
tests/cppunit/config_test.cc | 18 +++---
.../integration/cli_options/cli_options_test.go | 39 ++++++++++++
tests/gocase/util/server.go | 5 ++
7 files changed, 147 insertions(+), 67 deletions(-)
diff --git a/src/config/config.cc b/src/config/config.cc
index 4cb82b7c..6dd24856 100644
--- a/src/config/config.cc
+++ b/src/config/config.cc
@@ -601,32 +601,36 @@ void Config::ClearMaster() {
}
}
-Status Config::parseConfigFromString(const std::string &input, int
line_number) {
- auto parsed = ParseConfigLine(input);
- if (!parsed) return parsed.ToStatus();
-
- auto kv = std::move(*parsed);
-
- if (kv.first.empty() || kv.second.empty()) return Status::OK();
-
- std::string field_key = Util::ToLower(kv.first);
+Status Config::parseConfigFromPair(const std::pair<std::string, std::string>
&input, int line_number) {
+ std::string field_key = Util::ToLower(input.first);
const char ns_str[] = "namespace.";
size_t ns_str_size = sizeof(ns_str) - 1;
- if (!strncasecmp(kv.first.data(), ns_str, ns_str_size)) {
+ if (!strncasecmp(input.first.data(), ns_str, ns_str_size)) {
// namespace should keep key case-sensitive
- field_key = kv.first;
- tokens[kv.second] = kv.first.substr(ns_str_size);
+ field_key = input.first;
+ tokens[input.second] = input.first.substr(ns_str_size);
}
auto iter = fields_.find(field_key);
if (iter != fields_.end()) {
auto &field = iter->second;
field->line_number = line_number;
- auto s = field->Set(kv.second);
+ auto s = field->Set(input.second);
if (!s.IsOK()) return s;
}
return Status::OK();
}
+Status Config::parseConfigFromString(const std::string &input, int
line_number) {
+ auto parsed = ParseConfigLine(input);
+ if (!parsed) return parsed.ToStatus();
+
+ auto kv = std::move(*parsed);
+
+ if (kv.first.empty() || kv.second.empty()) return Status::OK();
+
+ return parseConfigFromPair(kv, line_number);
+}
+
Status Config::finish() {
if (requirepass.empty() && !tokens.empty()) {
return Status(Status::NotOK, "requirepass empty wasn't allowed while the
namespace exists");
@@ -657,30 +661,40 @@ Status Config::finish() {
return Status::OK();
}
-Status Config::Load(const std::string &path) {
- if (!path.empty()) {
- path_ = path;
- std::ifstream file(path_);
- if (!file.is_open()) return Status(Status::NotOK, strerror(errno));
+Status Config::Load(const CLIOptions &opts) {
+ if (!opts.conf_file.empty()) {
+ std::ifstream file;
+ std::istream *in = nullptr;
+ if (opts.conf_file == "-") {
+ in = &std::cin;
+ } else {
+ path_ = opts.conf_file;
+ file.open(path_);
+ if (!file.is_open()) return Status(Status::NotOK, strerror(errno));
+ in = &file;
+ }
std::string line;
int line_num = 1;
- while (!file.eof()) {
- std::getline(file, line);
- Status s = parseConfigFromString(line, line_num);
- if (!s.IsOK()) {
- file.close();
+ while (!in->eof()) {
+ std::getline(*in, line);
+ if (auto s = parseConfigFromString(line, line_num); !s) {
return Status(Status::NotOK, "at line: #L" + std::to_string(line_num)
+ ", err: " + s.Msg());
}
line_num++;
}
- file.close();
} else {
std::cout << "Warn: no config file specified, using the default config. "
"In order to specify a config file use kvrocks -c
/path/to/kvrocks.conf"
<< std::endl;
}
+ for (const auto &opt : opts.cli_options) {
+ if (Status s = parseConfigFromPair(opt, -1); !s) {
+ return Status(Status::NotOK, "CLI config option error: " + s.Msg());
+ }
+ }
+
for (const auto &iter : fields_) {
// line_number = 0 means the user didn't specify the field value
// on config file and would use default value, so won't validate here.
diff --git a/src/config/config.h b/src/config/config.h
index 68c028b1..6e3556fd 100644
--- a/src/config/config.h
+++ b/src/config/config.h
@@ -61,6 +61,14 @@ struct CompactionCheckerRange {
bool Enabled() { return Start != -1 || Stop != -1; }
};
+struct CLIOptions {
+ std::string conf_file;
+ std::vector<std::pair<std::string, std::string>> cli_options;
+
+ CLIOptions() = default;
+ explicit CLIOptions(std::string_view file) : conf_file(file) {}
+};
+
struct Config {
public:
Config();
@@ -186,7 +194,7 @@ struct Config {
public:
Status Rewrite();
- Status Load(const std::string &path);
+ Status Load(const CLIOptions &path);
void Get(std::string key, std::vector<std::string> *values);
Status Set(Server *svr, std::string key, const std::string &value);
void SetMaster(const std::string &host, int port);
@@ -209,6 +217,7 @@ struct Config {
void initFieldValidator();
void initFieldCallback();
+ Status parseConfigFromPair(const std::pair<std::string, std::string> &input,
int line_number);
Status parseConfigFromString(const std::string &input, int line_number);
Status finish();
Status isNamespaceLegal(const std::string &ns);
diff --git a/src/config/config_type.h b/src/config/config_type.h
index 2148cc06..998a3bc4 100644
--- a/src/config/config_type.h
+++ b/src/config/config_type.h
@@ -29,13 +29,13 @@
// forward declaration
class Server;
-typedef std::function<Status(const std::string &, const std::string &)>
validate_fn;
-typedef std::function<Status(Server *srv, const std::string &, const
std::string &)> callback_fn;
+using validate_fn = std::function<Status(const std::string &, const
std::string &)>;
+using callback_fn = std::function<Status(Server *, const std::string &, const
std::string &)>;
-typedef struct configEnum {
+struct configEnum {
const char *name;
const int val;
-} configEnum;
+};
int configEnumGetValue(configEnum *ce, const char *name);
const char *configEnumGetName(configEnum *ce, int val);
diff --git a/src/main.cc b/src/main.cc
index 4a28673e..9fa195c8 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -20,10 +20,12 @@
#include <dlfcn.h>
#include <fcntl.h>
-#include <getopt.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
+
+#include <iomanip>
+#include <ostream>
#ifdef __linux__
#define _XOPEN_SOURCE 700
#else
@@ -49,11 +51,6 @@ bool Symbolize(void *pc, char *out, size_t out_size);
std::function<void()> hup_handler;
-struct Options {
- std::string conf_file;
- bool show_usage = false;
-};
-
Server *srv = nullptr;
Server *GetServer() { return srv; }
@@ -112,30 +109,47 @@ void setupSigSegvAction() {
sigaction(SIGINT, &act, nullptr);
}
-static void usage(const char *program) {
- std::cout << program << " implements the Redis protocol based on rocksdb\n"
- << "\t-c config file\n"
- << "\t-h help\n";
- exit(0);
+struct NewOpt {
+ friend auto &operator<<(std::ostream &os, NewOpt) { return os <<
std::string(4, ' ') << std::setw(32); }
+} new_opt;
+
+static void printUsage(const char *program) {
+ std::cout << program << " implements the Redis protocol based on rocksdb" <<
std::endl
+ << "Usage:" << std::endl
+ << std::left << new_opt << "-c, --config <filename>"
+ << "set config file to <filename>, or `-' for stdin" << std::endl
+ << new_opt << "-v, --version"
+ << "print version information" << std::endl
+ << new_opt << "-h, --help"
+ << "print this help message" << std::endl
+ << new_opt << "--<config-key> <config-value>"
+ << "overwrite specific config option <config-key> to
<config-value>" << std::endl;
}
-static Options parseCommandLineOptions(int argc, char **argv) {
- int ch;
- Options opts;
- while ((ch = ::getopt(argc, argv, "c:hv")) != -1) {
- switch (ch) {
- case 'c':
- opts.conf_file = optarg;
- break;
- case 'h':
- opts.show_usage = true;
- break;
- case 'v':
- exit(0);
- default:
- usage(argv[0]);
+static void printVersion(std::ostream &os) { os << "kvrocks version " <<
VERSION << " @" << GIT_COMMIT << std::endl; }
+
+static CLIOptions parseCommandLineOptions(int argc, char **argv) {
+ using namespace std::string_view_literals;
+ CLIOptions opts;
+
+ for (int i = 1; i < argc; ++i) {
+ if ((argv[i] == "-c"sv || argv[i] == "--config"sv) && i + 1 < argc) {
+ opts.conf_file = argv[++i];
+ } else if (argv[i] == "-v"sv || argv[i] == "--version"sv) {
+ printVersion(std::cout);
+ std::exit(0);
+ } else if (argv[i] == "-h"sv || argv[i] == "--help"sv) {
+ printUsage(*argv);
+ std::exit(0);
+ } else if (std::string_view(argv[i], 2) == "--" &&
std::string_view(argv[i]).size() > 2 && i + 1 < argc) {
+ auto key = std::string_view(argv[i] + 2);
+ opts.cli_options.emplace_back(key, argv[++i]);
+ } else {
+ printUsage(*argv);
+ std::exit(1);
}
}
+
return opts;
}
@@ -272,19 +286,18 @@ int main(int argc, char *argv[]) {
setupSigSegvAction();
auto opts = parseCommandLineOptions(argc, argv);
- if (opts.show_usage) usage(argv[0]);
Redis::InitCommandsTable();
Redis::PopulateCommands();
Config config;
- Status s = config.Load(opts.conf_file);
+ Status s = config.Load(opts);
if (!s.IsOK()) {
std::cout << "Failed to load config, err: " << s.Msg() << std::endl;
exit(1);
}
initGoogleLog(&config);
- LOG(INFO) << "Version: " << VERSION << " @" << GIT_COMMIT << std::endl;
+ printVersion(LOG(INFO));
// Tricky: We don't expect that different instances running on the same port,
// but the server use REUSE_PORT to support the multi listeners. So we
connect
// the listen port to check if the port has already listened or not.
diff --git a/tests/cppunit/config_test.cc b/tests/cppunit/config_test.cc
index 4a9564b1..4d3812f9 100644
--- a/tests/cppunit/config_test.cc
+++ b/tests/cppunit/config_test.cc
@@ -34,7 +34,7 @@ TEST(Config, GetAndSet) {
const char *path = "test.conf";
Config config;
- config.Load(path);
+ config.Load(CLIOptions(path));
// Config.Set need accessing commands, so we should init and populate
// the command table here.
Redis::InitCommandsTable();
@@ -94,7 +94,7 @@ TEST(Config, GetAndSet) {
EXPECT_EQ(values[1], iter.second);
}
ASSERT_TRUE(config.Rewrite().IsOK());
- config.Load(path);
+ config.Load(CLIOptions(path));
for (const auto &iter : mutable_cases) {
auto s = config.Set(nullptr, iter.first, iter.second);
ASSERT_TRUE(s.IsOK());
@@ -150,11 +150,11 @@ TEST(Config, Rewrite) {
Config config;
Redis::PopulateCommands();
- ASSERT_TRUE(config.Load(path).IsOK());
+ ASSERT_TRUE(config.Load(CLIOptions(path)).IsOK());
ASSERT_TRUE(config.Rewrite().IsOK());
// Need to re-populate the command table since it has renamed by the previous
Redis::PopulateCommands();
- ASSERT_TRUE(config.Load(path).IsOK());
+ ASSERT_TRUE(config.Load(CLIOptions(path)).IsOK());
unlink(path);
}
@@ -163,7 +163,7 @@ TEST(Namespace, Add) {
unlink(path);
Config config;
- config.Load(path);
+ config.Load(CLIOptions(path));
config.slot_id_encoded = false;
EXPECT_TRUE(!config.AddNamespace("ns", "t0").IsOK());
config.requirepass = "foobared";
@@ -198,7 +198,7 @@ TEST(Namespace, Set) {
unlink(path);
Config config;
- config.Load(path);
+ config.Load(CLIOptions(path));
config.slot_id_encoded = false;
config.requirepass = "foobared";
std::vector<std::string> namespaces = {"n1", "n2", "n3", "n4"};
@@ -233,7 +233,7 @@ TEST(Namespace, Delete) {
unlink(path);
Config config;
- config.Load(path);
+ config.Load(CLIOptions(path));
config.slot_id_encoded = false;
config.requirepass = "foobared";
std::vector<std::string> namespaces = {"n1", "n2", "n3", "n4"};
@@ -259,7 +259,7 @@ TEST(Namespace, RewriteNamespaces) {
const char *path = "test.conf";
unlink(path);
Config config;
- config.Load(path);
+ config.Load(CLIOptions(path));
config.requirepass = "test";
config.backup_dir = "test";
config.slot_id_encoded = false;
@@ -272,7 +272,7 @@ TEST(Namespace, RewriteNamespaces) {
EXPECT_TRUE(config.DelNamespace("to-be-deleted-ns").IsOK());
Config new_config;
- auto s = new_config.Load(path);
+ auto s = new_config.Load(CLIOptions(path));
for (size_t i = 0; i < namespaces.size(); i++) {
std::string token;
new_config.GetNamespace(namespaces[i], &token);
diff --git a/tests/gocase/integration/cli_options/cli_options_test.go
b/tests/gocase/integration/cli_options/cli_options_test.go
new file mode 100644
index 00000000..d60a41de
--- /dev/null
+++ b/tests/gocase/integration/cli_options/cli_options_test.go
@@ -0,0 +1,39 @@
+/*
+* Licensed to the Apache Software Foundation (ASF) under one
+* or more contributor license agreements. See the NOTICE file
+* distributed with this work for additional information
+* regarding copyright ownership. The ASF licenses this file
+* to you under the Apache License, Version 2.0 (the
+* "License"); you may not use this file except in compliance
+* with the License. You may obtain a copy of the License at
+*
+* http://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing,
+* software distributed under the License is distributed on an
+* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+* KIND, either express or implied. See the License for the
+* specific language governing permissions and limitations
+* under the License.
+ */
+
+package clioptions
+
+import (
+ "context"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func TestCLIOptions(t *testing.T) {
+ srv := util.StartServerWithCLIOptions(t, map[string]string{},
[]string{"--maxclients", "23333"})
+ defer srv.Close()
+
+ ctx := context.Background()
+ rdb := srv.NewClient()
+ defer func() { require.NoError(t, rdb.Close()) }()
+
+ require.Equal(t, "23333", rdb.ConfigGet(ctx,
"maxclients").Val()["maxclients"])
+}
diff --git a/tests/gocase/util/server.go b/tests/gocase/util/server.go
index f6c80216..7bdfeec8 100644
--- a/tests/gocase/util/server.go
+++ b/tests/gocase/util/server.go
@@ -173,6 +173,10 @@ func StartTLSServer(t testing.TB, configs
map[string]string) *KvrocksServer {
}
func StartServer(t testing.TB, configs map[string]string) *KvrocksServer {
+ return StartServerWithCLIOptions(t, configs, []string{})
+}
+
+func StartServerWithCLIOptions(t testing.TB, configs map[string]string,
options []string) *KvrocksServer {
b := *binPath
require.NotEmpty(t, b, "please set the binary path by `-binPath`")
cmd := exec.Command(b)
@@ -198,6 +202,7 @@ func StartServer(t testing.TB, configs map[string]string)
*KvrocksServer {
}
cmd.Args = append(cmd.Args, "-c", f.Name())
+ cmd.Args = append(cmd.Args, options...)
stdout, err := os.Create(filepath.Join(dir, "stdout"))
require.NoError(t, err)