[
https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13447592#comment-13447592
]
Uma Maheswara Rao G commented on BOOKKEEPER-300:
------------------------------------------------
I have had gone through the patch.
I have few minor comments to address.
{code}
System.err.println(" metaformat [-nonInteractive] [-force]");
+ System.err.println(" bookieformat [-nonInteractive] [-force]");
+ System.err.println(" recover <bookieSrc> [bookieDest]");
{code}
I think we have to document this options right? Do you mind raising a separate
JIRA and link here?
{code}
File journalDir = conf.getJournalDir();
+ if (journalDir.exists() && journalDir.isDirectory()
{code}
{code}
File[] ledgerDirs = conf.getLedgerDirs();
+ for (File dir : ledgerDirs) {
{code}
I think, it may be good to have prechecks to ensure LedgerDirs & JournalDir are
configured, otherwise it may endup with NPEs right?
{code}
private int bkRecovery(BookKeeperAdmin bkAdmin, String[] args)
throws Exception {
{code}
I think here throws clause should not be a generic exception. Lets throw only
specific ones.
{code}
ZooKeeper zkc = new ZooKeeper(conf.getZkServers(), conf.getZkTimeout(),
null);
BookKeeper bkc = null;
{code}
I think we already has Zk creation code in admin constructor, which handle the
case of connection establishment timeouts.
Otherwise we may get connectionloss exceptions right. Let's reuse the similar
stuff.
{code}
try {
factoryClass = conf.getLedgerManagerFactoryClass();
} catch (Exception e) {
throw new IOException("Failed to get ledger manager factory class
from configuration : ", e);
}
{code}
I don't see a reason why you handled generic Exception here?
in createNewLMFactory
getLedgerManagerType is deprecated instead, please use
getLedgerManagerFactoryClass
{code}
try {
lmFactory = ReflectionUtils.newInstance(factoryClass);
} catch (Throwable t) {
throw new IOException(
"Fail to instantiate ledger manager factory class : "
+ factoryClass, t);
}
{code}
I feel unnecessary handling.
To be clean, remove dead store variables from the test
BTW, good work Vinay and great efforts from Sijie to get into this shape
> Create Bookie format command
> ----------------------------
>
> Key: BOOKKEEPER-300
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-300
> Project: Bookkeeper
> Issue Type: New Feature
> Components: bookkeeper-server
> Affects Versions: 4.1.0
> Reporter: Rakesh R
> Assignee: Vinay
> Fix For: 4.2.0
>
> Attachments: BOOKKEEPER-300.patch, BOOKKEEPER-300.patch,
> BOOKKEEPER-300.patch, BOOKKEEPER-300.patch, BOOKKEEPER-300.patch
>
>
> Provide a bookie format command. Then the admin would just have to run the
> command on each machine, which will prepare the bookie env
> +Zookeeper paths (znodes):+
> - ledger's root path
> - bookie's available path
> +Directories:+
> - Journal directories
> - Ledger directories
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
