You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by "Rakesh R (JIRA)" <ji...@apache.org> on 2012/06/19 08:30:43 UTC

[jira] [Created] (BOOKKEEPER-300) Create Bookie format command

Rakesh R created BOOKKEEPER-300:
-----------------------------------

             Summary: 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.0.0
            Reporter: Rakesh R


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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13447607#comment-13447607 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

Hi Uma,
Thanks for the Review and Suggestions for the optimized and good looking code.

BOOKKEEPER-388 has been raised to document the usage of bookie format command.
                
> 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13439639#comment-13439639 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

Thanks Sijie. I got it. Just need to make sure in any case instanceId should be same whether null or has some value.

I will make necessary changes and post a new patch with all mentioned changes.
                
> 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13440315#comment-13440315 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

thanks Vinay. the patch looks good. I have several minor comments, after that I think it is ready to be in. please check as below:


{code}
+    public static boolean format(ServerConfiguration conf,
+            boolean isInteractive, boolean force) {
+        File journalDir = conf.getJournalDir();

+        File[] ledgerDirs = conf.getLedgerDirs();
+        for (File dir : ledgerDirs) {
{code}

It is not a good idea to remove journalDir and ledgerDirs directly. since at most of cases, journalDir and ledgerDirs might be mount points for separated disks. It would be better to just remove files under these directories. FileSystemUpgrade is the example you could refer.

{code}
+        // In case of format we will create BK instance later
+        if (!conf.getBoolean("bookkeeper.format", false)) {
+            // Create the BookKeeper client instance
+            bkc = new BookKeeper(conf, zk);
+            this.lfr = new LedgerFragmentReplicator(bkc);
+        }
{code}
I don't think you need such a variable, since you would construct a bookkeeper instance again. so why not keep it clean, so we don't have two much branch conditions and additional setting.

{code}
+        } else {
+            zk.create(conf.getZkLedgersRootPath(), "".getBytes(),
+                    Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+        }
+
+        // Create available path if not exists
+        try {
+            zk.create(conf.getZkAvailableBookiesPath(), "".getBytes(),
+                    Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+        } catch (KeeperException.NodeExistsException e) {
+            LOG.info("available node already exists.");
+        }
+
{code}

seems that these two lines are doing same thing. why not make it easy. if there is an old instance, we clean it first, then fall into same path to create it.

{code}
+        // Recreate the BookKeeper Client to recreate the Ledger Layout
+        // information
+        if (null != bkc) {
+            bkc.close();
+        }
+        bkc = new BookKeeper(this.conf, zk);
{code}

I think the ledger layout could be created when calling LedgerManagerFactory#format. so all ledger manager related meta are handled by ledger manage factory itself, not scatter the code over files.
BookKeeperAdmin takes the responsibility of removing cookies and instanceid.

{code}
+        HierarchicalLedgerManager ledgerManager = (HierarchicalLedgerManager) newLedgerManager();
+        String ledgersRootPath = conf.getZkLedgersRootPath();
+        List<String> children = zk.getChildren(ledgersRootPath, false);
+        for (String child : children) {
+            if (ledgerManager.isSpecialZnode(child)) {
+                continue;
+            }
+            ZKUtil.deleteRecursive(zk, ledgersRootPath + "/" + child);
+        }
{code}

You need to delete idgen znode also.

{code}
+        BKAdminCmd() {
+            super(CMD_BKADMIN);
+            bkadminOpts.addOption("r", "recover", true,
+                    "Recover the bookie failure. <arg> refers to <bookieSrc> and [bookieDest], where [bookieDest] is optional");
+            bkadminOpts.addOption("fzk", "formatZk", false,
+                    "Format the bookkeeper metadata from zookeeper");
+        }
{code}

since recover and 'formatZk' would be never used together. so why not just let them be two separated shell commands, one is 'formatZk', the other one is 'recover', which makes the shell clear?

{quote}
+ * Bookie Shell to read/check bookie files and execute admin commands.
{quote}

How about 'Bookie Shell is to provide utilities for users to admin a bookkeeper cluster.'?

>> tests

It would be better to test an old version cookie to access new formatted cluster. You could add it in TestBackwardCompat, which uses an old version bookie.

                
> 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, 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

        

[jira] [Updated] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vinay updated BOOKKEEPER-300:
-----------------------------

    Attachment: BOOKKEEPER-300.patch

Attaching the patch with for initial review,
With,
# Cookies being serialized using protobuf,
# Metadata Format using BKAdmin,
# bookie format in bookie server
# LedgerManager with format api

                
> 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13442444#comment-13442444 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

{code}
But, any way if the bookie started with old version cookie and old version code itself, then we cannot avoid it accessing the cluster. Right..?
{code}

yes. we could do nothing with this case. admin guys need to take care of it. It might be better to add notes writing the documentation for format command. but it would be in another jira to do it.

the patch looks good to me. +1.
                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Uma Maheswara Rao G (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13448553#comment-13448553 ] 

Uma Maheswara Rao G commented on BOOKKEEPER-300:
------------------------------------------------

{code}
 if (null != watcher) {
+                    watcher.process(event);
+                }
{code}
still you are taking watcher from outside. is it necessary?
                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13439429#comment-13439429 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

{quote}
If cookie is of previous version, and Running Code is of version 4, then we should support that cookie because of backward compatibility right..?
If we add strict version check, then backward compatibility will be broken. Am I missing something..?
{quote}

I don't think it is compatibility. Compatibility means your new code could read old cookie format and do the right thing.

if there is instanceid existed in ZooKeeper, which means it is format using bookie.format, so all the old data has been removed. if you using an old cookie (version 3) to access a new instance (same as you use a old instance id access a new instance id), why not fail it?
                
> 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13437896#comment-13437896 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

@Vinay, thanks for the patch. please check my comments as below:

{quote}
+        && (c.layoutVersion == 3 || (instanceId != null && c.instanceId
+                .equals(instanceId))))) {
{quote}

for layoutVersion 3, should not have instanceId. if there is instanceId, it would needs to throw InvalidCookieException.
And you don't check c.instanceId is null or not before calling equals. you'd better change it to 'instanceId.equals(c.instanceId)'

For layoutVersion 4, should we fail it if there is no instanceId?

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}
+    private static void loadConfFile(AbstractConfiguration conf, String confFile)
+            throws IllegalArgumentException {
+            try {
+                conf.loadConf(new File(confFile).toURI().toURL());
+            } catch (MalformedURLException e) {
+                LOG.error("Could not open configuration file: " + confFile, e);
+                throw new IllegalArgumentException();
+            } catch (ConfigurationException e) {
+                LOG.error("Malformed configuration file: " + confFile, e);
+                throw new IllegalArgumentException();
+            }
+            LOG.info("Using configuration file " + confFile);
+        }
{quote}

the indent is not right.

{quote}
+    public boolean format() {
+        final AtomicInteger successResults = new AtomicInteger();
+        try {
+            Set<Long> zkLedgers = getLedgersInSingleNode(ledgerRootPath);
{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.

but I am doubting why not use ZKUtils.deleteRecusive?

{quote}
+            boolean ledgersFormatted = bkc.ledgerManager.format();
+            if (!ledgersFormatted) {
+                LOG.error("Error in formatting ledgers");
+                return false;
+            }
+            bkc.ledgerManagerFactory.format(conf, zk);
{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. 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}
+    public static void main(String[] args) throws IOException,
+            InterruptedException, KeeperException {
+        AbstractConfiguration conf = null;
+        try {
+            conf = parseArgs(args);
+        } catch (ParseException e) {
+            LOG.error("Error parsing command line arguments : ", e);
+            System.err.println(e.getMessage());
+            printUsage();
+            System.exit(ExitCode.INVALID_CONF);
+        }
{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. BTW, for Bookie format, it would better to put it in BookieShell to put all bookie commands together.




 
                
> 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

        

[jira] [Updated] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vinay updated BOOKKEEPER-300:
-----------------------------

    Attachment: BOOKKEEPER-300.patch

Attaching the updated patch with all comment fixes. Please review
                
> 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, 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Uma Maheswara Rao G (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13413880#comment-13413880 ] 

Uma Maheswara Rao G commented on BOOKKEEPER-300:
------------------------------------------------

Thanks a lot, Vinay for the patch.

{quote}
# Create the JournalDir and ledger dirs if not exists.
# If Journal directories exist and its not empty, then it will ask for the user confirmation. On Confirmation it will format all contents of Journal directories and ledger directories, and recreate the directories.

    * Here confirmation will be asked only for the journal directory. If journal directory is empty or not exists, it will format ledger directory contents even though its not empty. Because Bookie will never have non-empty ledger directory with empty journal directory.
{quote}

I think format should just take care of metadata. Storage directories anyway will be cleaned if there is no metadata presents related to that data.

Going to each bookie and executing format command may be a pain for the admins when we have more BookKeeper nodes in the cluster ( as it is scalable).
In any BookKeeper node, if admin execute format command, it should just clean metadata of BookKeeper Server by prompting for the confirmation like how you did now.

What about others opinion?


And other requirement for format from NameNode is:
 JournalManager implementation should support formatting of its respective storage. Currently NameNode is just skipping the sharedstorage format as there is no interface defined yet in JournalManager. Aaron and me Discussed some time back to provide that interface to support format. So, soon we will add that in JournalManager interface.

So, when namenode calls format, it will call the JournalManager's format implementation. In the BookKeeper shared storage case, I think we should have the implementation in BookKeeperJournalManager class. It should be like, check the metadata in ZK and format it.
This part of implementation should go in BKJM. 

Whether BookKeeper client will expose API for format and BKJM will just call it? 
(or)

 let BKJM handle it completely as BKJM already holding the zk instance? 

We can file the separate JIRA for this case based on the opinions from others.

Thanks
Uma

                
> 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.0.0
>            Reporter: Rakesh R
>         Attachments: 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Uma Maheswara Rao G (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13414475#comment-13414475 ] 

Uma Maheswara Rao G commented on BOOKKEEPER-300:
------------------------------------------------

Check the similar discussion about DN format in HDFS. HDFS-107. 
If you see, DN also can not start, if you format namenode due to incompatible namespace IDs. In that case, Admins carefully, have to clean their storage directories.
We have decided that just have to provide the automation script to delete the storage directories. 

Now, by looking at your patch, you are doing both the jobs in format. Actually metadata formation is not required to do in all the nodes. That will be just one time job if I understand your idea correctly. In one node, you will create metadata directories and from other nodes again you will recreate them and continue the same in remaining nodes? 

Do we need to separate this jobs cleanly? Otherwise someone need to give the metadata format confirmation for first node and should skip for remaining. This may not quit correct right?

{quote}
leaving other nodes ( cookies, LAYOOUT ) as it is
{quote}
Sijie can give thoughts on this. 

I guess LAYOUT files may contain some version related stuff as well. So, take a case, I have formatted the cluster and installed the newer version of BKs and trying to start the cluster. Then we may get incompatible format issues because LAYOUT may have older version related stuff right? Since I have formatted cluster, I should not have any active info which is related to older. 

Hi All, could you also please participate in the discussion for better ideas?
                
> 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
>
>
> 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

        

[jira] [Updated] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vinay updated BOOKKEEPER-300:
-----------------------------

    Affects Version/s:     (was: 4.0.0)
                       4.1.0
    
> 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
>
>
> 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13415129#comment-13415129 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

@Vinay, 

>> Cookie

It would be better to encode Cookie in protobuf for future extension. And you need to resolve original cookie format and protobuf encoded format for backward compatibility. (may be you could refer the changes on LedgerMetadata in BOOKKEEPER-303)

And for my proposal, there is two changes need for bookie format. I think it would be two subtasks to handle it, which make things clearly. one is instance-id and cookie format improvement, the other one is providing format interface in LedgerManager.

but before implemented it, I think it would be better to see others' comments first. 

@Flavio, could you take a look at this jira?
                
> 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
>
>
> 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13439432#comment-13439432 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

{quote}
BookieShell javadoc is saying "Bookie Shell to read/check bookie files."
Also, one more I've observed is, presently the commands in the shell are like: readledger, readlog, readjournal etc. I feel, all these are having readonly behaviours. But the format command is a kind of write permission. Are we designed the BookieShell to keep only readonly cmds?
{quote}

BookieShell is added for BookkKeeper, which is similar as HedwigConsole added for Hedwig. It is designed to put utilities for admin guys to operate/admin the cluster. I think it is OK to put utilities for administration in it.

I don't have strong feel on it. But I think putting utilities for administration together would help maintenance. 
                
> 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

        

[jira] [Updated] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vinay updated BOOKKEEPER-300:
-----------------------------

    Attachment: BOOKKEEPER-300.patch

Attaching the patch for bookkeeper format

Following command should be used to execute the format
{code}bookkeeper-server/bin/bookkeeper bookie --format{code}

Following are the behaviours of the format.
# Create the JournalDir and ledger dirs if not exists.
# If Journal directories exist and its not empty, then it will ask for the user confirmation. On Confirmation it will format all contents of Journal directories and ledger directories, and recreate the directories.
** Here confirmation will be asked only for the journal directory. If journal directory is empty or not exists, it will format ledger directory contents even though its not empty. Because Bookie will never have non-empty ledger directory with empty journal directory.
# Now it will go ahead for formatting the zookeeper nodes. If /ledgers (zk root path) exists then it will ask for confirmation, otherwise it will just simply create /ledgers and /ledgers/available nodes. On confirmation it will delete all nodes present under /ledgers and re-create /ledgers and /ledgers/available nodes.
                
> 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.0.0
>            Reporter: Rakesh R
>         Attachments: 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13450373#comment-13450373 ] 

Hudson commented on BOOKKEEPER-300:
-----------------------------------

Integrated in bookkeeper-trunk #695 (See [https://builds.apache.org/job/bookkeeper-trunk/695/])
    BOOKKEEPER-300: Create Bookie format command (Vinay via sijie) (Revision 1381870)

     Result = UNSTABLE
sijie : 
Files : 
* /zookeeper/bookkeeper/trunk/CHANGES.txt
* /zookeeper/bookkeeper/trunk/bookkeeper-server/bin/bookkeeper
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieException.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManagerFactory.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManagerFactory.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerLayout.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManagerFactory.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/DataFormats.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/IOUtils.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/proto/DataFormats.proto
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/TestLedgerUnderreplicationManager.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/TestReplicationWorker.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/TestBackwardCompat.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java

                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13452915#comment-13452915 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

Thanks, Sijie for the review and commit, also Uma for the review.
                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Uma Maheswara Rao G (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13447580#comment-13447580 ] 

Uma Maheswara Rao G commented on BOOKKEEPER-300:
------------------------------------------------

Thank a lot, Vinay for the patch.
Please give me some time, I am also reviewing this patch, if I can find some points.
                
> 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13447620#comment-13447620 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

{quote}
in createNewLMFactory
getLedgerManagerType is deprecated instead, please use getLedgerManagerFactoryClass
{quote}

@Uma, please note the getLedgerManagerType is used for backward compatibility. so it would be better to keep it there.  The caller of createNewLMFactory has called getLedgerManagerFactoryClass before.
                
> 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Uma Maheswara Rao G (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13448740#comment-13448740 ] 

Uma Maheswara Rao G commented on BOOKKEEPER-300:
------------------------------------------------

@Vinay,
some minor stuff:
 looks like you forgot to change to new method in ZookeeperUtil.
{code}
ZkUtils.getNewZookeeperClient
{code}

I think we can remove that method completely in ZookeeperUtil right? you can make use of it from ZKUtils dierectly?

Other than that Patch looks great. Good work Vinay.
I think once you change this, it is almost ready to go.

+1 on addressing the above comments.
                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13448615#comment-13448615 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

I don't like the style to pass a watcher in #getNewZookeeperClient. Actually ZooKeeper has provided #register method to overwrite with a new watcher. I would prefer to keep an utility function  simpler. How about let ZkUtil#getNewZookeeperClient just takes the responsibility of returning a connected zookeeper instance? use a different name like 'createConnectedZooKeeperClient'?
                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Rakesh R (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13439272#comment-13439272 ] 

Rakesh R commented on BOOKKEEPER-300:
-------------------------------------

@Sijie
bq.BTW, for Bookie format, it would better to put it in BookieShell to put all bookie commands together.

Thanks Sijie for the suggestion. I'm having following doubts about the BookieShell, would be great if you give more on this.
- BookieShell javadoc is saying "Bookie Shell to read/check bookie files."
- Also, one more I've observed is, presently the commands in the shell are like: readledger, readlog, readjournal etc. I feel, all these are having readonly behaviours. But the format command is a kind of write permission. Are we designed the BookieShell to keep only readonly cmds?

Is it ok to keep the 'format' command in the shell. Any thoughts? If yes, it would be good to look at and modify/update the javadocs also while creating the format patch. Thanks
                
> 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13439598#comment-13439598 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

{code}
+            String instanceId = getInstanceId(zk);
             boolean newEnv = false;
             Cookie masterCookie = Cookie.generateCookie(conf);
+            if (null != instanceId) {
+                masterCookie.setInstanceId(instanceId);
+            }
             try {
                 Cookie zkCookie = Cookie.readFromZooKeeper(zk, conf);
                 masterCookie.verify(zkCookie);
...

             journalCookie.verify(masterCookie)
{code}

from the patch, first getInstanceId from zookeeper and set it into masterCookie. then verify zkCookie and journalCookie.

since journalCookie/zkCookie is parsed from existed cookie data (which is version 3), so the checking condition would be:

{code}
// the existed cookie (journalCookie/zkCookie) is old, but the cluster has instanceid (which is new and formatted).
// so you should failed it.
if (c.layoutVersion == 3 && instanceId != null) {
    throw new BookieException.InvalidCookieException(...);
}
// the existed cookie (journalCookie/zkCookie) is new format. to compare instanceid.
if (c.layoutVersion == 4) {
    if ((instanceId == null && c.instanceId != null) ||
        (instanceId != null && !instanceId.equals(c.instanceId))) {
        throw new BookieException.InvalidCookieException(...);
    }
}
{code}

Does it make sense?




                
> 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

        

[jira] [Updated] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vinay updated BOOKKEEPER-300:
-----------------------------

    Attachment: BOOKKEEPER-300.patch

Oops!!! I didn't check it, trusting the eclipse.!! ;) Thanks Uma for finding it out.

Anyway.. Here is the corrected patch.

                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13438729#comment-13438729 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

{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}

What happened if a version 3 cookie access version 4 bookkeeper? the code is used to validate cookie, you need to cover the cases, I assumed.

{quote}
You're trying to suggest me to still have LedgerManager.format() and in the abstract format should use 'ZKUtils.deleteRecusive()'..?
{quote}

you could use asyncProcessLedgers or ZKUtils.deleteRecursive. either is OK.

{quote}
You mean to say, implement complete format() in 'ledger manager factory' itself instead of implementing under 'ledger manager'..?
{quote}

yes.

{quote}
These will be automatically created when LedgerManagerFactory.newLedgerManagerFactory(..) is called. Do you think we need to call this after format..?
{quote}

format means cleaning old data and creating a new one. so I think creating a new one would be needed.

{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}

it is OK.
                
> 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

        

[jira] [Updated] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vinay updated BOOKKEEPER-300:
-----------------------------

    Attachment: BOOKKEEPER-300.patch

Attaching the patch addressing above comments
                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Uma Maheswara Rao G (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13449531#comment-13449531 ] 

Uma Maheswara Rao G commented on BOOKKEEPER-300:
------------------------------------------------

+1 for the patch, thanks Vinay for incorporating all the comments.

@Sijie, could you please take a look and push this in, if you like?
                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13414586#comment-13414586 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

{quote}
I think format should just take care of metadata. Storage directories anyway will be cleaned if there is no metadata presents related to that data.
{quote}
{quote}
If we clean only zookeeper nodes, then bookie itself will not start. In each and every storage directory, cookie file will be present. If that information present in storage dir and not in storage directory, then bookie will not start.
{quote}

I think the bookies would still start because there is no cookie existed in ZooKeeper. bookies would treat it as a new environment and create its cookie node.

but there is an inconsistent issue if we just clean metadata. there is a time period before all old data was cleaned by GC thread. so those data might be introduced in the system again due some corner cases.

so if we want to introduce format here for BookKeeper, we need to introduce a mechanism to distinguish  bookies between current format and previous formats first. the initial idea is to generate an INSTANCE-ID (could be timestamp, uuid) for bookkeeper during formatting. all bookies join this instance of bookkeeper would generate cookie based on INSTANCE-ID and recorded them in local directories. if a bookie server join bookkeeper using different INSTANCE-ID, it should fail to start.

after introducing such mechanism, we could avoid bookie server starts with old data between different bookkeeper format.

so the format progress would contains two parts, one is metadata format, the other one is bookie format.

metadata format: could be run in either bookie server and run only once. it takes the responsibility on cleaning metadata. after cleaning old metadata, it would generate a new INSTANCE-ID for new bookkeeper instance.

bookie format: be run each bookie server. it takes the responsibility on cleaning old data.

if a bookie server doesn't run 'bookie-format' after the metadata is formatted. it should fail to start due to providing different INSTANCE-ID. so admin guys could run 'bookie -format' to clean local bookie data.

{code}
leaving other nodes ( cookies, LAYOOUT ) as it is
{code}

as Uma mentioned, 'format' means there is no old info introduced to new environment. so cookies and LAYOUT should be also removed.

before removing LAYOUT, you should not remove ledgers znode directly. LAYOUT introduced is for different ledger metadata management. in BOOKKEEPER-203, we had a zk-independent LedgerManager interface to handle metadata management for bookkeeper. so a possible way to handle metadata formatting is to provide a 'format' interface in LedgerManager. and different LedgerManager implementation should provide a way to format their metadata.

so the metadata format progress would be:

1) reading LAYOUT to instantiate a LedgerManager.
2) call LedgerManager#format to format its metadata layout.
3) remove LAYOUT znode.
3) remove cookie znodes.

{quote}
Format should be implememted as API in server side or Client side.?
If BKJM also want to call format, which needs to format only ledger details in zk, then it would be better to implement at client side. i.e. in BookKeeper class.
{quote}

for metadata format, it might be better to put it in BookKeeperAdmin. for local data format, you could put it in Bookie.









                
> 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
>
>
> 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13448568#comment-13448568 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

This is required, in future, if someone uses same utility, and wants to handle its own events. For Ex: Bookie may want to handle expired event.
                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13439436#comment-13439436 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

You mean to say, if instanceid present in zookeeper, then we need to do strict checking of version. I agree with this.

Problem may come in case of upgrade cookie version 3 to cookie version 4.
Say current cluster is running with cookie version 3. In this case instanceid will not be present in zookeeper.
Now, without doing format, if the cluster is upgraded to bookie which are having cookie version 4, in this case should allow cookie version 3. Right..?

Please check whether below conditions work for above cases

{code}        if (c.layoutVersion < 3 && c.layoutVersion != layoutVersion) {
            String errMsg = "Cookie is of too old version " + c.layoutVersion;
            LOG.error(errMsg);
            throw new BookieException.InvalidCookieException(errMsg);
        } else if (!(c.layoutVersion >= 3 && c.bookieHost.equals(bookieHost)
                && c.journalDir.equals(journalDir)
                && c.ledgerDirs.equals(ledgerDirs)
        // check instance id only for layoutVersion 4 onwards
        && ((c.layoutVersion == 3 && instanceId == null) || (instanceId != null && instanceId
                .equals(c.instanceId))))) {
            String errMsg = "Cookie [" + this + "] is not matching with [" + c
                    + "]";
            throw new BookieException.InvalidCookieException(errMsg);
        }{code}

                
> 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13415081#comment-13415081 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

One small doubt here. Since we are adding one more field INSTANCEID to Cookie, do we need to change the Cookie layout version.? If yes, to what version.?
Using this we can consider the backward compatibility.
                
> 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
>
>
> 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13414320#comment-13414320 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

Thanks Uma for taking look at this.

{quote}I think format should just take care of metadata. Storage directories anyway will be cleaned if there is no metadata presents related to that data.{quote}
If we clean only zookeeper nodes, then bookie itself will not start. In each and every storage directory, cookie file will be present. If that information present in storage dir and not in storage directory, then bookie will not start.

One thing we can do here. We can delete the only Ledger nodes in zookeeper, leaving other nodes ( cookies, LAYOOUT ) as it is. In this case,  storage directories will be automatically deleted once the metadata for ledgers not available.

{quote}Whether BookKeeper client will expose API for format and BKJM will just call it? {quote}
Regarding this, I think API should be provided in both sides BKJM as well as BookKeeper.
Let BookKeeper take care of formatting the ledger details in zk. i.e. ledgers under /ledgers node.
BKJM can format its own nodes such as inprogress node, edit log segment nodes, etc.

One question I have here.
*Format should be implememted as API in server side or Client side.?*
If BKJM also want to call format, which needs to format only ledger details in zk, then it would be better to implement at client side. i.e. in BookKeeper class.


                
> 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.0.0
>            Reporter: Rakesh R
>         Attachments: 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

        

[jira] [Comment Edited] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13414586#comment-13414586 ] 

Sijie Guo edited comment on BOOKKEEPER-300 at 7/15/12 8:10 AM:
---------------------------------------------------------------

{quote}
I think format should just take care of metadata. Storage directories anyway will be cleaned if there is no metadata presents related to that data.
{quote}
{quote}
If we clean only zookeeper nodes, then bookie itself will not start. In each and every storage directory, cookie file will be present. If that information present in storage dir and not in storage directory, then bookie will not start.
{quote}

I think the bookies would still start because there is no cookie existed in ZooKeeper. bookies would treat it as a new environment and create its cookie node.

but there is an inconsistent issue if we just clean metadata. there is a time period before all old data was cleaned by GC thread. so those data might be introduced in the system again due some corner cases.

so if we want to introduce format here for BookKeeper, we need to introduce a mechanism to distinguish  bookies between current format and previous formats first. the initial idea is to generate an INSTANCE-ID (could be timestamp, uuid) for bookkeeper during formatting. all bookies join this instance of bookkeeper would generate cookie based on INSTANCE-ID and recorded them in local directories. if a bookie server join bookkeeper using different INSTANCE-ID, it should fail to start.

after introducing such mechanism, we could avoid bookie server starts with old data between different bookkeeper format.

so the format progress would contains two parts, one is metadata format, the other one is bookie format.

metadata format: could be run in either bookie server and run only once. it takes the responsibility on cleaning metadata. after cleaning old metadata, it would generate a new INSTANCE-ID for new bookkeeper instance.

bookie format: be run each bookie server. it takes the responsibility on cleaning old data.

if a bookie server doesn't run 'bookie-format' after the metadata is formatted. it should fail to start due to providing different INSTANCE-ID. so admin guys could run 'bookie -format' to clean local bookie data.

{quote}
leaving other nodes ( cookies, LAYOOUT ) as it is
{quote}

as Uma mentioned, 'format' means there is no old info introduced to new environment. so cookies and LAYOUT should be also removed.

before removing LAYOUT, you should not remove ledgers znode directly. LAYOUT introduced is for different ledger metadata management. in BOOKKEEPER-203, we had a zk-independent LedgerManager interface to handle metadata management for bookkeeper. so a possible way to handle metadata formatting is to provide a 'format' interface in LedgerManager. and different LedgerManager implementation should provide a way to format their metadata.

so the metadata format progress would be:

1) reading LAYOUT to instantiate a LedgerManager.
2) call LedgerManager#format to format its metadata layout.
3) remove LAYOUT znode.
3) remove cookie znodes.

{quote}
Format should be implememted as API in server side or Client side.?
If BKJM also want to call format, which needs to format only ledger details in zk, then it would be better to implement at client side. i.e. in BookKeeper class.
{quote}

for metadata format, it might be better to put it in BookKeeperAdmin. for local data format, you could put it in Bookie.









                
      was (Author: hustlmsp):
    {quote}
I think format should just take care of metadata. Storage directories anyway will be cleaned if there is no metadata presents related to that data.
{quote}
{quote}
If we clean only zookeeper nodes, then bookie itself will not start. In each and every storage directory, cookie file will be present. If that information present in storage dir and not in storage directory, then bookie will not start.
{quote}

I think the bookies would still start because there is no cookie existed in ZooKeeper. bookies would treat it as a new environment and create its cookie node.

but there is an inconsistent issue if we just clean metadata. there is a time period before all old data was cleaned by GC thread. so those data might be introduced in the system again due some corner cases.

so if we want to introduce format here for BookKeeper, we need to introduce a mechanism to distinguish  bookies between current format and previous formats first. the initial idea is to generate an INSTANCE-ID (could be timestamp, uuid) for bookkeeper during formatting. all bookies join this instance of bookkeeper would generate cookie based on INSTANCE-ID and recorded them in local directories. if a bookie server join bookkeeper using different INSTANCE-ID, it should fail to start.

after introducing such mechanism, we could avoid bookie server starts with old data between different bookkeeper format.

so the format progress would contains two parts, one is metadata format, the other one is bookie format.

metadata format: could be run in either bookie server and run only once. it takes the responsibility on cleaning metadata. after cleaning old metadata, it would generate a new INSTANCE-ID for new bookkeeper instance.

bookie format: be run each bookie server. it takes the responsibility on cleaning old data.

if a bookie server doesn't run 'bookie-format' after the metadata is formatted. it should fail to start due to providing different INSTANCE-ID. so admin guys could run 'bookie -format' to clean local bookie data.

{code}
leaving other nodes ( cookies, LAYOOUT ) as it is
{code}

as Uma mentioned, 'format' means there is no old info introduced to new environment. so cookies and LAYOUT should be also removed.

before removing LAYOUT, you should not remove ledgers znode directly. LAYOUT introduced is for different ledger metadata management. in BOOKKEEPER-203, we had a zk-independent LedgerManager interface to handle metadata management for bookkeeper. so a possible way to handle metadata formatting is to provide a 'format' interface in LedgerManager. and different LedgerManager implementation should provide a way to format their metadata.

so the metadata format progress would be:

1) reading LAYOUT to instantiate a LedgerManager.
2) call LedgerManager#format to format its metadata layout.
3) remove LAYOUT znode.
3) remove cookie znodes.

{quote}
Format should be implememted as API in server side or Client side.?
If BKJM also want to call format, which needs to format only ledger details in zk, then it would be better to implement at client side. i.e. in BookKeeper class.
{quote}

for metadata format, it might be better to put it in BookKeeperAdmin. for local data format, you could put it in Bookie.









                  
> 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
>
>
> 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13447289#comment-13447289 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

ah, seems the patch has been a bit older. it conflicts with the latest branch, due to BOOKKEEPER-376.

@Vinay, could you help rebase it to latest trunk? After that, I think it is ready to be in. sorry to make you inconvenient.
                
> 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
>
>
> 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

[jira] [Updated] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vinay updated BOOKKEEPER-300:
-----------------------------

    Attachment: BOOKKEEPER-300.patch

Attaching the re-based patch.
                
> 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13447478#comment-13447478 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

Yes Sijie,
I will post the re-based patch in sometime.
                
> 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
>
>
> 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13447624#comment-13447624 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

Hi Uma, here are some of the clarifications on above comments.
{quote}I think, it may be good to have prechecks to ensure LedgerDirs & JournalDir are configured, otherwise it may endup with NPEs right?{quote}
If journalDir and ledgerDirs are not configured, then Configuration will return default values /tmp/bk-txn and /tmp/bk-data. So Null check not required.

{quote}in createNewLMFactory
getLedgerManagerType is deprecated instead, please use getLedgerManagerFactoryClass{quote}
getLedgerManagerFactoryClass(..) also used before calling createNewLMFactory(..). But for backward compatibility,  getLedgerManagerType() is used. This code was present earlier. I just extracted to common method. ;)

Other comments I will fix and post a new patch soon....
                
> 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13414863#comment-13414863 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

Thanks Siji for the detailed information and suggestion. I will look into 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
>
>
> 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

        

[jira] [Updated] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vinay updated BOOKKEEPER-300:
-----------------------------

    Attachment: BOOKKEEPER-300.patch

Attaching one more patch. Removes the unnecessary method in ZookeeperUtil. Directly using createConnectedZkClient(..) instead.
                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13414322#comment-13414322 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

I forgot to mention here..
Intention of this Jira is to create the zk nodes /ledgers and /ledgers/available which will not be done automatically. Anyway creating the journaldir and ledger directories will be done by bookie if not exists.
This should be called by admin before any server starts.


                
> 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
>
>
> 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

        

[jira] [Updated] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vinay updated BOOKKEEPER-300:
-----------------------------

    Attachment: BOOKKEEPER-300.patch

Attaching the patch with comment fixes.
                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13442956#comment-13442956 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

@Sijie, thanks for the review.

{quote}It might be better to add notes writing the documentation for format command. but it would be in another jira to do it.{quote}
Sure, we will add in doc.
                
> 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, 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

[jira] [Updated] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vinay updated BOOKKEEPER-300:
-----------------------------

    Attachment: BOOKKEEPER-300.patch

Attaching the patch addressing Uma's comments.
                
> 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, 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

[jira] [Assigned] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vinay reassigned BOOKKEEPER-300:
--------------------------------

    Assignee: Vinay
    
> 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
>
>
> 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Uma Maheswara Rao G (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13447623#comment-13447623 ] 

Uma Maheswara Rao G commented on BOOKKEEPER-300:
------------------------------------------------

Yes, Sijie. Just seen the comment above to that line in code. Its for BWC. Vinay also discussed the same with me Just few mins back. He is taking care of this in his next patch. Thanks for noticing.
                
> 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Uma Maheswara Rao G (JIRA)" <ji...@apache.org>.
    [ 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Uma Maheswara Rao G (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13448028#comment-13448028 ] 

Uma Maheswara Rao G commented on BOOKKEEPER-300:
------------------------------------------------

Thanks a lot, Vinay for incorporating the comments.
{code}
zk = ZkUtils.getNewZookeeperClient(conf.getZkServers(),
+                conf.getZkTimeout(), new Watcher() {
+                    @Override
+                    public void process(WatchedEvent event) {
+                        if (LOG.isDebugEnabled()) {
+                            LOG.debug("Process: " + event.getType() + " "
+                                    + event.getPath());
+                        }
+                    }
+                });
{code}
You added this watcher just for logging purpose? can't we make use of util watcher alone?
                
> 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, 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

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Vinay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13439405#comment-13439405 ] 

Vinay commented on BOOKKEEPER-300:
----------------------------------

Hi @Sijie,
{quote}What happened if a version 3 cookie access version 4 bookkeeper? {quote}
If cookie is of previous version, and Running Code is of version 4, then we should support that cookie because of backward compatibility right..?
If we add strict version check, then backward compatibility will be broken. Am I missing something..?


Others comments I will try to implement. Thanks
                
> 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Rakesh R (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13396547#comment-13396547 ] 

Rakesh R commented on BOOKKEEPER-300:
-------------------------------------

Format command would be similar as follows:
{code}
bookkeeper-server/bin/bookkeeper bookie -format 
{code}

----

Discussion Thread as follows: 
http://mail-archives.apache.org/mod_mbox/zookeeper-bookkeeper-dev/201206.mbox/%3CC2496325850AA74C92AAF83AA9662D2630DE1B56@szxeml531-mbx.china.huawei.com%3E
                
> 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.0.0
>            Reporter: Rakesh R
>
> 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

        

[jira] [Commented] (BOOKKEEPER-300) Create Bookie format command

Posted by "Sijie Guo (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13439623#comment-13439623 ] 

Sijie Guo commented on BOOKKEEPER-300:
--------------------------------------

ah, I think we don't need to check layoutVersion when comparing instanceId. we just compare them directly, just to ensure the instanceid are same.

{code}
...
    if ((instanceId == null && c.instanceId != null) ||
        (instanceId != null && !instanceId.equals(c.instanceId))) {
        throw new BookieException.InvalidCookieException(...);
    }
...
{code}

                
> 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