asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdullah alamoudi (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Detect IO errors before NullPointerException
Date Mon, 13 Mar 2017 22:33:56 GMT
abdullah alamoudi has submitted this change and it was merged.

Change subject: Detect IO errors before NullPointerException
......................................................................


Detect IO errors before NullPointerException

Change-Id: I808b12590791a17b749084d1e85f34b9c4ac5893
Reviewed-on: https://asterix-gerrit.ics.uci.edu/1572
Reviewed-by: Michael Blow <mblow@apache.org>
Sonar-Qube: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
BAD: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
---
M hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java
M hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTree.java
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/pom.xml
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexFileManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndexFileManager.java
6 files changed, 65 insertions(+), 33 deletions(-)

Approvals:
  Michael Blow: Looks good to me, approved
  Jenkins: Verified; No violations found; No violations found; Verified



diff --git a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java
b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java
index 401103b..a301d7c 100644
--- a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java
+++ b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java
@@ -60,6 +60,11 @@
     public static final int NO_RESULTSET = 24;
     public static final int JOB_CANCELED = 25;
     public static final int NODE_FAILED = 26;
+    public static final int FILE_IS_NOT_DIRECTORY = 27;
+    public static final int CANNOT_READ_FILE = 28;
+    public static final int UNIDENTIFIED_IO_ERROR_READING_FILE = 29;
+    public static final int FILE_DOES_NOT_EXISTS = 30;
+    public static final int UNIDENTIFIED_IO_ERROR_DELETING_DIR = 31;
 
     // Compilation error codes.
     public static final int RULECOLLECTION_NOT_INSTANCE_OF_LIST = 10001;
diff --git a/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
b/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
index 12601fb..61b30af 100644
--- a/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
+++ b/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
@@ -45,5 +45,10 @@
 24 = No result set for job %1$s
 25 = Job %1$s has been cancelled by a user
 26 = Node %1$s failed
+27 = File %1$s is not a directory
+28 = User doesn't have read permissions on the file %1$s
+29 = Unidentified IO error occurred while reading the file %1$s
+30 = File %1$s doesn't exists
+31 = Unidentified IO error occurred while deleting the dir %1$s
 
 10000 = The given rule collection %1$s is not an instance of the List class.
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTree.java
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTree.java
index 53b8405..b78de8b 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTree.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTree.java
@@ -57,10 +57,10 @@
 import org.apache.hyracks.storage.am.common.ophelpers.MultiComparator;
 import org.apache.hyracks.storage.am.common.tuples.PermutingTupleReference;
 import org.apache.hyracks.storage.am.lsm.btree.tuples.LSMBTreeTupleReference;
-import org.apache.hyracks.storage.am.lsm.common.api.ILSMDiskComponent;
 import org.apache.hyracks.storage.am.lsm.common.api.ILSMComponent;
 import org.apache.hyracks.storage.am.lsm.common.api.ILSMComponentFilterFactory;
 import org.apache.hyracks.storage.am.lsm.common.api.ILSMComponentFilterFrameFactory;
+import org.apache.hyracks.storage.am.lsm.common.api.ILSMDiskComponent;
 import org.apache.hyracks.storage.am.lsm.common.api.ILSMHarness;
 import org.apache.hyracks.storage.am.lsm.common.api.ILSMIOOperation;
 import org.apache.hyracks.storage.am.lsm.common.api.ILSMIOOperationCallback;
@@ -163,7 +163,7 @@
         if (isActivated) {
             throw new HyracksDataException("Failed to create the index since it is activated.");
         }
-
+        // Why delete is part of the create??
         fileManager.deleteDirs();
         fileManager.createDirs();
         diskComponents.clear();
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/pom.xml b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/pom.xml
index ebfb6bd..05c2927 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/pom.xml
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/pom.xml
@@ -16,7 +16,8 @@
  ! specific language governing permissions and limitations
  ! under the License.
  !-->
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
   <modelVersion>4.0.0</modelVersion>
   <artifactId>hyracks-storage-am-lsm-common</artifactId>
   <parent>
@@ -80,5 +81,9 @@
       <groupId>org.apache.commons</groupId>
       <artifactId>commons-lang3</artifactId>
     </dependency>
+    <dependency>
+      <groupId>commons-io</groupId>
+      <artifactId>commons-io</artifactId>
+    </dependency>
   </dependencies>
 </project>
\ No newline at end of file
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexFileManager.java
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexFileManager.java
index a464d96..ce31e2e 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexFileManager.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexFileManager.java
@@ -37,7 +37,7 @@
 public interface ILSMIndexFileManager {
     public void createDirs();
 
-    public void deleteDirs();
+    public void deleteDirs() throws HyracksDataException;
 
     public LSMComponentFileReferences getRelFlushFileReference() throws HyracksDataException;
 
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndexFileManager.java
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndexFileManager.java
index 731d312..36c0866 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndexFileManager.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndexFileManager.java
@@ -33,6 +33,8 @@
 import java.util.HashSet;
 import java.util.List;
 
+import org.apache.commons.io.FileUtils;
+import org.apache.hyracks.api.exceptions.ErrorCode;
 import org.apache.hyracks.api.exceptions.HyracksDataException;
 import org.apache.hyracks.api.io.FileReference;
 import org.apache.hyracks.api.io.IIOManager;
@@ -70,7 +72,7 @@
     private String prevTimestamp = null;
 
     public AbstractLSMIndexFileManager(IIOManager ioManager, IFileMapProvider fileMapProvider,
FileReference file,
-                                       TreeIndexFactory<? extends ITreeIndex> treeFactory)
{
+            TreeIndexFactory<? extends ITreeIndex> treeFactory) {
         this.ioManager = ioManager;
         this.baseDir = file.getFile().getAbsolutePath();
         if (!baseDir.endsWith(System.getProperty("file.separator"))) {
@@ -96,8 +98,8 @@
                 return TreeIndexState.INVALID;
             }
             ITreeIndexMetadataFrame metadataFrame = treeIndex.getPageManager().createMetadataFrame();
-            ICachedPage page = bufferCache.pin(BufferedFileHandle.getDiskPageId(treeIndex.getFileId(),
metadataPage),
-                    false);
+            ICachedPage page =
+                    bufferCache.pin(BufferedFileHandle.getDiskPageId(treeIndex.getFileId(),
metadataPage), false);
             page.acquireReadLatch();
             try {
                 metadataFrame.setPage(page);
@@ -118,11 +120,10 @@
     }
 
     protected void cleanupAndGetValidFilesInternal(FilenameFilter filter,
-                                                   TreeIndexFactory<? extends ITreeIndex>
treeFactory,
-                                                   ArrayList<ComparableFileName> allFiles)
+            TreeIndexFactory<? extends ITreeIndex> treeFactory, ArrayList<ComparableFileName>
allFiles)
             throws HyracksDataException, IndexException {
+        String[] files = listDirFiles(baseDir, filter);
         File dir = new File(baseDir);
-        String[] files = dir.list(filter);
         for (String fileName : files) {
             FileReference fileRef = ioManager.resolveAbsolutePath(dir.getPath() + File.separator
+ fileName);
             if (treeFactory == null) {
@@ -138,10 +139,28 @@
         }
     }
 
+    static String[] listDirFiles(String path, FilenameFilter filter) throws HyracksDataException
{
+        File dir = new File(path);
+        /*
+         * Returns null if this abstract pathname does not denote a directory, or if an I/O
error occurs.
+         */
+        String[] files = dir.list(filter);
+        if (files == null) {
+            if (!dir.canRead()) {
+                throw HyracksDataException.create(ErrorCode.CANNOT_READ_FILE, path);
+            } else if (!dir.exists()) {
+                throw HyracksDataException.create(ErrorCode.FILE_DOES_NOT_EXISTS, path);
+            } else if (!dir.isDirectory()) {
+                throw HyracksDataException.create(ErrorCode.FILE_IS_NOT_DIRECTORY, path);
+            }
+            throw HyracksDataException.create(ErrorCode.UNIDENTIFIED_IO_ERROR_READING_FILE,
path);
+        }
+        return files;
+    }
+
     protected void validateFiles(HashSet<String> groundTruth, ArrayList<ComparableFileName>
validFiles,
-                                 FilenameFilter filter,
-                                 TreeIndexFactory<? extends ITreeIndex> treeFactory
-    ) throws HyracksDataException, IndexException {
+            FilenameFilter filter, TreeIndexFactory<? extends ITreeIndex> treeFactory)
+            throws HyracksDataException, IndexException {
         ArrayList<ComparableFileName> tmpAllInvListsFiles = new ArrayList<>();
         cleanupAndGetValidFilesInternal(filter, treeFactory, tmpAllInvListsFiles);
         for (ComparableFileName cmpFileName : tmpAllInvListsFiles) {
@@ -163,18 +182,17 @@
     }
 
     @Override
-    public void deleteDirs() {
+    public void deleteDirs() throws HyracksDataException {
         File f = new File(baseDir);
-        delete(f);
+        if (f.exists()) {
+            delete(f);
+        }
     }
 
-    private void delete(File f) {
-        if (f.isDirectory()) {
-            for (File c : f.listFiles()) {
-                delete(c);
-            }
+    private void delete(File f) throws HyracksDataException {
+        if (!FileUtils.deleteQuietly(f)) {
+            throw HyracksDataException.create(ErrorCode.UNIDENTIFIED_IO_ERROR_DELETING_DIR,
f.getPath());
         }
-        f.delete();
     }
 
     protected static FilenameFilter bloomFilterFilter = new FilenameFilter() {
@@ -205,8 +223,8 @@
         String[] firstTimestampRange = firstFileName.split(SPLIT_STRING);
         String[] lastTimestampRange = lastFileName.split(SPLIT_STRING);
         // Get the range of timestamps by taking the earliest and the latest timestamps
-        return new LSMComponentFileReferences(createMergeFile(baseDir + firstTimestampRange[0]
+ SPLIT_STRING
-                + lastTimestampRange[1]), null, null);
+        return new LSMComponentFileReferences(
+                createMergeFile(baseDir + firstTimestampRange[0] + SPLIT_STRING + lastTimestampRange[1]),
null, null);
     }
 
     @Override
@@ -291,8 +309,8 @@
 
     @Override
     public void recoverTransaction() throws HyracksDataException {
+        String[] files = listDirFiles(baseDir, transactionFileNameFilter);
         File dir = new File(baseDir);
-        String[] files = dir.list(transactionFileNameFilter);
         try {
             if (files.length == 0) {
                 // Do nothing
@@ -345,16 +363,16 @@
     // This function is used to delete transaction files for aborted transactions
     @Override
     public void deleteTransactionFiles() throws HyracksDataException {
-        File dir = new File(baseDir);
-        String[] files = dir.list(transactionFileNameFilter);
+        String[] files = listDirFiles(baseDir, transactionFileNameFilter);
         if (files.length == 0) {
             // Do nothing
         } else if (files.length > 1) {
             throw new HyracksDataException("Found more than one transaction");
         } else {
+            File dir = new File(baseDir);
             //create transaction filter
             FilenameFilter transactionFilter = createTransactionFilter(files[0], true);
-            String[] componentsFiles = dir.list(transactionFilter);
+            String[] componentsFiles = listDirFiles(baseDir, transactionFilter);
             for (String fileName : componentsFiles) {
                 try {
                     String absFileName = dir.getPath() + File.separator + fileName;
@@ -398,8 +416,8 @@
     };
 
     protected static FilenameFilter createTransactionFilter(String transactionFileName, final
boolean inclusive) {
-        final String timeStamp = transactionFileName.substring(transactionFileName.indexOf(TRANSACTION_PREFIX)
-                + TRANSACTION_PREFIX.length());
+        final String timeStamp = transactionFileName
+                .substring(transactionFileName.indexOf(TRANSACTION_PREFIX) + TRANSACTION_PREFIX.length());
         return new FilenameFilter() {
             @Override
             public boolean accept(File dir, String name) {
@@ -412,9 +430,8 @@
         };
     }
 
-    protected FilenameFilter getTransactionFileFilter(boolean inclusive) {
-        File dir = new File(baseDir);
-        String[] files = dir.list(transactionFileNameFilter);
+    protected FilenameFilter getTransactionFileFilter(boolean inclusive) throws HyracksDataException
{
+        String[] files = listDirFiles(baseDir, transactionFileNameFilter);
         if (files.length == 0) {
             return dummyFilter;
         } else {
@@ -433,7 +450,7 @@
 
     /**
      * @return The string format of the current timestamp.
-     * The returned results of this method are guaranteed to not have duplicates.
+     *         The returned results of this method are guaranteed to not have duplicates.
      */
     protected String getCurrentTimestamp() {
         Date date = new Date();

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1572
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I808b12590791a17b749084d1e85f34b9c4ac5893
Gerrit-PatchSet: 7
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mblow@apache.org>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>

Mime
View raw message