Rename type check methods into meaningful names
This change renames methods used for checking operation in upsert
operators into meaningful names. In addition, it removes the
unnecessary search in case of delete operations with only a primary
index in the pipeline.
Change-Id: I35e5ed919aff2c374be1fbbb00ad7a752916a3dc
Reviewed-on: https://asterix-gerrit.ics.uci.edu/1735
Sonar-Qube: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Yingyi Bu <buyingyi@gmail.com>
diff --git a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/CancellationTestExecutor.java b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/CancellationTestExecutor.java
index 5ab9c1f..ac4d1c7 100644
--- a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/CancellationTestExecutor.java
+++ b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/CancellationTestExecutor.java
@@ -30,8 +30,11 @@
import java.util.concurrent.Future;
import org.apache.asterix.common.utils.Servlets;
+import org.apache.asterix.test.runtime.SqlppExecutionWithCancellationTest;
import org.apache.asterix.testframework.context.TestCaseContext;
import org.apache.asterix.testframework.xml.TestCase;
+import org.apache.asterix.testframework.xml.TestCase.CompilationUnit;
+import org.apache.commons.lang3.mutable.MutableInt;
import org.apache.http.HttpResponse;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.client.methods.RequestBuilder;
@@ -87,4 +90,36 @@
builder.setCharset(StandardCharsets.UTF_8);
return builder.build();
}
+
+ @Override
+ protected boolean isUnExpected(Exception e, CompilationUnit cUnit, int numOfErrors, MutableInt queryCount) {
+ if (super.isUnExpected(e, cUnit, numOfErrors, queryCount)) {
+ String errorMsg = getErrorMessage(e);
+ // Expected, "HYR0025" means a user cancelled the query.)
+ if (errorMsg.startsWith("HYR0025")) {
+ SqlppExecutionWithCancellationTest.numCancelledQueries++;
+ queryCount.increment();
+ } else {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ public static String getErrorMessage(Throwable th) {
+ Throwable cause = getRootCause(th);
+ return cause.getMessage();
+ }
+
+ // Finds the root cause of Throwable.
+ private static Throwable getRootCause(Throwable e) {
+ Throwable current = e;
+ Throwable cause = e.getCause();
+ while (cause != null && cause != current) {
+ Throwable nextCause = current.getCause();
+ current = cause;
+ cause = nextCause;
+ }
+ return current;
+ }
}
diff --git a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
index 9913493..3c5f823 100644
--- a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
+++ b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
@@ -381,8 +381,8 @@
if (match && !negate || negate && !match) {
continue;
}
- throw new Exception(
- "Result for " + scriptFile + ": expected pattern '" + expression + "' not found in result: "+actual);
+ throw new Exception("Result for " + scriptFile + ": expected pattern '" + expression
+ + "' not found in result: " + actual);
}
} catch (Exception e) {
System.err.println("Actual results file: " + actualFile.toString());
@@ -1241,27 +1241,11 @@
expectedResultFileCtxs, testFile, actualPath);
} catch (Exception e) {
System.err.println("testFile " + testFile.toString() + " raised an exception: " + e);
- boolean unExpectedFailure = false;
numOfErrors++;
- String expectedError = null;
- if (cUnit.getExpectedError().size() < numOfErrors) {
- unExpectedFailure = true;
- } else {
- // Get the expected exception
- expectedError = cUnit.getExpectedError().get(numOfErrors - 1);
- if (e.toString().contains(expectedError)) {
- System.err.println("...but that was expected.");
- } else {
- unExpectedFailure = true;
- }
- }
+ boolean unExpectedFailure = isUnExpected(e, cUnit, numOfErrors, queryCount);
if (unExpectedFailure) {
e.printStackTrace();
System.err.println("...Unexpected!");
- if (expectedError != null) {
- System.err.println("Expected to find the following in error text:\n+++++\n" + expectedError
- + "\n+++++");
- }
if (failedGroup != null) {
failedGroup.getTestCase().add(testCaseCtx.getTestCase());
}
@@ -1283,6 +1267,24 @@
}
}
+ protected boolean isUnExpected(Exception e, CompilationUnit cUnit, int numOfErrors, MutableInt queryCount) {
+ String expectedError = null;
+ if (cUnit.getExpectedError().size() < numOfErrors) {
+ return true;
+ } else {
+ // Get the expected exception
+ expectedError = cUnit.getExpectedError().get(numOfErrors - 1);
+ if (e.toString().contains(expectedError)) {
+ System.err.println("...but that was expected.");
+ return false;
+ } else {
+ System.err
+ .println("Expected to find the following in error text:\n+++++\n" + expectedError + "\n+++++");
+ return true;
+ }
+ }
+ }
+
private static File getTestCaseQueryBeforeCrashFile(String actualPath, TestCaseContext testCaseCtx,
CompilationUnit cUnit) {
return new File(
diff --git a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/runtime/SqlppExecutionWithCancellationTest.java b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/runtime/SqlppExecutionWithCancellationTest.java
index edf5741..cce069c 100644
--- a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/runtime/SqlppExecutionWithCancellationTest.java
+++ b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/runtime/SqlppExecutionWithCancellationTest.java
@@ -36,7 +36,7 @@
@RunWith(Parameterized.class)
public class SqlppExecutionWithCancellationTest {
protected static final String TEST_CONFIG_FILE_NAME = "asterix-build-configuration.xml";
- private static int numCancelledQueries = 0;
+ public static int numCancelledQueries = 0;
@BeforeClass
public static void setUp() throws Exception {
@@ -70,31 +70,12 @@
try {
LangExecutionUtil.test(tcCtx);
} catch (Exception e) {
- Throwable cause = getRootCause(e);
- String errorMsg = cause.getMessage();
- if (errorMsg.startsWith("HYR0025") // Expected, "HYR0025" means a user cancelled the query.
- || errorMsg.contains("\"status\": ") // Expected, "status" results for cancelled queries can change.
- || errorMsg.contains("reference count = 1") // not expected, but is a false alarm.
- || errorMsg.contains("pinned and file is being closed") // not expected, but maybe a false alarm.
- // happens after the test query: big_object_load_20M.
+ String errorMsg = CancellationTestExecutor.getErrorMessage(e);
+ if (!errorMsg.contains("reference count = 1") // not expected, but is a false alarm.
+ && !errorMsg.contains("pinned and file is being closed") // not expected, but maybe a false alarm.
) {
- numCancelledQueries++;
- } else {
- // Re-throw other kinds of exceptions.
throw e;
}
}
}
-
- // Finds the root cause of Throwable.
- private Throwable getRootCause(Throwable e) {
- Throwable current = e;
- Throwable cause = e.getCause();
- while (cause != null) {
- Throwable nextCause = current.getCause();
- current = cause;
- cause = nextCause;
- }
- return current;
- }
}
diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeTagUtil.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeTagUtil.java
index 1af3578..5d91d2f 100644
--- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeTagUtil.java
+++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeTagUtil.java
@@ -20,6 +20,7 @@
import org.apache.asterix.common.exceptions.AsterixException;
import org.apache.asterix.om.utils.RecordUtil;
+import org.apache.hyracks.dataflow.common.data.accessors.ITupleReference;
public class TypeTagUtil {
@@ -91,4 +92,8 @@
throw new AsterixException("Typetag " + typeTag + " is not a built-in type");
}
}
+
+ public static boolean isType(ITupleReference tuple, int fieldIdx, byte tag) {
+ return tuple.getFieldData(fieldIdx)[tuple.getFieldStart(fieldIdx)] == tag;
+ }
}
diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java
index ea404e1..bff3ca6 100644
--- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java
+++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java
@@ -31,6 +31,7 @@
import org.apache.asterix.om.pointables.nonvisitor.ARecordPointable;
import org.apache.asterix.om.types.ARecordType;
import org.apache.asterix.om.types.ATypeTag;
+import org.apache.asterix.om.types.TypeTagUtil;
import org.apache.asterix.transaction.management.opcallbacks.AbstractIndexModificationOperationCallback;
import org.apache.asterix.transaction.management.opcallbacks.AbstractIndexModificationOperationCallback.Operation;
import org.apache.asterix.transaction.management.opcallbacks.LockThenSearchOperationCallback;
@@ -199,11 +200,11 @@
}
}
- public static boolean isNull(ITupleReference t1, int field) {
- return t1.getFieldData(field)[t1.getFieldStart(field)] == ATypeTag.SERIALIZED_MISSING_TYPE_TAG;
+ private static boolean isDeleteOperation(ITupleReference t1, int field) {
+ return TypeTagUtil.isType(t1, field, ATypeTag.SERIALIZED_MISSING_TYPE_TAG);
}
- private void addNullField() throws IOException {
+ private void writeMissingField() throws IOException {
dos.write(missingTupleBuilder.getByteArray());
tb.addFieldEndOffset();
}
@@ -219,51 +220,37 @@
while (i < tupleCount) {
tb.reset();
boolean recordWasInserted = false;
+ boolean recordWasDeleted = false;
tuple.reset(accessor, i);
resetSearchPredicate(i);
- if (hasSecondaries || isNull(tuple, numOfPrimaryKeys)) {
+ if (hasSecondaries) {
lsmAccessor.search(cursor, searchPred);
if (cursor.hasNext()) {
cursor.next();
prevTuple = cursor.getTuple();
- cursor.reset();
- if (isFiltered) {
- prevTuple = getPrevTupleWithFilter(prevTuple);
- }
- dos.write(prevTuple.getFieldData(numOfPrimaryKeys), prevTuple.getFieldStart(numOfPrimaryKeys),
- prevTuple.getFieldLength(numOfPrimaryKeys));
- tb.addFieldEndOffset();
- // if has meta, then append meta
- if (hasMeta) {
- dos.write(prevTuple.getFieldData(metaFieldIndex), prevTuple.getFieldStart(metaFieldIndex),
- prevTuple.getFieldLength(metaFieldIndex));
- tb.addFieldEndOffset();
- }
- // if with filters, append the filter
- if (isFiltered) {
- dos.write(prevTuple.getFieldData(filterFieldIndex),
- prevTuple.getFieldStart(filterFieldIndex),
- prevTuple.getFieldLength(filterFieldIndex));
- tb.addFieldEndOffset();
- }
- if (isNull(tuple, numOfPrimaryKeys)) {
- // Only delete if it is a delete and not upsert
- abstractModCallback.setOp(Operation.DELETE);
- if (firstModification) {
- lsmAccessor.delete(prevTuple);
- firstModification = false;
- } else {
- lsmAccessor.forceDelete(prevTuple);
- }
- }
+ cursor.reset(); // end the search
+ appendFilterToPrevTuple();
+ appendPrevRecord();
+ appendPreviousMeta();
+ appendFilterToOutput();
} else {
- appendNullPreviousTuple();
+ appendPreviousTupleAsMissing();
}
} else {
- searchCallback.before(key);
- appendNullPreviousTuple();
+ searchCallback.before(key); // lock
+ appendPreviousTupleAsMissing();
}
- if (!isNull(tuple, numOfPrimaryKeys)) {
+ if (isDeleteOperation(tuple, numOfPrimaryKeys)) {
+ // Only delete if it is a delete and not upsert
+ abstractModCallback.setOp(Operation.DELETE);
+ if (firstModification) {
+ lsmAccessor.delete(tuple);
+ firstModification = false;
+ } else {
+ lsmAccessor.forceDelete(tuple);
+ }
+ recordWasDeleted = true;
+ } else {
abstractModCallback.setOp(Operation.UPSERT);
if (firstModification) {
lsmAccessor.upsert(tuple);
@@ -273,7 +260,7 @@
}
recordWasInserted = true;
}
- writeOutput(i, recordWasInserted, prevTuple != null);
+ writeOutput(i, recordWasInserted, recordWasDeleted);
i++;
}
// callback here before calling nextFrame on the next operator
@@ -284,15 +271,39 @@
}
}
- private void appendNullPreviousTuple() throws IOException {
- prevTuple = null;
- addNullField();
+ private void appendFilterToOutput() throws IOException {
+ // if with filters, append the filter
+ if (isFiltered) {
+ dos.write(prevTuple.getFieldData(filterFieldIndex), prevTuple.getFieldStart(filterFieldIndex),
+ prevTuple.getFieldLength(filterFieldIndex));
+ tb.addFieldEndOffset();
+ }
+ }
+
+ private void appendPrevRecord() throws IOException {
+ dos.write(prevTuple.getFieldData(numOfPrimaryKeys), prevTuple.getFieldStart(numOfPrimaryKeys),
+ prevTuple.getFieldLength(numOfPrimaryKeys));
+ tb.addFieldEndOffset();
+ }
+
+ private void appendPreviousMeta() throws IOException {
+ // if has meta, then append meta
if (hasMeta) {
- addNullField();
+ dos.write(prevTuple.getFieldData(metaFieldIndex), prevTuple.getFieldStart(metaFieldIndex),
+ prevTuple.getFieldLength(metaFieldIndex));
+ tb.addFieldEndOffset();
+ }
+ }
+
+ private void appendPreviousTupleAsMissing() throws IOException {
+ prevTuple = null;
+ writeMissingField();
+ if (hasMeta) {
+ writeMissingField();
}
// if with filters, append null
if (isFiltered) {
- addNullField();
+ writeMissingField();
}
cursor.reset();
}
@@ -306,24 +317,26 @@
appender.write(writer, true);
}
- private ITupleReference getPrevTupleWithFilter(ITupleReference prevTuple) throws IOException, AsterixException {
- prevRecWithPKWithFilterValue.reset();
- for (int i = 0; i < prevTuple.getFieldCount(); i++) {
- prevDos.write(prevTuple.getFieldData(i), prevTuple.getFieldStart(i), prevTuple.getFieldLength(i));
+ private void appendFilterToPrevTuple() throws IOException, AsterixException {
+ if (isFiltered) {
+ prevRecWithPKWithFilterValue.reset();
+ for (int i = 0; i < prevTuple.getFieldCount(); i++) {
+ prevDos.write(prevTuple.getFieldData(i), prevTuple.getFieldStart(i), prevTuple.getFieldLength(i));
+ prevRecWithPKWithFilterValue.addFieldEndOffset();
+ }
+ recPointable.set(prevTuple.getFieldData(numOfPrimaryKeys), prevTuple.getFieldStart(numOfPrimaryKeys),
+ prevTuple.getFieldLength(numOfPrimaryKeys));
+ // copy the field data from prevTuple
+ byte tag = recPointable.getClosedFieldType(recordType, presetFieldIndex).getTypeTag().serialize();
+ prevDos.write(tag);
+ prevDos.write(recPointable.getByteArray(), recPointable.getClosedFieldOffset(recordType, presetFieldIndex),
+ recPointable.getClosedFieldSize(recordType, presetFieldIndex));
prevRecWithPKWithFilterValue.addFieldEndOffset();
+ // prepare the tuple
+ prevTupleWithFilter.reset(prevRecWithPKWithFilterValue.getFieldEndOffsets(),
+ prevRecWithPKWithFilterValue.getByteArray());
+ prevTuple = prevTupleWithFilter;
}
- recPointable.set(prevTuple.getFieldData(numOfPrimaryKeys), prevTuple.getFieldStart(numOfPrimaryKeys),
- prevTuple.getFieldLength(numOfPrimaryKeys));
- // copy the field data from prevTuple
- byte tag = recPointable.getClosedFieldType(recordType, presetFieldIndex).getTypeTag().serialize();
- prevDos.write(tag);
- prevDos.write(recPointable.getByteArray(), recPointable.getClosedFieldOffset(recordType, presetFieldIndex),
- recPointable.getClosedFieldSize(recordType, presetFieldIndex));
- prevRecWithPKWithFilterValue.addFieldEndOffset();
- // prepare the tuple
- prevTupleWithFilter.reset(prevRecWithPKWithFilterValue.getFieldEndOffsets(),
- prevRecWithPKWithFilterValue.getByteArray());
- return prevTupleWithFilter;
}
private RangePredicate createSearchPredicate() {
diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java
index a2ddf27..a22e5e7 100644
--- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java
+++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java
@@ -20,6 +20,8 @@
import java.nio.ByteBuffer;
+import org.apache.asterix.om.types.ATypeTag;
+import org.apache.asterix.om.types.TypeTagUtil;
import org.apache.asterix.transaction.management.opcallbacks.AbstractIndexModificationOperationCallback;
import org.apache.asterix.transaction.management.opcallbacks.AbstractIndexModificationOperationCallback.Operation;
import org.apache.hyracks.api.context.IHyracksTaskContext;
@@ -53,7 +55,7 @@
public class LSMSecondaryUpsertOperatorNodePushable extends LSMIndexInsertUpdateDeleteOperatorNodePushable {
private final PermutingFrameTupleReference prevValueTuple = new PermutingFrameTupleReference();
- private final int numberOfFields;
+ private int numberOfFields;
private AbstractIndexModificationOperationCallback abstractModCallback;
public LSMSecondaryUpsertOperatorNodePushable(IHyracksTaskContext ctx, int partition,
@@ -107,9 +109,10 @@
// if both previous value and new value are null, then we skip
tuple.reset(accessor, i);
prevValueTuple.reset(accessor, i);
- boolean isNewNull = LSMPrimaryUpsertOperatorNodePushable.isNull(tuple, 0);
- boolean isPrevValueNull = LSMPrimaryUpsertOperatorNodePushable.isNull(prevValueTuple, 0);
- if (isNewNull && isPrevValueNull) {
+ boolean isNewValueMissing = isMissing(tuple, 0);
+ boolean isOldValueMissing = isMissing(prevValueTuple, 0);
+ if (isNewValueMissing && isOldValueMissing) {
+ // No op
continue;
}
// At least, one is not null
@@ -117,21 +120,18 @@
if (equalTuples(tuple, prevValueTuple, numberOfFields)) {
continue;
}
- if (!isPrevValueNull) {
- // previous is not null, we need to delete previous
+ if (!isOldValueMissing) {
+ // We need to delete previous
abstractModCallback.setOp(Operation.DELETE);
lsmAccessor.forceDelete(prevValueTuple);
}
- if (!isNewNull) {
- // new is not null, we need to insert the new value
+ if (!isNewValueMissing) {
+ // we need to insert the new value
abstractModCallback.setOp(Operation.INSERT);
lsmAccessor.forceInsert(tuple);
}
-
- } catch (HyracksDataException e) {
- throw e;
} catch (Exception e) {
- throw new HyracksDataException(e);
+ throw HyracksDataException.create(e);
}
}
// No partial flushing was necessary. Forward entire frame.
@@ -139,4 +139,8 @@
FrameUtils.copyAndFlip(buffer, writeBuffer.getBuffer());
FrameUtils.flushFrame(writeBuffer.getBuffer(), writer);
}
+
+ private boolean isMissing(PermutingFrameTupleReference tuple, int fieldIdx) {
+ return TypeTagUtil.isType(tuple, fieldIdx, ATypeTag.SERIALIZED_MISSING_TYPE_TAG);
+ }
}