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

Jump to bottom

add a console command to show meta leader #2542

Merged
merged 8 commits into from
Aug 26, 2021
Merged

add a console command to show meta leader #2542

merged 8 commits into from
Aug 26, 2021

Conversation

liuyu85cn
Copy link
Contributor

@liuyu85cn liuyu85cn commented Aug 19, 2021 •
edited
Loading

add a command to show the leader of meta.

(root@nebula) [(none)]> show meta leader
+----------------------+---------------------------+
| Meta Leader          | secs from last heart beat |
+----------------------+---------------------------+
| "192.168.8.211:6500" | 4                         |
+----------------------+---------------------------+
Got 1 rows (time spent 690/1559 us)

Tue, 24 Aug 2021 12:56:02 CST

critical27
critical27 reviewed Aug 19, 2021
View reviewed changes
src/graph/executor/admin/ShowMetaLeaderExecutor.cpp

folly::Future<Status> ShowMetaLeaderExecutor::execute() {
SCOPED_TIMER(&execTime_);
auto metaLeader = qctx()->getMetaClient()->getMetaLeader();
Copy link
Contributor

@critical27 critical27 Aug 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about record a time of last hb succeed in meta client, when now - last > threshold, just return some error.

@critical27 critical27 added the ready-for-testing PR: ready for the CI test label Aug 24, 2021
critical27
critical27 previously approved these changes Aug 24, 2021
View reviewed changes
Copy link
Contributor

@critical27 critical27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job

panda-sheep
panda-sheep previously approved these changes Aug 24, 2021
View reviewed changes
Copy link
Contributor

@panda-sheep panda-sheep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job！

src/graph/executor/admin/ShowMetaLeaderExecutor.cpp Outdated
auto metaLeader = qctx()->getMetaClient()->getMetaLeader();
auto lastHeartBeatTime = qctx()->getMetaClient()->getLastHeartBeatTime();
auto localTime = time::TimeUtils::utcToDateTime(lastHeartBeatTime);
DataSet ds({"Meta Leader", "Last success heartbeat"});
Copy link
Contributor

@panda-sheep panda-sheep Aug 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last successful heartbeat or Last heartbeat ？
Not important

yixinglu
yixinglu reviewed Aug 24, 2021
View reviewed changes
src/clients/meta/MetaClient.cpp Outdated
@@ -608,6 +609,8 @@ void MetaClient::getResponse(Request req,
auto&& resp = t.value();
if (resp.get_code() == nebula::cpp2::ErrorCode::SUCCEEDED) {
// succeeded
lastHeartBeatTime_ = time::TimeUtils::utcDateTime();
LOG(INFO) << "utcDateTime() : " << lastHeartBeatTime_.toString();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not proper to log here, this interface is heavily used by MetaClient.

src/clients/meta/MetaClient.h Outdated
@@ -599,6 +599,10 @@ class MetaClient {

folly::Future<StatusOr<bool>> ingest(GraphSpaceID spaceId);

HostAddr getMetaLeader() { return leader_; }

DateTime getLastHeartBeatTime() { return lastHeartBeatTime_; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to HeatbeatTime

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean heartbeat is one word not two words, please don't capitalize the part beat. Use Heartbeat

src/clients/meta/MetaClient.h Outdated Show resolved Hide resolved
src/graph/executor/admin/ShowMetaLeaderExecutor.cpp Outdated Show resolved Hide resolved
@yixinglu yixinglu added doc affected PR: improvements or additions to documentation and removed ready-for-testing PR: ready for the CI test labels Aug 24, 2021
@CLAassistant
Copy link

CLAassistant commented Aug 24, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@HarrisChu
Copy link
Contributor

just a question, "Last success heartbeat" means meta raft heartbeat or heartbeat from graph to meta leader?
and the "Meta Leader" means the leader when last heartbeat from graph, is it right?

  1. graph send heartbeat to meta, meta leader is A
  2. meta leader change, now is B
  3. run "show meta leader" command
  4. result is A or B ?

@liuyu85cn
Copy link
Contributor Author

just a question, "Last success heartbeat" means meta raft heartbeat or heartbeat from graph to meta leader?
and the "Meta Leader" means the leader when last heartbeat from graph, is it right?

  1. graph send heartbeat to meta, meta leader is A
  2. meta leader change, now is B
  3. run "show meta leader" command
  4. result is A or B ?

Good question, the answer is A.
That's why need the second column to show the time point of heart beat.

(Actually I will change it to interval from last heart beat,
it will be more clearly to determine if there are something wrong in meta client)

@HarrisChu
Copy link
Contributor

OK, clear

@liuyu85cn liuyu85cn dismissed stale reviews from panda-sheep and critical27 via 3389c8a August 24, 2021 04:56
panda-sheep
panda-sheep previously approved these changes Aug 24, 2021
View reviewed changes
@critical27 critical27 added the ready-for-testing PR: ready for the CI test label Aug 25, 2021
bright-starry-sky
bright-starry-sky approved these changes Aug 26, 2021
View reviewed changes
Copy link
Contributor

@bright-starry-sky bright-starry-sky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job.

@Shylock-Hg
Copy link
Contributor

Please add test

@critical27 critical27 merged commit 31cdf3d into vesoft-inc:master Aug 26, 2021
@jamieliu1023 jamieliu1023 mentioned this pull request Aug 28, 2021
Closed
@liuyu85cn liuyu85cn deleted the get_meta_leader branch October 19, 2021 05:56
yixinglu pushed a commit to yixinglu/nebula that referenced this pull request Sep 14, 2023
@nebula-bots @AntiTopQuark @Sophie-Xie
Support the tck format for the output of EXPLAIN (vesoft-inc#2542)
666cf5c 
* Support the tck format for the output of EXPLAIN

* Support the tck format for the output of EXPLAIN

---------

Co-authored-by: AntiTopQuark <[email protected]>
Co-authored-by: Sophie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yixinglu yixinglu yixinglu left review comments

@panda-sheep panda-sheep panda-sheep approved these changes

@bright-starry-sky bright-starry-sky bright-starry-sky approved these changes

@critical27 critical27 critical27 approved these changes

@cangfengzhs cangfengzhs Awaiting requested review from cangfengzhs

@darionyaphet darionyaphet Awaiting requested review from darionyaphet

Assignees
No one assigned
Labels
doc affected PR: improvements or additions to documentation ready-for-testing PR: ready for the CI test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@liuyu85cn @CLAassistant @HarrisChu @Shylock-Hg @yixinglu @critical27 @bright-starry-sky @panda-sheep