Skip to content

Commit c078c6c

Browse files
AieeAiee
authored
Fix session ID generation (#5916)
* Fix session ID generation * Fix session create time * Fix tck cases * Fmt * Fix parser * Fix parser
1 parent fee249b commit c078c6c

File tree

8 files changed

+39
-34
lines changed

8 files changed

+39
-34
lines changed

‎src/graph/executor/admin/KillQueryExecutor.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ Status KillQueryExecutor::verifyTheQueriesByLocalCache(QueriesMap& toBeVerifiedQ
7979

8080
auto sessionId = sessionVal.getInt();
8181
auto epId = epVal.getInt();
82-
if (sessionId == session->id() || sessionId < 0) {
82+
if (sessionId == session->id() || sessionId == 0) {
8383
if (!session->findQuery(epId)) {
8484
return Status::Error("ExecutionPlanId[%ld] does not exist in current Session.", epId);
8585
}

‎src/meta/processors/session/SessionManagerProcessor.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,20 @@
55

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

8+
#include <folly/Random.h>
9+
810
namespace nebula {
911
namespace meta {
1012

1113
void CreateSessionProcessor::process(const cpp2::CreateSessionReq &req) {
1214
auto user = req.get_user();
1315
cpp2::Session session;
14-
// The sessionId is generated by microsecond timestamp
15-
session.session_id_ref() = time::WallClock::fastNowInMicroSec();
16-
session.create_time_ref() = session.get_session_id();
16+
int64_t rand = 0;
17+
while (rand == 0) {
18+
rand = static_cast<int64_t>(folly::Random::rand64());
19+
}
20+
session.session_id_ref() = rand; // 0 is used as a special value in kill query
21+
session.create_time_ref() = time::WallClock::fastNowInMicroSec();
1722
session.update_time_ref() = session.get_create_time();
1823
session.user_name_ref() = user;
1924
session.graph_addr_ref() = req.get_graph_addr();

‎src/parser/parser.yy

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3497,7 +3497,7 @@ show_sentence
34973497
| KW_SHOW KW_LOCAL KW_SESSIONS {
34983498
$$ = new ShowSessionsSentence(true);
34993499
}
3500-
| KW_SHOW KW_SESSION legal_integer {
3500+
| KW_SHOW KW_SESSION unary_integer {
35013501
$$ = new ShowSessionsSentence($3);
35023502
}
35033503
| KW_SHOW KW_META KW_LEADER {
@@ -3886,7 +3886,7 @@ kill_session_sentence
38863886
;
38873887

38883888
query_unique_identifier_value
3889-
: legal_integer {
3889+
: unary_integer {
38903890
$$ = ConstantExpression::make(qctx->objPool(), $1);
38913891
}
38923892
| input_prop_expression {
@@ -3896,7 +3896,7 @@ query_unique_identifier_value
38963896

38973897
query_unique_identifier
38983898
: KW_PLAN ASSIGN query_unique_identifier_value {
3899-
$$ = new QueryUniqueIdentifier($3, ConstantExpression::make(qctx->objPool(), Value(-1)));
3899+
$$ = new QueryUniqueIdentifier($3, ConstantExpression::make(qctx->objPool(), Value(0)));
39003900
}
39013901
| KW_SESSION ASSIGN query_unique_identifier_value COMMA KW_PLAN ASSIGN query_unique_identifier_value {
39023902
$$ = new QueryUniqueIdentifier($7, $3);

‎tests/tck/features/admin/Sessions.feature

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ Feature: Test sessions
1313
SHOW SESSIONS;
1414
"""
1515
Then the result should contain:
16-
| SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp |
17-
| /\d+/ | "root" | "" | /.*/ | /.*/ | /.*/ | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
16+
| SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp |
17+
| /[-+]?\d+/ | "root" | "" | /.*/ | /.*/ | /.*/ | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
1818
When executing query:
1919
"""
2020
CREATE USER user1 WITH PASSWORD 'nebula1';
@@ -29,9 +29,9 @@ Feature: Test sessions
2929
SHOW SESSIONS;
3030
"""
3131
Then the result should contain, replace the holders with cluster info:
32-
| SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp |
33-
| /\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/ |
34-
| /\d+/ | "user1" | "" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[1].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
32+
| SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp |
33+
| /[-+]?\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/ |
34+
| /[-+]?\d+/ | "user1" | "" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[1].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
3535

3636
@distonly
3737
Scenario: Show local sessions
@@ -40,8 +40,8 @@ Feature: Test sessions
4040
SHOW SESSIONS;
4141
"""
4242
Then the result should contain:
43-
| SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp |
44-
| /\d+/ | "root" | "" | /.*/ | /.*/ | /.*/ | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
43+
| SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp |
44+
| /[-+]?\d+/ | "root" | "" | /.*/ | /.*/ | /.*/ | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
4545
When executing query:
4646
"""
4747
CREATE USER IF NOT EXISTS user1 WITH PASSWORD 'nebula1';
@@ -61,14 +61,14 @@ Feature: Test sessions
6161
SHOW SESSIONS;
6262
"""
6363
Then the result should contain, replace the holders with cluster info:
64-
| SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp |
65-
| /\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/ |
66-
| /\d+/ | "user1" | "" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[1].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
67-
| /\d+/ | "user2" | "" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[2].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
64+
| SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp |
65+
| /[-+]?\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/ |
66+
| /[-+]?\d+/ | "user1" | "" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[1].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
67+
| /[-+]?\d+/ | "user2" | "" | /.*/ | /.*/ | "127.0.0.1:${cluster.graphd_processes[2].tcp_port}" | 0 | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
6868
When executing query:
6969
"""
7070
SHOW LOCAL SESSIONS;
7171
"""
7272
Then the result should contain, replace the holders with cluster info:
73-
| SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp |
74-
| /\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/ |
73+
| SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp |
74+
| /[-+]?\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/ |

‎tests/tck/slowquery/KillSlowQueryViaDiffrentService.feature

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ Feature: Slow Query Test
3030
WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100000 STEPS";
3131
"""
3232
Then the result should be, in order:
33-
| sid | eid | dur |
34-
| /\d+/ | /\d+/ | /\d+/ |
33+
| sid | eid | dur |
34+
| /[-+]?\d+/ | /\d+/ | /\d+/ |
3535
# sessionId not exist
3636
When executing query via graph 1:
3737
"""

‎tests/tck/slowquery/KillSlowQueryViaSameService.feature

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ Feature: Slow Query Test
3030
WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100000 STEPS";
3131
"""
3232
Then the result should be, in order:
33-
| sid | eid | dur |
34-
| /\d+/ | /\d+/ | /\d+/ |
33+
| sid | eid | dur |
34+
| /[-+]?\d+/ | /\d+/ | /\d+/ |
3535
When executing query:
3636
"""
3737
SHOW QUERIES

‎tests/tck/slowquery/PermissionViaDifferentService.feature

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ Feature: Test kill queries permission from different services
3232
WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100001 STEPS";
3333
"""
3434
Then the result should be, in order:
35-
| sid | eid | dur |
36-
| /\d+/ | /\d+/ | /\d+/ |
35+
| sid | eid | dur |
36+
| /[-+]?\d+/ | /\d+/ | /\d+/ |
3737
# Kill failed by user test_permission
3838
When executing query with user "test_permission" and password "test":
3939
"""
@@ -88,8 +88,8 @@ Feature: Test kill queries permission from different services
8888
WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100002 STEPS";
8989
"""
9090
Then the result should be, in order:
91-
| sid | eid | dur |
92-
| /\d+/ | /\d+/ | /\d+/ |
91+
| sid | eid | dur |
92+
| /[-+]?\d+/ | /\d+/ | /\d+/ |
9393
When executing query with user "test_permission" and password "test":
9494
"""
9595
USE nba;
@@ -132,8 +132,8 @@ Feature: Test kill queries permission from different services
132132
WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100003 STEPS";
133133
"""
134134
Then the result should be, in order:
135-
| sid | eid | dur |
136-
| /\d+/ | /\d+/ | /\d+/ |
135+
| sid | eid | dur |
136+
| /[-+]?\d+/ | /\d+/ | /\d+/ |
137137
When executing query with user "root" and password "nebula":
138138
"""
139139
USE nba;

‎tests/tck/slowquery/PermissionViaSameService.feature

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ Feature: Test kill queries from same service
3131
WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100001 STEPS";
3232
"""
3333
Then the result should be, in order:
34-
| sid | eid | dur |
35-
| /\d+/ | /\d+/ | /\d+/ |
34+
| sid | eid | dur |
35+
| /[-+]?\d+/ | /\d+/ | /\d+/ |
3636
When executing query with user "test_permission" and password "test":
3737
"""
3838
USE nba;
@@ -85,8 +85,8 @@ Feature: Test kill queries from same service
8585
WHERE $-.DurationInUSec > 1000000 AND $-.`Query` CONTAINS "GO 100002 STEPS";
8686
"""
8787
Then the result should be, in order:
88-
| sid | eid | dur |
89-
| /\d+/ | /\d+/ | /\d+/ |
88+
| sid | eid | dur |
89+
| /[-+]?\d+/ | /\d+/ | /\d+/ |
9090
When executing query with user "test_permission" and password "test":
9191
"""
9292
USE nba;

0 commit comments

Comments
 (0)