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