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

ASF GitHub Bot commented on ZOOKEEPER-2967:
-------------------------------------------

Github user mfenes commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/450#discussion_r163282090
  
    --- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
    @@ -136,13 +136,51 @@ public FileTxnSnapLog(File dataDir, File snapDir) 
throws IOException {
                 throw new DatadirException("Cannot write to snap directory " + 
this.snapDir);
             }
     
    +        // check content of transaction log and snapshot dirs if they are 
two different directories
    +        if(!this.dataDir.getPath().equals(this.snapDir.getPath())){
    +            checkLogDir();
    +            checkSnapDir();
    +        }
    +
             txnLog = new FileTxnLog(this.dataDir);
             snapLog = new FileSnap(this.snapDir);
     
             autoCreateDB = 
Boolean.parseBoolean(System.getProperty(ZOOKEEPER_DB_AUTOCREATE,
                     ZOOKEEPER_DB_AUTOCREATE_DEFAULT));
         }
     
    +    private void checkLogDir() throws LogdirContentCheckException {
    +        File[] files = this.dataDir.listFiles();
    --- End diff --
    
    If I used FilenameFilter, then Util.isSnapshotFile() / Util.isLogFile() 
check would be run for all the files in the directory and 
listFiles(FilenameFilter filter) would return all the files satisfying the 
filter condition, however I need only the first occurrence which satisfies the 
condition, not all of them. The current logic quits from the for loop 
immediately when it finds a file violating the configuration and throws an 
exception, while your proposal would iterate over all the files in the 
directory and would call Util.isSnapshotFile() / Util.isLogFile() for each of 
the files inside FilenameFilter to prepare the filtered File[]. So using 
FilenameFilter would be a bit slower, but yes, it might need less lines in 
code, also at the price of obscuring the purpose of the code (i.e. 
hasSnapshotFiles / hasLogFiles boolean variables tell what the problem exactly 
is, while if (snapshotFiles.length > 0) { throw new Exception(...) } would 
not). However, if we prefer using Java library classes over standard coding 
patterns even in cases when it does not fit the purpose entirely, then 
FilenameFilter can be the winner.


> Add check to validate dataDir and dataLogDir parameters at startup
> ------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-2967
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
>             Project: ZooKeeper
>          Issue Type: Improvement
>          Components: server
>    Affects Versions: 3.4.11
>            Reporter: Andor Molnar
>            Assignee: Mark Fenes
>            Priority: Major
>              Labels: startup, validation
>             Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to