From 945b0a6a73fa648b4bba35799827700fe328cfb6 Mon Sep 17 00:00:00 2001 From: Aiee <18348405+Aiee@users.noreply.github.com> Date: Tue, 6 Aug 2024 18:12:01 +0800 Subject: [PATCH 1/6] Fix session ID generation --- src/meta/processors/session/SessionManagerProcessor.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/meta/processors/session/SessionManagerProcessor.cpp b/src/meta/processors/session/SessionManagerProcessor.cpp index 47449671ef2..9ea499a6ae8 100644 --- a/src/meta/processors/session/SessionManagerProcessor.cpp +++ b/src/meta/processors/session/SessionManagerProcessor.cpp @@ -5,6 +5,8 @@ #include "meta/processors/session/SessionManagerProcessor.h" +#include + namespace nebula { namespace meta { @@ -12,7 +14,7 @@ void CreateSessionProcessor::process(const cpp2::CreateSessionReq &req) { auto user = req.get_user(); cpp2::Session session; // The sessionId is generated by microsecond timestamp - session.session_id_ref() = time::WallClock::fastNowInMicroSec(); + session.session_id_ref() = folly::Random::rand64(); session.create_time_ref() = session.get_session_id(); session.update_time_ref() = session.get_create_time(); session.user_name_ref() = user; From 7ef1f17d93282bb1e1c0ae8879dfdcd268d2a07d Mon Sep 17 00:00:00 2001 From: Aiee <18348405+Aiee@users.noreply.github.com> Date: Wed, 7 Aug 2024 12:42:45 +0800 Subject: [PATCH 2/6] Fix session create time --- src/meta/processors/session/SessionManagerProcessor.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/meta/processors/session/SessionManagerProcessor.cpp b/src/meta/processors/session/SessionManagerProcessor.cpp index 9ea499a6ae8..6c02fbc5aa7 100644 --- a/src/meta/processors/session/SessionManagerProcessor.cpp +++ b/src/meta/processors/session/SessionManagerProcessor.cpp @@ -14,8 +14,8 @@ void CreateSessionProcessor::process(const cpp2::CreateSessionReq &req) { auto user = req.get_user(); cpp2::Session session; // The sessionId is generated by microsecond timestamp - session.session_id_ref() = folly::Random::rand64(); - session.create_time_ref() = session.get_session_id(); + session.session_id_ref() = static_cast(folly::Random::rand64()); + session.create_time_ref() = time::WallClock::fastNowInMicroSec(); session.update_time_ref() = session.get_create_time(); session.user_name_ref() = user; session.graph_addr_ref() = req.get_graph_addr(); From 4bb6401072825e6eabeb02bfc5026d93cd89ef9e Mon Sep 17 00:00:00 2001 From: Aiee <18348405+Aiee@users.noreply.github.com> Date: Wed, 7 Aug 2024 15:50:33 +0800 Subject: [PATCH 3/6] Fix tck cases --- tests/tck/features/admin/Sessions.feature | 26 +++++++++---------- .../KillSlowQueryViaDiffrentService.feature | 4 +-- .../KillSlowQueryViaSameService.feature | 5 ++-- .../PermissionViaDifferentService.feature | 6 ++--- .../PermissionViaSameService.feature | 4 +-- 5 files changed, 23 insertions(+), 22 deletions(-) diff --git a/tests/tck/features/admin/Sessions.feature b/tests/tck/features/admin/Sessions.feature index 2b2ddc1dfa1..5944f3cd004 100644 --- a/tests/tck/features/admin/Sessions.feature +++ b/tests/tck/features/admin/Sessions.feature @@ -13,8 +13,8 @@ Feature: Test sessions SHOW SESSIONS; """ Then the result should contain: - | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp | - | /\d+/ | "root" | "" | /.*/ | /.*/ | /.*/ | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | + | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp | + | /[-+]?\d+/ | "root" | "" | /.*/ | /.*/ | /.*/ | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | When executing query: """ CREATE USER user1 WITH PASSWORD 'nebula1'; @@ -29,9 +29,9 @@ Feature: Test sessions SHOW SESSIONS; """ Then the result should contain, replace the holders with cluster info: - | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp | - | /\d+/ | "root" | "s1" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[0].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | - | /\d+/ | "user1" | "" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[1].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | + | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp | + | /[-+]?\d+/ | "root" | "s1" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[0].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | + | /[-+]?\d+/ | "user1" | "" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[1].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | @distonly Scenario: Show local sessions @@ -40,8 +40,8 @@ Feature: Test sessions SHOW SESSIONS; """ Then the result should contain: - | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp | - | /\d+/ | "root" | "" | /.*/ | /.*/ | /.*/ | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | + | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp | + | /[-+]?\d+/ | "root" | "" | /.*/ | /.*/ | /.*/ | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | When executing query: """ CREATE USER IF NOT EXISTS user1 WITH PASSWORD 'nebula1'; @@ -61,14 +61,14 @@ Feature: Test sessions SHOW SESSIONS; """ Then the result should contain, replace the holders with cluster info: - | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp | - | /\d+/ | "root" | "root_space" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[0].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | - | /\d+/ | "user1" | "" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[1].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | - | /\d+/ | "user2" | "" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[2].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | + | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp | + | /[-+]?\d+/ | "root" | "root_space" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[0].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | + | /[-+]?\d+/ | "user1" | "" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[1].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | + | /[-+]?\d+/ | "user2" | "" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[2].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | When executing query: """ SHOW LOCAL SESSIONS; """ Then the result should contain, replace the holders with cluster info: - | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp | - | /\d+/ | "root" | "root_space" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[0].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | + | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp | + | /[-+]?\d+/ | "root" | "root_space" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[0].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ | diff --git a/tests/tck/slowquery/KillSlowQueryViaDiffrentService.feature b/tests/tck/slowquery/KillSlowQueryViaDiffrentService.feature index bd049e943bf..1786359e176 100644 --- a/tests/tck/slowquery/KillSlowQueryViaDiffrentService.feature +++ b/tests/tck/slowquery/KillSlowQueryViaDiffrentService.feature @@ -30,8 +30,8 @@ Feature: Slow Query Test WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100000 STEPS"; """ Then the result should be, in order: - | sid | eid | dur | - | /\d+/ | /\d+/ | /\d+/ | + | sid | eid | dur | + | /[-+]?\d+/ | /\d+/ | /\d+/ | # sessionId not exist When executing query via graph 1: """ diff --git a/tests/tck/slowquery/KillSlowQueryViaSameService.feature b/tests/tck/slowquery/KillSlowQueryViaSameService.feature index b911704d0e3..c8d9ffe9b11 100644 --- a/tests/tck/slowquery/KillSlowQueryViaSameService.feature +++ b/tests/tck/slowquery/KillSlowQueryViaSameService.feature @@ -1,6 +1,7 @@ # Copyright (c) 2021 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. +@aiee Feature: Slow Query Test Background: @@ -30,8 +31,8 @@ Feature: Slow Query Test WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100000 STEPS"; """ Then the result should be, in order: - | sid | eid | dur | - | /\d+/ | /\d+/ | /\d+/ | + | sid | eid | dur | + | /[-+]?\d+/ | /\d+/ | /\d+/ | When executing query: """ SHOW QUERIES diff --git a/tests/tck/slowquery/PermissionViaDifferentService.feature b/tests/tck/slowquery/PermissionViaDifferentService.feature index 066edc49152..4cd5c96c40f 100644 --- a/tests/tck/slowquery/PermissionViaDifferentService.feature +++ b/tests/tck/slowquery/PermissionViaDifferentService.feature @@ -33,7 +33,7 @@ Feature: Test kill queries permission from different services """ Then the result should be, in order: | sid | eid | dur | - | /\d+/ | /\d+/ | /\d+/ | + | /[-+]?\d+/ | /\d+/ | /\d+/ | # Kill failed by user test_permission When executing query with user "test_permission" and password "test": """ @@ -89,7 +89,7 @@ Feature: Test kill queries permission from different services """ Then the result should be, in order: | sid | eid | dur | - | /\d+/ | /\d+/ | /\d+/ | + | /[-+]?\d+/ | /\d+/ | /\d+/ | When executing query with user "test_permission" and password "test": """ USE nba; @@ -133,7 +133,7 @@ Feature: Test kill queries permission from different services """ Then the result should be, in order: | sid | eid | dur | - | /\d+/ | /\d+/ | /\d+/ | + | /[-+]?\d+/ | /\d+/ | /\d+/ | When executing query with user "root" and password "nebula": """ USE nba; diff --git a/tests/tck/slowquery/PermissionViaSameService.feature b/tests/tck/slowquery/PermissionViaSameService.feature index 5b4a66badb0..fad73a5265b 100644 --- a/tests/tck/slowquery/PermissionViaSameService.feature +++ b/tests/tck/slowquery/PermissionViaSameService.feature @@ -32,7 +32,7 @@ Feature: Test kill queries from same service """ Then the result should be, in order: | sid | eid | dur | - | /\d+/ | /\d+/ | /\d+/ | + | /[-+]?\d+/ | /\d+/ | /\d+/ | When executing query with user "test_permission" and password "test": """ USE nba; @@ -86,7 +86,7 @@ Feature: Test kill queries from same service """ Then the result should be, in order: | sid | eid | dur | - | /\d+/ | /\d+/ | /\d+/ | + | /[-+]?\d+/ | /\d+/ | /\d+/ | When executing query with user "test_permission" and password "test": """ USE nba; From f2ac79700275eb8fadb87ddf04302880af4188ce Mon Sep 17 00:00:00 2001 From: Aiee <18348405+Aiee@users.noreply.github.com> Date: Wed, 7 Aug 2024 15:53:05 +0800 Subject: [PATCH 4/6] Fmt --- tests/tck/slowquery/PermissionViaDifferentService.feature | 6 +++--- tests/tck/slowquery/PermissionViaSameService.feature | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/tck/slowquery/PermissionViaDifferentService.feature b/tests/tck/slowquery/PermissionViaDifferentService.feature index 4cd5c96c40f..75a1630cf58 100644 --- a/tests/tck/slowquery/PermissionViaDifferentService.feature +++ b/tests/tck/slowquery/PermissionViaDifferentService.feature @@ -32,7 +32,7 @@ Feature: Test kill queries permission from different services WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100001 STEPS"; """ Then the result should be, in order: - | sid | eid | dur | + | sid | eid | dur | | /[-+]?\d+/ | /\d+/ | /\d+/ | # Kill failed by user test_permission When executing query with user "test_permission" and password "test": @@ -88,7 +88,7 @@ Feature: Test kill queries permission from different services WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100002 STEPS"; """ Then the result should be, in order: - | sid | eid | dur | + | sid | eid | dur | | /[-+]?\d+/ | /\d+/ | /\d+/ | When executing query with user "test_permission" and password "test": """ @@ -132,7 +132,7 @@ Feature: Test kill queries permission from different services WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100003 STEPS"; """ Then the result should be, in order: - | sid | eid | dur | + | sid | eid | dur | | /[-+]?\d+/ | /\d+/ | /\d+/ | When executing query with user "root" and password "nebula": """ diff --git a/tests/tck/slowquery/PermissionViaSameService.feature b/tests/tck/slowquery/PermissionViaSameService.feature index fad73a5265b..49061a1ff7a 100644 --- a/tests/tck/slowquery/PermissionViaSameService.feature +++ b/tests/tck/slowquery/PermissionViaSameService.feature @@ -31,7 +31,7 @@ Feature: Test kill queries from same service WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100001 STEPS"; """ Then the result should be, in order: - | sid | eid | dur | + | sid | eid | dur | | /[-+]?\d+/ | /\d+/ | /\d+/ | When executing query with user "test_permission" and password "test": """ @@ -85,7 +85,7 @@ Feature: Test kill queries from same service WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100002 STEPS"; """ Then the result should be, in order: - | sid | eid | dur | + | sid | eid | dur | | /[-+]?\d+/ | /\d+/ | /\d+/ | When executing query with user "test_permission" and password "test": """ From d3b820af9f95d719719702bce83d49b4fb098075 Mon Sep 17 00:00:00 2001 From: Aiee <18348405+Aiee@users.noreply.github.com> Date: Wed, 7 Aug 2024 16:19:41 +0800 Subject: [PATCH 5/6] Fix parser --- src/parser/parser.yy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser/parser.yy b/src/parser/parser.yy index 27042a80db4..ebca4c3fac5 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -3497,7 +3497,7 @@ show_sentence | KW_SHOW KW_LOCAL KW_SESSIONS { $$ = new ShowSessionsSentence(true); } - | KW_SHOW KW_SESSION legal_integer { + | KW_SHOW KW_SESSION unary_integer { $$ = new ShowSessionsSentence($3); } | KW_SHOW KW_META KW_LEADER { From 7938f7f2589448fd2b0ce641f5337c53cd8665ac Mon Sep 17 00:00:00 2001 From: Aiee <18348405+Aiee@users.noreply.github.com> Date: Wed, 7 Aug 2024 17:56:27 +0800 Subject: [PATCH 6/6] Fix parser --- src/graph/executor/admin/KillQueryExecutor.cpp | 2 +- src/meta/processors/session/SessionManagerProcessor.cpp | 7 +++++-- src/parser/parser.yy | 4 ++-- tests/tck/slowquery/KillSlowQueryViaSameService.feature | 1 - 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/graph/executor/admin/KillQueryExecutor.cpp b/src/graph/executor/admin/KillQueryExecutor.cpp index 55d827c6316..f59aeddf9b0 100644 --- a/src/graph/executor/admin/KillQueryExecutor.cpp +++ b/src/graph/executor/admin/KillQueryExecutor.cpp @@ -79,7 +79,7 @@ Status KillQueryExecutor::verifyTheQueriesByLocalCache(QueriesMap& toBeVerifiedQ auto sessionId = sessionVal.getInt(); auto epId = epVal.getInt(); - if (sessionId == session->id() || sessionId < 0) { + if (sessionId == session->id() || sessionId == 0) { if (!session->findQuery(epId)) { return Status::Error("ExecutionPlanId[%ld] does not exist in current Session.", epId); } diff --git a/src/meta/processors/session/SessionManagerProcessor.cpp b/src/meta/processors/session/SessionManagerProcessor.cpp index 6c02fbc5aa7..52a3a53c218 100644 --- a/src/meta/processors/session/SessionManagerProcessor.cpp +++ b/src/meta/processors/session/SessionManagerProcessor.cpp @@ -13,8 +13,11 @@ namespace meta { void CreateSessionProcessor::process(const cpp2::CreateSessionReq &req) { auto user = req.get_user(); cpp2::Session session; - // The sessionId is generated by microsecond timestamp - session.session_id_ref() = static_cast(folly::Random::rand64()); + int64_t rand = 0; + while (rand == 0) { + rand = static_cast(folly::Random::rand64()); + } + session.session_id_ref() = rand; // 0 is used as a special value in kill query session.create_time_ref() = time::WallClock::fastNowInMicroSec(); session.update_time_ref() = session.get_create_time(); session.user_name_ref() = user; diff --git a/src/parser/parser.yy b/src/parser/parser.yy index ebca4c3fac5..5a902606095 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -3886,7 +3886,7 @@ kill_session_sentence ; query_unique_identifier_value - : legal_integer { + : unary_integer { $$ = ConstantExpression::make(qctx->objPool(), $1); } | input_prop_expression { @@ -3896,7 +3896,7 @@ query_unique_identifier_value query_unique_identifier : KW_PLAN ASSIGN query_unique_identifier_value { - $$ = new QueryUniqueIdentifier($3, ConstantExpression::make(qctx->objPool(), Value(-1))); + $$ = new QueryUniqueIdentifier($3, ConstantExpression::make(qctx->objPool(), Value(0))); } | KW_SESSION ASSIGN query_unique_identifier_value COMMA KW_PLAN ASSIGN query_unique_identifier_value { $$ = new QueryUniqueIdentifier($7, $3); diff --git a/tests/tck/slowquery/KillSlowQueryViaSameService.feature b/tests/tck/slowquery/KillSlowQueryViaSameService.feature index c8d9ffe9b11..19c081fab68 100644 --- a/tests/tck/slowquery/KillSlowQueryViaSameService.feature +++ b/tests/tck/slowquery/KillSlowQueryViaSameService.feature @@ -1,7 +1,6 @@ # Copyright (c) 2021 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. -@aiee Feature: Slow Query Test Background: