Murtadha Hubail has submitted this change and it was merged. Change subject: ASTERIXDB-1377: Prevent Operations from Exiting Components Multiple Times ......................................................................
ASTERIXDB-1377: Prevent Operations from Exiting Components Multiple Times Change-Id: I5907b1b4c76ce48366f5447f2940f3561c474cfe Reviewed-on: https://asterix-gerrit.ics.uci.edu/769 Tested-by: Jenkins <[email protected]> Reviewed-by: Yingyi Bu <[email protected]> --- M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeOpContext.java M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeWithBuddyOpContext.java M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeOpContext.java M hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexOperationContext.java A hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndexOperationContext.java M hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/ExternalIndexHarness.java M hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java M hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMIndexSearchCursor.java M hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexOpContext.java M hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/ExternalRTreeOpContext.java M hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeOpContext.java 11 files changed, 105 insertions(+), 21 deletions(-) Approvals: Yingyi Bu: Looks good to me, approved Jenkins: Verified diff --git a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeOpContext.java b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeOpContext.java index d63671e..29fedef 100644 --- a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeOpContext.java +++ b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeOpContext.java @@ -31,9 +31,9 @@ import org.apache.hyracks.storage.am.common.ophelpers.MultiComparator; import org.apache.hyracks.storage.am.lsm.common.api.ILSMComponent; import org.apache.hyracks.storage.am.lsm.common.api.ILSMHarness; -import org.apache.hyracks.storage.am.lsm.common.api.ILSMIndexOperationContext; +import org.apache.hyracks.storage.am.lsm.common.impls.AbstractLSMIndexOperationContext; -public class ExternalBTreeOpContext implements ILSMIndexOperationContext { +public class ExternalBTreeOpContext extends AbstractLSMIndexOperationContext { public ITreeIndexFrameFactory insertLeafFrameFactory; public ITreeIndexFrameFactory deleteLeafFrameFactory; public IBTreeLeafFrame insertLeafFrame; @@ -86,6 +86,7 @@ @Override public void reset() { + super.reset(); componentHolder.clear(); componentsToBeMerged.clear(); componentsToBeReplicated.clear(); diff --git a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeWithBuddyOpContext.java b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeWithBuddyOpContext.java index a837301..c44f529 100644 --- a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeWithBuddyOpContext.java +++ b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeWithBuddyOpContext.java @@ -31,9 +31,9 @@ import org.apache.hyracks.storage.am.common.ophelpers.MultiComparator; import org.apache.hyracks.storage.am.lsm.common.api.ILSMComponent; import org.apache.hyracks.storage.am.lsm.common.api.ILSMHarness; -import org.apache.hyracks.storage.am.lsm.common.api.ILSMIndexOperationContext; +import org.apache.hyracks.storage.am.lsm.common.impls.AbstractLSMIndexOperationContext; -public class ExternalBTreeWithBuddyOpContext implements ILSMIndexOperationContext { +public class ExternalBTreeWithBuddyOpContext extends AbstractLSMIndexOperationContext { private IndexOperation op; private MultiComparator bTreeCmp; private MultiComparator buddyBTreeCmp; @@ -74,6 +74,7 @@ @Override public void reset() { + super.reset(); componentHolder.clear(); componentsToBeMerged.clear(); componentsToBeReplicated.clear(); diff --git a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeOpContext.java b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeOpContext.java index e833283..31c9d40 100644 --- a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeOpContext.java +++ b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeOpContext.java @@ -38,9 +38,9 @@ import org.apache.hyracks.storage.am.common.tuples.PermutingTupleReference; import org.apache.hyracks.storage.am.lsm.common.api.ILSMComponent; import org.apache.hyracks.storage.am.lsm.common.api.ILSMHarness; -import org.apache.hyracks.storage.am.lsm.common.api.ILSMIndexOperationContext; +import org.apache.hyracks.storage.am.lsm.common.impls.AbstractLSMIndexOperationContext; -public final class LSMBTreeOpContext implements ILSMIndexOperationContext { +public final class LSMBTreeOpContext extends AbstractLSMIndexOperationContext { public ITreeIndexFrameFactory insertLeafFrameFactory; public ITreeIndexFrameFactory deleteLeafFrameFactory; @@ -145,6 +145,7 @@ @Override public void reset() { + super.reset(); componentHolder.clear(); componentsToBeMerged.clear(); componentsToBeReplicated.clear(); diff --git a/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexOperationContext.java b/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexOperationContext.java index 99f981d..acf2233 100644 --- a/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexOperationContext.java +++ b/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexOperationContext.java @@ -41,4 +41,11 @@ public ISearchPredicate getSearchPredicate(); public List<ILSMComponent> getComponentsToBeReplicated(); + + /** + * @return true if this operation entered the components. Otherwise false. + */ + public boolean isAccessingComponents(); + + public void setAccessingComponents(boolean accessingComponents); } diff --git a/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndexOperationContext.java b/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndexOperationContext.java new file mode 100644 index 0000000..3b907c3 --- /dev/null +++ b/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndexOperationContext.java @@ -0,0 +1,41 @@ +/* + * 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.am.lsm.common.impls; + +import org.apache.hyracks.storage.am.lsm.common.api.ILSMIndexOperationContext; + +public abstract class AbstractLSMIndexOperationContext implements ILSMIndexOperationContext { + + private boolean accessingComponents = false; + + @Override + public boolean isAccessingComponents() { + return accessingComponents; + } + + @Override + public void setAccessingComponents(boolean accessingComponents) { + this.accessingComponents = accessingComponents; + } + + @Override + public void reset() { + accessingComponents = false; + } +} diff --git a/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/ExternalIndexHarness.java b/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/ExternalIndexHarness.java index 8e39223..e4be66b 100644 --- a/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/ExternalIndexHarness.java +++ b/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/ExternalIndexHarness.java @@ -42,14 +42,15 @@ public class ExternalIndexHarness extends LSMHarness { private static final Logger LOGGER = Logger.getLogger(ExternalIndexHarness.class.getName()); - public ExternalIndexHarness(ILSMIndexInternal lsmIndex, ILSMMergePolicy mergePolicy, - ILSMOperationTracker opTracker, boolean replicationEnabled) { + public ExternalIndexHarness(ILSMIndexInternal lsmIndex, ILSMMergePolicy mergePolicy, ILSMOperationTracker opTracker, + boolean replicationEnabled) { super(lsmIndex, mergePolicy, opTracker, replicationEnabled); } @Override protected boolean getAndEnterComponents(ILSMIndexOperationContext ctx, LSMOperationType opType, boolean isTryOperation) throws HyracksDataException { + validateOperationEnterComponentsState(ctx); synchronized (opTracker) { while (true) { lsmIndex.getOperationalComponents(ctx); @@ -75,6 +76,7 @@ @Override protected boolean enterComponents(ILSMIndexOperationContext ctx, LSMOperationType opType) throws HyracksDataException { + validateOperationEnterComponentsState(ctx); List<ILSMComponent> components = ctx.getComponentHolder(); int numEntered = 0; boolean entranceSuccessful = false; @@ -97,6 +99,7 @@ } return false; } + ctx.setAccessingComponents(true); } // Check if there is any action that is needed to be taken based on the operation type switch (opType) { @@ -111,6 +114,13 @@ private void exitComponents(ILSMIndexOperationContext ctx, LSMOperationType opType, ILSMComponent newComponent, boolean failedOperation) throws HyracksDataException, IndexException { + /** + * FLUSH and MERGE operations should always exit the components + * to notify waiting threads. + */ + if (!ctx.isAccessingComponents() && opType != LSMOperationType.FLUSH && opType != LSMOperationType.MERGE) { + return; + } synchronized (opTracker) { try { // First check if there is any action that is needed to be taken based on the state of each component. @@ -130,6 +140,7 @@ break; } } + ctx.setAccessingComponents(false); // Then, perform any action that is needed to be taken based on the operation type. switch (opType) { case MERGE: @@ -156,8 +167,8 @@ } @Override - public void forceModify(ILSMIndexOperationContext ctx, ITupleReference tuple) throws HyracksDataException, - IndexException { + public void forceModify(ILSMIndexOperationContext ctx, ITupleReference tuple) + throws HyracksDataException, IndexException { throw new IndexException("2PC LSM Inedx doesn't support modify"); } @@ -216,8 +227,8 @@ } @Override - public void merge(ILSMIndexOperationContext ctx, ILSMIOOperation operation) throws HyracksDataException, - IndexException { + public void merge(ILSMIndexOperationContext ctx, ILSMIOOperation operation) + throws HyracksDataException, IndexException { if (LOGGER.isLoggable(Level.INFO)) { LOGGER.info("Started a merge operation for index: " + lsmIndex + " ..."); } @@ -293,8 +304,8 @@ } @Override - public void flush(ILSMIndexOperationContext ctx, ILSMIOOperation operation) throws HyracksDataException, - IndexException { + public void flush(ILSMIndexOperationContext ctx, ILSMIOOperation operation) + throws HyracksDataException, IndexException { } @Override diff --git a/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java b/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java index 0224c5c..a19532f 100644 --- a/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java +++ b/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java @@ -72,6 +72,7 @@ protected boolean getAndEnterComponents(ILSMIndexOperationContext ctx, LSMOperationType opType, boolean isTryOperation) throws HyracksDataException { + validateOperationEnterComponentsState(ctx); synchronized (opTracker) { while (true) { lsmIndex.getOperationalComponents(ctx); @@ -133,6 +134,7 @@ protected boolean enterComponents(ILSMIndexOperationContext ctx, LSMOperationType opType) throws HyracksDataException { + validateOperationEnterComponentsState(ctx); List<ILSMComponent> components = ctx.getComponentHolder(); int numEntered = 0; boolean entranceSuccessful = false; @@ -162,6 +164,7 @@ } return false; } + ctx.setAccessingComponents(true); } // Check if there is any action that is needed to be taken based on the operation type switch (opType) { @@ -185,6 +188,13 @@ private void exitComponents(ILSMIndexOperationContext ctx, LSMOperationType opType, ILSMComponent newComponent, boolean failedOperation) throws HyracksDataException, IndexException { + /** + * FLUSH and MERGE operations should always exit the components + * to notify waiting threads. + */ + if (!ctx.isAccessingComponents() && opType != LSMOperationType.FLUSH && opType != LSMOperationType.MERGE) { + return; + } List<ILSMComponent> inactiveDiskComponents = null; List<ILSMComponent> inactiveDiskComponentsToBeDeleted = null; try { @@ -225,6 +235,7 @@ } i++; } + ctx.setAccessingComponents(false); // Then, perform any action that is needed to be taken based on the operation type. switch (opType) { case FLUSH: @@ -506,4 +517,10 @@ throw new HyracksDataException(e); } } + + protected void validateOperationEnterComponentsState(ILSMIndexOperationContext ctx) throws HyracksDataException { + if (ctx.isAccessingComponents()) { + throw new HyracksDataException("Opeartion already has access to components of index " + lsmIndex); + } + } } diff --git a/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMIndexSearchCursor.java b/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMIndexSearchCursor.java index c4d2fcc..befdd85 100644 --- a/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMIndexSearchCursor.java +++ b/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMIndexSearchCursor.java @@ -141,7 +141,9 @@ @Override public void close() throws HyracksDataException { try { - outputPriorityQueue.clear(); + if (outputPriorityQueue != null) { + outputPriorityQueue.clear(); + } for (int i = 0; i < rangeCursors.length; i++) { rangeCursors[i].close(); } diff --git a/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexOpContext.java b/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexOpContext.java index c511a67..828e296 100644 --- a/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexOpContext.java +++ b/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexOpContext.java @@ -32,10 +32,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.common.api.ILSMComponent; -import org.apache.hyracks.storage.am.lsm.common.api.ILSMIndexOperationContext; +import org.apache.hyracks.storage.am.lsm.common.impls.AbstractLSMIndexOperationContext; import org.apache.hyracks.storage.am.lsm.invertedindex.api.IInvertedIndexAccessor; -public class LSMInvertedIndexOpContext implements ILSMIndexOperationContext { +public class LSMInvertedIndexOpContext extends AbstractLSMIndexOperationContext { private static final int NUM_DOCUMENT_FIELDS = 1; @@ -109,6 +109,7 @@ @Override public void reset() { + super.reset(); componentHolder.clear(); componentsToBeMerged.clear(); componentsToBeReplicated.clear(); diff --git a/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/ExternalRTreeOpContext.java b/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/ExternalRTreeOpContext.java index 6a9a640..358a42a 100644 --- a/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/ExternalRTreeOpContext.java +++ b/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/ExternalRTreeOpContext.java @@ -30,9 +30,9 @@ import org.apache.hyracks.storage.am.common.ophelpers.MultiComparator; import org.apache.hyracks.storage.am.lsm.common.api.ILSMComponent; import org.apache.hyracks.storage.am.lsm.common.api.ILSMHarness; -import org.apache.hyracks.storage.am.lsm.common.api.ILSMIndexOperationContext; +import org.apache.hyracks.storage.am.lsm.common.impls.AbstractLSMIndexOperationContext; -public class ExternalRTreeOpContext implements ILSMIndexOperationContext { +public class ExternalRTreeOpContext extends AbstractLSMIndexOperationContext { private IndexOperation op; private MultiComparator bTreeCmp; private MultiComparator rTreeCmp; @@ -74,6 +74,7 @@ @Override public void reset() { + super.reset(); componentHolder.clear(); componentsToBeMerged.clear(); componentsToBeReplicated.clear(); diff --git a/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeOpContext.java b/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeOpContext.java index 686cd2b..62f572f 100644 --- a/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeOpContext.java +++ b/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeOpContext.java @@ -35,11 +35,11 @@ import org.apache.hyracks.storage.am.common.tuples.PermutingTupleReference; import org.apache.hyracks.storage.am.lsm.common.api.ILSMComponent; import org.apache.hyracks.storage.am.lsm.common.api.ILSMHarness; -import org.apache.hyracks.storage.am.lsm.common.api.ILSMIndexOperationContext; +import org.apache.hyracks.storage.am.lsm.common.impls.AbstractLSMIndexOperationContext; import org.apache.hyracks.storage.am.rtree.impls.RTree; import org.apache.hyracks.storage.am.rtree.impls.RTreeOpContext; -public final class LSMRTreeOpContext implements ILSMIndexOperationContext { +public final class LSMRTreeOpContext extends AbstractLSMIndexOperationContext { public RTree.RTreeAccessor[] mutableRTreeAccessors; public RTree.RTreeAccessor currentMutableRTreeAccessor; @@ -131,6 +131,7 @@ @Override public void reset() { + super.reset(); componentHolder.clear(); componentsToBeMerged.clear(); } -- To view, visit https://asterix-gerrit.ics.uci.edu/769 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5907b1b4c76ce48366f5447f2940f3561c474cfe Gerrit-PatchSet: 3 Gerrit-Project: hyracks Gerrit-Branch: master Gerrit-Owner: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]>
