zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mfenes <...@git.apache.org>
Subject [GitHub] zookeeper pull request #450: ZOOKEEPER-2967: Add check to validate dataDir a...
Date Tue, 23 Jan 2018 15:40:12 GMT
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.


---

Mime
View raw message