-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Add managed-ledger admin tool to access ml binary information #526
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
Conversation
|
We don't have doc for managed-ledger protobuf compilation instruction. If we think it will be helpful then I will create a separate PR to add ML-protobuf location, compilation and will add python protobuf generation instruction as well.?? |
merlimat
left a comment
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.
Change looks good and a very useful addition. Just few comment on minor naming stuff.
| @@ -0,0 +1,342 @@ | |||
| # Generated by the protocol buffer compiler. DO NOT EDIT! | |||
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.
Make sure to that the maven license plugin will ignore this, otherwise maven install will fail
| @@ -0,0 +1,261 @@ | |||
| #!/usr/bin/env python | |||
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.
Should we just place this scripts under bin/python/... ?
And include this in the src/bin distribution tgz.
At the same time, let's just use a similar naming as the other CLI tools. Eg:
bin/pulsar-managed-ledger-admin
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.
Also, license headers are missing
| import argparse | ||
| import traceback | ||
|
|
||
| from kazoo.client import KazooClient |
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.
if importing kazoo fails, can we print something like:
You ned Kazoo ZK client library. Get it from: "pip install kazoo"
| parser = argparse.ArgumentParser() | ||
| commandHelpText = 'Managed-ledger command: \n{}, {}, {}, {}'.format(printMlCommand, deleteMlLedgerIds, printCursorsCommands, updateMakDeleteCursor) | ||
| parser.add_argument("--zkServer", "-zk", required=True, help="ZooKeeperServer:port") | ||
| parser.add_argument("--command", "-cmd", required=True, help=commandHelpText) |
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.
Instead of getting --command as an argument, to behave the same way as the other tools, can we get it as a positional parameter? eg:
ManagedLedgerAdminTool.py print-cursor --zkServer localhost:2181 \
--mlPath sample/standalone/ns1/persistent/test --cursorName s1
| commandHelpText = 'Managed-ledger command: \n{}, {}, {}, {}'.format(printMlCommand, deleteMlLedgerIds, printCursorsCommands, updateMakDeleteCursor) | ||
| parser.add_argument("--zkServer", "-zk", required=True, help="ZooKeeperServer:port") | ||
| parser.add_argument("--command", "-cmd", required=True, help=commandHelpText) | ||
| parser.add_argument("--mlPath", "-mlp", required=True, help="Managed-ledger 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.
mlPath and ml in general is quite confusing for people not used to the context. Can we use managed-ledger instead?
| # get managed-ledger info | ||
| mlData = zk.get(mlPath)[0] | ||
| mlInfo = MLDataFormats_pb2.ManagedLedgerInfo() | ||
| mlInfo.ParseFromString(mlData) |
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.
Is this handling the protobuf text format as well?
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.
yes, earlier I thought we will use this tool after merging PR which forces to write in Binary format. However, I have added Text support also.
… handling kazoo package, update mlPath-argument
|
@merlimat addressed all the changes. |
bin/pulsar-managed-ledger-admin.py
Outdated
| @@ -0,0 +1,312 @@ | |||
| #!/usr/bin/env bash | |||
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.
This should be python instead of bash
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.
We can just call it bin/pulsar-managed-ledger-admin, no need for py extension here.
bin/pulsar-managed-ledger-admin.py
Outdated
| try: | ||
| from kazoo.client import KazooClient | ||
| except Exception as missingLib: | ||
| print "You need Kazoo ZK client library. Get it from: pip install kazoo" |
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.
Need to exit here with error code
merlimat
left a comment
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.
👍
Motivation
After merging #394, broker will start storing managed-ledger data into binary format which can't be access by
bin/pulsar zookeeper-shellanymore. So, incase of any data-corruption (eg. any human-error while accessing znode), we need a quick way to build a tool/script to access ml-data. therefore, adding python-admin tool which provides API and command-line tool access to access ml data.Modifications
Added a tool which provides api to
Result
No functional change.