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.

Sign up for GitHub

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

Fix session ID generation #5916

Merged
merged 6 commits into from
Aug 8, 2024
Merged

Fix session ID generation #5916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
945b0a6
Fix session ID generation
Aiee Aug 6, 2024
7ef1f17
Fix session create time
Aiee Aug 7, 2024
4bb6401
Fix tck cases
Aiee Aug 7, 2024
f2ac797
Fmt
Aiee Aug 7, 2024
d3b820a
Fix parser
Aiee Aug 7, 2024
7938f7f
Fix parser
Aiee Aug 7, 2024
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
2 changes: 1 addition & 1 deletion src/graph/executor/admin/KillQueryExecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
11 changes: 8 additions & 3 deletions src/meta/processors/session/SessionManagerProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,20 @@

#include "meta/processors/session/SessionManagerProcessor.h"

#include <folly/Random.h>

namespace nebula {
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() = time::WallClock::fastNowInMicroSec();
session.create_time_ref() = session.get_session_id();
int64_t rand = 0;
while (rand == 0) {
rand = static_cast<int64_t>(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;
session.graph_addr_ref() = req.get_graph_addr();
Expand Down
6 changes: 3 additions & 3 deletions src/parser/parser.yy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -3886,7 +3886,7 @@ kill_session_sentence
;

query_unique_identifier_value
: legal_integer {
: unary_integer {
$$ = ConstantExpression::make(qctx->objPool(), $1);
}
| input_prop_expression {
Expand All @@ -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);
Expand Down
26 changes: 13 additions & 13 deletions tests/tck/features/admin/Sessions.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
Expand All @@ -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';
Expand All @@ -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/ |
4 changes: 2 additions & 2 deletions tests/tck/slowquery/KillSlowQueryViaDiffrentService.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
"""
Expand Down
4 changes: 2 additions & 2 deletions tests/tck/slowquery/KillSlowQueryViaSameService.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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+/ |
When executing query:
"""
SHOW QUERIES
Expand Down
12 changes: 6 additions & 6 deletions tests/tck/slowquery/PermissionViaDifferentService.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ 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 |
| /\d+/ | /\d+/ | /\d+/ |
| sid | eid | dur |
| /[-+]?\d+/ | /\d+/ | /\d+/ |
# Kill failed by user test_permission
When executing query with user "test_permission" and password "test":
"""
Expand Down Expand Up @@ -88,8 +88,8 @@ 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 |
| /\d+/ | /\d+/ | /\d+/ |
| sid | eid | dur |
| /[-+]?\d+/ | /\d+/ | /\d+/ |
When executing query with user "test_permission" and password "test":
"""
USE nba;
Expand Down Expand Up @@ -132,8 +132,8 @@ 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 |
| /\d+/ | /\d+/ | /\d+/ |
| sid | eid | dur |
| /[-+]?\d+/ | /\d+/ | /\d+/ |
When executing query with user "root" and password "nebula":
"""
USE nba;
Expand Down
8 changes: 4 additions & 4 deletions tests/tck/slowquery/PermissionViaSameService.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ 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 |
| /\d+/ | /\d+/ | /\d+/ |
| sid | eid | dur |
| /[-+]?\d+/ | /\d+/ | /\d+/ |
When executing query with user "test_permission" and password "test":
"""
USE nba;
Expand Down Expand Up @@ -85,8 +85,8 @@ 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 |
| /\d+/ | /\d+/ | /\d+/ |
| sid | eid | dur |
| /[-+]?\d+/ | /\d+/ | /\d+/ |
When executing query with user "test_permission" and password "test":
"""
USE nba;
Expand Down
Loading