abdullah alamoudi has submitted this change and it was merged.

Change subject: [ASTERIXDB-2025][STO] Fix Merge Lifecycle
......................................................................


[ASTERIXDB-2025][STO] Fix Merge Lifecycle

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

Details:
- Complete merge operation after deletion of old components
  files.

Change-Id: I843de8b26c181205e43f4eabe22a7c43f3ebfcbc
Reviewed-on: https://asterix-gerrit.ics.uci.edu/1930
Sonar-Qube: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Reviewed-by: Murtadha Hubail <mhub...@apache.org>
---
M 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
M 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetLifecycleManager.java
M 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/PrimaryIndexOperationTracker.java
M 
asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/PersistentLocalResourceRepository.java
M 
hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
M 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java
M 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMTreeIndexAccessor.java
M 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexAccessor.java
8 files changed, 47 insertions(+), 10 deletions(-)

Approvals:
  Anon. E. Moose #1000171: 
  Jenkins: Verified; No violations found; ; Verified
  Murtadha Hubail: Looks good to me, approved



diff --git 
a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
 
b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
index 9cb1de5..e74600e 100644
--- 
a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
+++ 
b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
@@ -47,8 +47,7 @@
     @Override
     public void afterOperation(ILSMIndex index, LSMOperationType opType, 
ISearchOperationCallback searchCallback,
             IModificationOperationCallback modificationCallback) throws 
HyracksDataException {
-        if (opType == LSMOperationType.FLUSH || opType == 
LSMOperationType.MERGE
-                || opType == LSMOperationType.REPLICATE) {
+        if (opType == LSMOperationType.FLUSH || opType == 
LSMOperationType.REPLICATE) {
             dsInfo.undeclareActiveIOOperation();
         }
     }
@@ -56,6 +55,9 @@
     @Override
     public void completeOperation(ILSMIndex index, LSMOperationType opType, 
ISearchOperationCallback searchCallback,
             IModificationOperationCallback modificationCallback) throws 
HyracksDataException {
+        if (opType == LSMOperationType.MERGE) {
+            dsInfo.undeclareActiveIOOperation();
+        }
     }
 
     public void exclusiveJobCommitted() throws HyracksDataException {
diff --git 
a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetLifecycleManager.java
 
b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetLifecycleManager.java
index 8abbeab..37bd789 100644
--- 
a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetLifecycleManager.java
+++ 
b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetLifecycleManager.java
@@ -147,7 +147,8 @@
                     //notification will come from DatasetInfo class 
(undeclareActiveIOOperation)
                     dsInfo.wait();
                 } catch (InterruptedException e) {
-                    throw new HyracksDataException(e);
+                    Thread.currentThread().interrupt();
+                    throw HyracksDataException.create(e);
                 }
             }
         }
diff --git 
a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/PrimaryIndexOperationTracker.java
 
b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/PrimaryIndexOperationTracker.java
index 903bb50..67b25b6 100644
--- 
a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/PrimaryIndexOperationTracker.java
+++ 
b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/PrimaryIndexOperationTracker.java
@@ -68,8 +68,7 @@
     public void afterOperation(ILSMIndex index, LSMOperationType opType, 
ISearchOperationCallback searchCallback,
             IModificationOperationCallback modificationCallback) throws 
HyracksDataException {
         // Searches are immediately considered complete, because they should 
not prevent the execution of flushes.
-        if (opType == LSMOperationType.FLUSH || opType == 
LSMOperationType.MERGE
-                || opType == LSMOperationType.REPLICATE) {
+        if (opType == LSMOperationType.FLUSH || opType == 
LSMOperationType.REPLICATE) {
             completeOperation(index, opType, searchCallback, 
modificationCallback);
         }
     }
diff --git 
a/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/PersistentLocalResourceRepository.java
 
b/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/PersistentLocalResourceRepository.java
index b117cf1..85ad6b4 100644
--- 
a/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/PersistentLocalResourceRepository.java
+++ 
b/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/PersistentLocalResourceRepository.java
@@ -59,6 +59,7 @@
 import 
org.apache.hyracks.api.replication.IReplicationJob.ReplicationExecutionType;
 import org.apache.hyracks.api.replication.IReplicationJob.ReplicationJobType;
 import org.apache.hyracks.api.replication.IReplicationJob.ReplicationOperation;
+import org.apache.hyracks.api.util.IoUtil;
 import org.apache.hyracks.storage.am.common.api.ITreeIndexFrame;
 import org.apache.hyracks.storage.common.ILocalResourceRepository;
 import org.apache.hyracks.storage.common.LocalResource;
@@ -223,7 +224,7 @@
                 // Invalidate before deleting the file just in case file 
deletion throws some exception.
                 // Since it's just a cache invalidation, it should not affect 
correctness.
                 resourceCache.invalidate(relativePath);
-                resourceFile.delete();
+                IoUtil.delete(resourceFile);
             } finally {
                 // Regardless of successfully deleted or not, the operation 
should be replicated.
                 //if replication enabled, delete resource from remote replicas
diff --git 
a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
 
b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
index 1e959e1..af843d2 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
@@ -19,6 +19,9 @@
 
 package org.apache.hyracks.storage.am.common.dataflow;
 
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
 import org.apache.hyracks.api.application.INCServiceContext;
 import org.apache.hyracks.api.exceptions.HyracksDataException;
 import org.apache.hyracks.api.io.FileReference;
@@ -32,6 +35,7 @@
 
 public class IndexDataflowHelper implements IIndexDataflowHelper {
 
+    private static final Logger LOGGER = 
Logger.getLogger(IndexDataflowHelper.class.getName());
     private final INCServiceContext ctx;
     private final IResourceLifecycleManager<IIndex> lcManager;
     private final ILocalResourceRepository localResourceRepository;
@@ -85,6 +89,7 @@
 
     @Override
     public void destroy() throws HyracksDataException {
+        LOGGER.log(Level.INFO, "Dropping index " + 
resourceRef.getRelativePath() + " on node " + ctx.getNodeId());
         synchronized (lcManager) {
             index = lcManager.get(resourceRef.getRelativePath());
             if (index != null) {
diff --git 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java
 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java
index b1005bd..d0dc4b3 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java
@@ -138,7 +138,8 @@
                     }
                     opTracker.wait();
                 } catch (InterruptedException e) {
-                    throw new HyracksDataException(e);
+                    Thread.currentThread().interrupt();
+                    throw HyracksDataException.create(e);
                 }
             }
         }
@@ -346,12 +347,11 @@
                         lsmIndex.scheduleReplication(null, 
inactiveDiskComponentsToBeDeleted, false,
                                 ReplicationOperation.DELETE, opType);
                     }
