diff --git a/conf/nebula-graphd.conf.default b/conf/nebula-graphd.conf.default index fc2432a20f3..69eee083941 100644 --- a/conf/nebula-graphd.conf.default +++ b/conf/nebula-graphd.conf.default @@ -51,10 +51,11 @@ --reuse_port=false # Backlog of the listen socket, adjust this together with net.core.somaxconn --listen_backlog=1024 -# Seconds before the idle connections are closed, 0 for never closed ---client_idle_timeout_secs=0 -# Seconds before the idle sessions are expired, 0 for no expiration ---session_idle_timeout_secs=0 +# The number of seconds Nebula service waits before closing the idle connections +--client_idle_timeout_secs=28800 +# The number of seconds before idle sessions expire +# The range should be in [1, 604800] +--session_idle_timeout_secs=28800 # The number of threads to accept incoming connections --num_accept_threads=1 # The number of networking IO threads, 0 for # of CPU cores diff --git a/conf/nebula-graphd.conf.production b/conf/nebula-graphd.conf.production index 09f26e5169e..3c1518f90cd 100644 --- a/conf/nebula-graphd.conf.production +++ b/conf/nebula-graphd.conf.production @@ -49,10 +49,11 @@ --reuse_port=false # Backlog of the listen socket, adjust this together with net.core.somaxconn --listen_backlog=1024 -# Seconds before the idle connections are closed, 0 for never closed ---client_idle_timeout_secs=0 -# Seconds before the idle sessions are expired, 0 for no expiration ---session_idle_timeout_secs=60000 +# The number of seconds Nebula service waits before closing the idle connections +--client_idle_timeout_secs=28800 +# The number of seconds before idle sessions expire +# The range should be in [1, 604800] +--session_idle_timeout_secs=28800 # The number of threads to accept incoming connections --num_accept_threads=1 # The number of networking IO threads, 0 for # of CPU cores diff --git a/src/graph/executor/admin/ConfigExecutor.cpp b/src/graph/executor/admin/ConfigExecutor.cpp index bdffc7ea6dd..e5f49b6261b 100644 --- a/src/graph/executor/admin/ConfigExecutor.cpp +++ b/src/graph/executor/admin/ConfigExecutor.cpp @@ -63,9 +63,30 @@ folly::Future SetConfigExecutor::execute() { SCOPED_TIMER(&execTime_); auto *scNode = asNode(node()); + auto module = scNode->getModule(); + auto name = scNode->getName(); + auto value = scNode->getValue(); + + // This is a workaround for gflag value validation + // Currently, only --session_idle_timeout_secs has a gflag validtor + if (module == meta::cpp2::ConfigModule::GRAPH) { + // Update local cache before sending request + // Gflag value will be checked in SetCommandLineOption() if the flag validtor is registed + auto valueStr = meta::GflagsManager::ValueToGflagString(value); + if (value.isMap() && value.getMap().kvs.empty()) { + // Be compatible with previous configuration + valueStr = "{}"; + } + // Check result + auto setRes = gflags::SetCommandLineOption(name.c_str(), valueStr.c_str()); + if (setRes.empty()) { + return Status::Error("Invalid value %s for gflag --%s", valueStr.c_str(), name.c_str()); + } + } + return qctx() ->getMetaClient() - ->setConfig(scNode->getModule(), scNode->getName(), scNode->getValue()) + ->setConfig(module, name, value) .via(runner()) .thenValue([scNode](StatusOr resp) { if (!resp.ok()) { diff --git a/src/graph/service/GraphFlags.cpp b/src/graph/service/GraphFlags.cpp index c95272d563e..e31439ff2c3 100644 --- a/src/graph/service/GraphFlags.cpp +++ b/src/graph/service/GraphFlags.cpp @@ -9,15 +9,13 @@ DEFINE_int32(port, 3699, "Nebula Graph daemon's listen port"); DEFINE_int32(client_idle_timeout_secs, - 0, - "Seconds before we close the idle connections, 0 for infinite"); -DEFINE_int32(session_idle_timeout_secs, - 0, - "Seconds before we expire the idle sessions, 0 for infinite"); + 28800, + "The number of seconds Nebula service waits before closing the idle connections"); +DEFINE_int32(session_idle_timeout_secs, 28800, "The number of seconds before idle sessions expire"); DEFINE_int32(session_reclaim_interval_secs, 10, "Period we try to reclaim expired sessions"); DEFINE_int32(num_netio_threads, 0, - "Number of networking threads, 0 for number of physical CPU cores"); + "The number of networking threads, 0 for number of physical CPU cores"); DEFINE_int32(num_accept_threads, 1, "Number of threads to accept incoming connections"); DEFINE_int32(num_worker_threads, 0, "Number of threads to execute user queries"); DEFINE_bool(reuse_port, true, "Whether to turn on the SO_REUSEPORT option"); @@ -71,3 +69,16 @@ DEFINE_string(client_white_list, "A white list for different client versions, separate with colon."); DEFINE_int32(num_rows_to_check_memory, 1024, "number rows to check memory"); + +// Sanity-checking Flag Values +static bool ValidateSessIdleTimeout(const char* flagname, int32_t value) { + // The max timeout is 604800 seconds(a week) + if (value > 0 && value < 604801) // value is ok + return true; + + FLOG_WARN("Invalid value for --%s: %d, the timeout should be an integer between 1 and 604800\n", + flagname, + (int)value); + return false; +} +DEFINE_validator(session_idle_timeout_secs, &ValidateSessIdleTimeout); diff --git a/src/graph/session/GraphSessionManager.cpp b/src/graph/session/GraphSessionManager.cpp index a5faa5c22b5..c11003d9eb1 100644 --- a/src/graph/session/GraphSessionManager.cpp +++ b/src/graph/session/GraphSessionManager.cpp @@ -151,7 +151,10 @@ void GraphSessionManager::threadFunc() { // TODO(dutor) Now we do a brute-force scanning, of course we could make it more // efficient. void GraphSessionManager::reclaimExpiredSessions() { + DCHECK_GT(FLAGS_session_idle_timeout_secs, 0); if (FLAGS_session_idle_timeout_secs == 0) { + LOG(ERROR) << "Program should not reach here, session_idle_timeout_secs should be an integer " + "between 1 and 604800"; return; } diff --git a/tests/admin/test_configs.py b/tests/admin/test_configs.py index 2a8fe58736f..600d4f0eee2 100644 --- a/tests/admin/test_configs.py +++ b/tests/admin/test_configs.py @@ -36,6 +36,12 @@ def test_configs(self): resp = self.client.execute('UPDATE CONFIGS storage:v={}'.format(3)) self.check_resp_succeeded(resp) + # update flag to an invalid value, expected to fail + resp = self.client.execute('UPDATE CONFIGS graph:session_idle_timeout_secs={}'.format(0)) + self.check_resp_failed(resp) + resp = self.client.execute('UPDATE CONFIGS graph:session_idle_timeout_secs={}'.format(999999)) + self.check_resp_failed(resp) + # get resp = self.client.execute('GET CONFIGS meta:v') self.check_resp_failed(resp) @@ -66,7 +72,7 @@ def test_configs(self): ['GRAPH', 'accept_partial_success', 'bool', 'MUTABLE', False], ['GRAPH', 'system_memory_high_watermark_ratio', 'float', 'MUTABLE', 0.95], ['GRAPH', 'num_rows_to_check_memory', 'int', 'MUTABLE', 4], - ['GRAPH', 'session_idle_timeout_secs', 'int', 'MUTABLE', 0], + ['GRAPH', 'session_idle_timeout_secs', 'int', 'MUTABLE', 28800], ['GRAPH', 'session_reclaim_interval_secs', 'int', 'MUTABLE', 2], ['GRAPH', 'max_allowed_connections', 'int', 'MUTABLE', 9223372036854775807], ['GRAPH', 'disable_octal_escape_char', 'bool', 'MUTABLE', False], @@ -101,7 +107,7 @@ def test_configs(self): max_write_buffer_number="4"} ''') self.check_resp_succeeded(resp) - + # get result resp = self.client.execute('GET CONFIGS storage:rocksdb_column_family_options') self.check_resp_succeeded(resp) diff --git a/tests/job/test_session.py b/tests/job/test_session.py index 19b602a23c6..7f89a960633 100644 --- a/tests/job/test_session.py +++ b/tests/job/test_session.py @@ -127,7 +127,7 @@ def test_sessions(self): time.sleep(3) resp = self.execute('SHOW SESSION {}'.format(session_id)) self.check_resp_failed(resp, ttypes.ErrorCode.E_EXECUTION_ERROR) - resp = self.execute('UPDATE CONFIGS graph:session_idle_timeout_secs = 0') + resp = self.execute('UPDATE CONFIGS graph:session_idle_timeout_secs = 28800') self.check_resp_succeeded(resp) time.sleep(3)