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 <mb...@apache.org> Sonar-Qube: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> BAD: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Integration-Tests: Jenkins <jenk...@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 <bamou...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>