abdullah alamoudi has uploaded a new change for review.

  https://asterix-gerrit.ics.uci.edu/2496

Change subject: [ASTERIXDB-2321][STO] Follow the contract in 
LSMBTreeRangeSearchCursor
......................................................................

[ASTERIXDB-2321][STO] Follow the contract in LSMBTreeRangeSearchCursor

- user model changes: no
- storage format changes: no
- interface changes: no

Details:
- The index cursor contract says that an open call which returns
  successfully, leaves the cursor in the open state, otherwise,
  the cursor remains in the closed state.
- The LSMBTreeRangeSearchCursor has many cursors inside. In the
  case where one of the cursors fails to open, and an exception
  is about to be thrown, we must close all previously open cursors
  since the LSMBTreeRangeSearchCursor will be in the closed state
  and close will not be called.

Change-Id: I19db2afd2d6ca4a2ca1056cd95ae504b2be69813
---
M 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeRangeSearchCursor.java
M 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndex.java
A 
hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/util/IndexCursorUtils.java
3 files changed, 93 insertions(+), 14 deletions(-)


  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb 
refs/changes/96/2496/1

diff --git 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeRangeSearchCursor.java
 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeRangeSearchCursor.java
index 0cb7d1c..070b694 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeRangeSearchCursor.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeRangeSearchCursor.java
@@ -42,6 +42,7 @@
 import org.apache.hyracks.storage.common.IIndexCursor;
 import org.apache.hyracks.storage.common.ISearchOperationCallback;
 import org.apache.hyracks.storage.common.ISearchPredicate;
+import org.apache.hyracks.storage.common.util.IndexCursorUtils;
 
 public class LSMBTreeRangeSearchCursor extends LSMIndexSearchCursor {
     private final ArrayTupleReference copyTuple;
@@ -350,22 +351,30 @@
             btreeAccessors = new BTreeAccessor[numBTrees];
         }
 
-        for (int i = 0; i < numBTrees; i++) {
-            ILSMComponent component = operationalComponents.get(i);
-            BTree btree;
-            if (component.getType() == LSMComponentType.MEMORY) {
-                includeMutableComponent = true;
+        int i = 0;
+        try {
+            for (; i < numBTrees; i++) {
+                ILSMComponent component = operationalComponents.get(i);
+                BTree btree;
+                if (component.getType() == LSMComponentType.MEMORY) {
+                    includeMutableComponent = true;
+                }
+                btree = (BTree) component.getIndex();
+                if (btreeAccessors[i] == null) {
+                    btreeAccessors[i] = 
btree.createAccessor(NoOpIndexAccessParameters.INSTANCE);
+                    rangeCursors[i] = 
btreeAccessors[i].createSearchCursor(false);
+                } else {
+                    // re-use
+                    btreeAccessors[i].reset(btree, 
NoOpOperationCallback.INSTANCE, NoOpOperationCallback.INSTANCE);
+                    rangeCursors[i].close();
+                }
+                btreeAccessors[i].search(rangeCursors[i], searchPred);
             }
-            btree = (BTree) component.getIndex();
-            if (btreeAccessors[i] == null) {
-                btreeAccessors[i] = 
btree.createAccessor(NoOpIndexAccessParameters.INSTANCE);
-                rangeCursors[i] = btreeAccessors[i].createSearchCursor(false);
-            } else {
-                // re-use
-                btreeAccessors[i].reset(btree, NoOpOperationCallback.INSTANCE, 
NoOpOperationCallback.INSTANCE);
-                rangeCursors[i].close();
+        } catch (Throwable th) { // NOSONAR: Must never leave any cursor open
+            for (int j = 0; j < i; j++) {
+                th = IndexCursorUtils.close(rangeCursors[j], th);
             }
-            btreeAccessors[i].search(rangeCursors[i], searchPred);
+            throw HyracksDataException.create(th);
         }
 
         setPriorityQueueComparator();
diff --git 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndex.java
 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndex.java
index 62493f4..0ea06d3 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndex.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndex.java
@@ -61,6 +61,15 @@
 
     void modify(IIndexOperationContext ictx, ITupleReference tuple) throws 
HyracksDataException;
 
+    /**
+     * If this method returns successfully, then the cursor has been opened, 
and need to be closed
+     * Otherwise, it has not been opened
+     * 
+     * @param ictx
+     * @param cursor
+     * @param pred
+     * @throws HyracksDataException
+     */
     void search(ILSMIndexOperationContext ictx, IIndexCursor cursor, 
ISearchPredicate pred) throws HyracksDataException;
 
     public void scanDiskComponents(ILSMIndexOperationContext ctx, IIndexCursor 
cursor) throws HyracksDataException;
diff --git 
a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/util/IndexCursorUtils.java
 
b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/util/IndexCursorUtils.java
new file mode 100644
index 0000000..e4986c8
--- /dev/null
+++ 
b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/util/IndexCursorUtils.java
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hyracks.storage.common.util;
+
+import org.apache.hyracks.api.util.ExceptionUtils;
+import org.apache.hyracks.storage.common.IIndexCursor;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+public class IndexCursorUtils {
+
+    private static final Logger LOGGER = LogManager.getLogger();
+
+    private IndexCursorUtils() {
+    }
+
+    /**
+     * Close the IIndexCursor and suppress any Throwable thrown by the close 
call.
+     * This method must NEVER throw any Throwable
+     *
+     * @param cursor
+     *            the cursor to close
+     * @param root
+     *            the first exception encountered during release of resources
+     * @return the root Throwable if not null or a new Throwable if any was 
thrown, otherwise, it returns null
+     */
+    public static Throwable close(IIndexCursor cursor, Throwable root) {
+        if (cursor != null) {
+            try {
+                cursor.close();
+            } catch (Throwable th) { // NOSONAR Will be suppressed
+                try {
+                    LOGGER.log(Level.WARN, "Failure closing a cursor", th);
+                } catch (Throwable loggingFailure) { // NOSONAR: Ignore 
catching Throwable
+                    // NOSONAR ignore logging failure
+                }
+                root = ExceptionUtils.suppress(root, th); // NOSONAR
+            }
+        }
+        return root;
+    }
+
+}

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I19db2afd2d6ca4a2ca1056cd95ae504b2be69813
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <[email protected]>

Reply via email to