-
                     for (ILSMComponent c : inactiveDiskComponentsToBeDeleted) {
                         ((AbstractLSMDiskComponent) c).destroy();
                     }
                 } catch (Throwable e) {
-                    e.printStackTrace();
+                    LOGGER.log(Level.WARNING, "Failure scheduling replication 
or destroying merged component", e);
                     throw e;
                 }
             }
@@ -561,10 +561,25 @@
             lsmIndex.markAsValid(newComponent);
         } catch (Throwable e) {
             failedOperation = true;
-            e.printStackTrace();
+            LOGGER.log(Level.SEVERE, "Failed merge operation on " + lsmIndex, 
e);
             throw e;
         } finally {
             exitComponents(ctx, LSMOperationType.MERGE, newComponent, 
failedOperation);
+            // Completion of the merge operation is called here to and not on 
afterOperation because
+            // Deletion of the old components comes after afterOperation is 
called and the number of
+            // io operation should not be decremented before the operation is 
complete to avoid
+            // index destroy from competing with the merge on deletion of the 
files.
+            // The order becomes:
+            // 1. scheduleMerge
+            // 2. enterComponents
+            // 3. beforeOperation (increment the numOfIoOperations)
+            // 4. merge
+            // 5. exitComponents
+            // 6. afterOperation (no op)
+            // 7. delete components
+            // 8. completeOperation (decrement the numOfIoOperations)
+            opTracker.completeOperation(lsmIndex, LSMOperationType.MERGE, 
ctx.getSearchOperationCallback(),
+                    ctx.getModificationCallback());
             operation.getCallback().afterFinalize(LSMOperationType.MERGE, 
newComponent);
         }
         if (LOGGER.isLoggable(Level.INFO)) {
@@ -701,4 +716,9 @@
         }
         throw 
HyracksDataException.create(ErrorCode.CANNOT_MODIFY_INDEX_DISK_IS_FULL);
     }
+
+    @Override
+    public String toString() {
+        return getClass().getSimpleName() + ":" + lsmIndex;
+    }
 }
diff --git 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMTreeIndexAccessor.java
 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMTreeIndexAccessor.java
index b9714a6..a45225d 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMTreeIndexAccessor.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMTreeIndexAccessor.java
@@ -219,4 +219,9 @@
         ctx.setOperation(IndexOperation.DISK_COMPONENT_SCAN);
         lsmHarness.scanDiskComponents(ctx, cursor);
     }
+
+    @Override
+    public String toString() {
+        return getClass().getSimpleName() + ':' + lsmHarness.toString();
+    }
 }
diff --git 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexAccessor.java
 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexAccessor.java
index 7751116..dddd14a 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexAccessor.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexAccessor.java
@@ -207,6 +207,10 @@
     @Override
     public void scanDiskComponents(IIndexCursor cursor) throws 
HyracksDataException {
         throw 
HyracksDataException.create(ErrorCode.DISK_COMPONENT_SCAN_NOT_ALLOWED_FOR_SECONDARY_INDEX);
+    }
 
+    @Override
+    public String toString() {
+        return getClass().getSimpleName() + ':' + lsmHarness.toString();
     }
 }

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I843de8b26c181205e43f4eabe22a7c43f3ebfcbc
Gerrit-PatchSet: 15
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <mhub...@apache.org>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>

Reply via email to