-
Notifications
You must be signed in to change notification settings - Fork 6k
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
tools/ceph_kvstore_tool: Move summary output to print_summary #26666
Conversation
src/tools/kvstore_tool.cc
Outdated
@@ -179,6 +179,19 @@ bool StoreTool::rm_prefix(const string& prefix) | |||
return (ret == 0); | |||
} | |||
|
|||
void StoreTool::print_summary(const uint64_t total_keys, const uint64_t total_size, | |||
const uint64_t total_txs, const string &store_path, | |||
const string &other_path, const int duration) const |
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.
nit, might want to put &
right next to string
. as it's part of type.
src/tools/kvstore_tool.h
Outdated
@@ -62,6 +62,9 @@ class StoreTool | |||
ceph::bufferlist& val); | |||
bool rm(const std::string& prefix, const std::string& key); | |||
bool rm_prefix(const std::string& prefix); | |||
void print_summary(const uint64_t total_keys, const uint64_t total_size, | |||
const uint64_t total_txs, const string &store_path, |
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.
ditto.
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.
Bah! Must be a clang-format thing... using LLVM style...
f504546
to
7935cf8
Compare
src/tools/kvstore_tool.h
Outdated
@@ -62,6 +62,9 @@ class StoreTool | |||
ceph::bufferlist& val); | |||
bool rm(const std::string& prefix, const std::string& key); | |||
bool rm_prefix(const std::string& prefix); | |||
void print_summary(const uint64_t total_keys, const uint64_t total_size, | |||
const uint64_t total_txs, const string& store_path, |
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.
s/string/std::string/
we'd better not relying on using namespace std
or using std::string
somewhere else.
Post 301a642 we are still seeing an ICE in the copy_store_to code. Moving the summary printing to its own function alleviates the issue. Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
7935cf8
to
504ab05
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.
voodoo.
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.
Honestly I did not read the content carefully. But I'd like to contribute my +1 vote here since my local compiler is happy again with this patch applied.
Thanks!
It would be much appreciated if we can get this patch merged ASAP. |
Testing it now. |
Seems like the testing queue has stalled. I'll check it in the morning APAC time. |
Post 301a642 we are still seeing an ICE
in the copy_store_to code. Moving the summary printing to its own
function alleviates the issue.
Signed-off-by: Brad Hubbard bhubbard@redhat.com