hadoop-zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Patrick Hunt (JIRA)" <j...@apache.org>
Subject [jira] Commented: (ZOOKEEPER-872) Small fixes to PurgeTxnLog
Date Mon, 01 Nov 2010 18:48:24 GMT

    [ https://issues.apache.org/jira/browse/ZOOKEEPER-872?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12927082#action_12927082
] 

Patrick Hunt commented on ZOOKEEPER-872:
----------------------------------------

Hi Vishal, I noticed a couple issues.

This class is a command line utility. As such we are outputting to both the command line and
to the log. The usage() in particular should go to std out so that the user will see it regardless
of the log settings (fine if you want to output it to LOG as well, but I think this is unnecessary).

good catch on the error handling for this:

     public static void purge(File dataDir, File snapDir, int num) throws IOException {
-        if (num < 3) {
-            throw new IllegalArgumentException("count should be greater than 3");
+        if (num < 2) {
+            throw new IllegalArgumentException("count should be greater than 1");
         }

However the number 3 was chosen to ensure that ppl don't shoot themselves in the foot (if
the most recent logs get corrupted we'll fall back to the prior when attempting to recover).
There really should be a comment to this effect (would be good to add). I don't know how Mahadev
feels on this setting (min 3 vs some other number) but he might have more insight as IIRC
he implemented this originally.

this following is there to provide feedback to the user when running on command line:

-            System.out.println("Removing file: "+
-                DateFormat.getDateTimeInstance().format(f.lastModified())+
-                "\t"+f.getPath());

again, regardless of logging setup.

Perhaps we should have a "-q" option that turns off the CLI logging and just logs to the log
file? I know this has been an issue previously (stdout/err) given that cron will spitout emails
by default containing stdout/err.

Also, is there a test for this? If you're up to it would be great to add.


> Small fixes to PurgeTxnLog 
> ---------------------------
>
>                 Key: ZOOKEEPER-872
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-872
>             Project: Zookeeper
>          Issue Type: Bug
>    Affects Versions: 3.3.1
>            Reporter: Vishal K
>            Assignee: Vishal K
>            Priority: Minor
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-872
>
>
> PurgeTxnLog forces us to have at least 2 backups (by having count >= 3. Also, it prints
to stdout instead of using Logger.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message