[
https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13438503#comment-13438503
]
Vinay commented on BOOKKEEPER-300:
----------------------------------
{quote}for layoutVersion 3, should not have instanceId. if there is instanceId,
it would needs to throw InvalidCookieException.{quote}
of-course, for layoutVersion 3 instanceId will not be present, because it will
be added only in layoutVersion 4 (current patch) onwards. So no need to check
for it instanceId in version 3.
instanceId must be present for further versions.
{quote}And you don't check c.instanceId is null or not before calling equals.
you'd better change it to 'instanceId.equals(c.instanceId)'{quote}
I got this, will update in next patch.
{quote}If an invalid cookie contains a different instance id, it would provide
more detail information in the InvalidCookieException.
so users could know how to do it when encountering it.
BTW, it would be better to provide test cases in CookieTest for different
layout versions and compatibility.{quote}
Yeah.. I was also thinking same. I will add this information.
{quote}the format logic doesn't work correctly for HierarchicalLedgerManager.
actually you could use the asyncProcessLedgers to delete the ledgers. For
HierarchicalLedgerManagers, you also need to delete hierarchical znodes.
{quote}
Ok will check this.
{quote}but I am doubting why not use ZKUtils.deleteRecusive?{quote}
In the first patch, I have used the same to delete all znodes under /ledgers.
After that we had discussion and comes with an interface in
LedgerManager.format().
I couldn't see any usage of providing LedgerManager.format() if we are directly
deleting through 'ZKUtils.deleteRecusive()', since there is no separation
between Flat and Hierarchial.
You're trying to suggest me to still have LedgerManager.format() and in the
abstract format should use 'ZKUtils.deleteRecusive()'..?
{quote}I think it should only have one format method for per ledger manager
factory. the format method takes the responsibility of removing ledgers
metadata (not only ledgers but also znode for id generation and hierarchical
znodes) and its layout data.{quote}
You mean to say, implement complete format() in 'ledger manager factory' itself
instead of implementing under 'ledger manager'..?
{quote}The format is not only delete old ledger metadatas, but it also needs to
create necessary znodes for new system. you missed it in this patch.{quote}
These will be automatically created when
{{LedgerManagerFactory.newLedgerManagerFactory(..)}} is called. Do you think we
need to call this after format..?
{quote}Actually there is a tool class BookKeeperTools in util package for
bookie recovery which also uses BKAdmin. I am guessing it would be better to
leverage it to support different admin commands like what BookieShell does for
bookie side.{quote}
I think, we can add one more command in BookieShell as "bkadmin", which can
take the arguments "recover" or "formatZk" and corresponding additional
arguments if required. In that case BookKeeperTools may not be required anymore.
{quote}BTW, for Bookie format, it would better to put it in BookieShell to put
all bookie commands together{quote}
I will check this.
> 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
> Attachments: 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:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
