-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
Support "show parts" command #1086
Conversation
7102a05
to
aeeb9ae
Compare
aeeb9ae
to
acbbc2c
Compare
acbbc2c
to
4428029
Compare
Unit testing failed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's cool. Could we show the results displayed by console?
Jenkins go |
Unit testing failed. |
of course! |
4428029
to
1a3530a
Compare
Jenkins go |
Unit testing passed. |
1 similar comment
Unit testing passed. |
1a3530a
to
243d67a
Compare
Jenkins go |
Unit testing passed. |
1 similar comment
Unit testing passed. |
You could post it in the summary. |
Thanks, I have updated the summary of this PR. |
I means you could post the results displayed by console in this summary. |
Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done!
src/common/network/NetworkUtils.cpp
Outdated
std::string addrStr = network::NetworkUtils::ipFromHostAddr(host); | ||
int32_t port = network::NetworkUtils::portFromHostAddr(host); | ||
std::string portStr = folly::to<std::string>(port); | ||
hostsString += addrStr + ":" + portStr + ", "; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
folly::stringPrintf("%s:%d,", addrStr.c_str(), port);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
Unit testing passed. |
Unit testing failed. |
namespace nebula { | ||
namespace meta { | ||
|
||
void ListPartsProcessor::process(const cpp2::ListPartsReq& req) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we can support list the partition specified in console? If we have to many partitions, it's hard to look for a single partition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. We'd better support to show some part information specified too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The result is sorted by partition ID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, we could open another pr for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, would it be better to use the LIMIT
clause?
2a8f708
to
ef9ea77
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean code! LGTM
Jenkins go! |
@@ -306,6 +306,19 @@ StatusOr<std::vector<HostAddr>> NetworkUtils::toHosts(const std::string& peersSt | |||
return hosts; | |||
} | |||
|
|||
std::string NetworkUtils::toHosts(const std::vector<HostAddr>& hosts) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
concatHosts may be a better func name to explain this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx. I prefer the toHosts
function overload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
concatHosts
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, the Concat generally refers to joining the same kind of things and returning the original form. For example, joining multiple strings returns a string. There is a formal shift here(HostAddr to String, with the meaning of convert to), not a simple join or concat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toHosts +1
Unit testing passed. |
2 similar comments
Unit testing passed. |
Unit testing passed. |
Unit testing passed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
subtask of #724 |
ListPartsProcessor
to get all partitions' information(id, leader, peers, losts) of one graph space.SHOW PARTS
command can show the results displayed by console, after doingUSE space_name
.close #930
Example:
Init and start the nebula graph cluster:
Create the graph spaces(named myspace) with 9 partitions and 3 replicas:
Show the hosts with myspace:
Show parts of myspace, before leader election:
Show parts of myspace, after leader election:
Stop the storaged 172.25.61.1:44500:
Wait for the expired_threshold_sec(10 * 60) seconds:
Start the storaged 172.25.61.1:44500 again: