Steven Jacobs has submitted this change and it was merged. Change subject: [ASTERIXDB-2332][RT] Fix concurrency issue with RecordMerge and RecordRemoveFields ......................................................................
[ASTERIXDB-2332][RT] Fix concurrency issue with RecordMerge and RecordRemoveFields - user model changes: no - storage format changes: no - interface changes: no Details: Removed shared instance of PointableHelper.STRING_BINARY_COMPARATOR which was not thread safe. Change-Id: I4c5db58184235474f7aed7e5a4b91b6b5685fc06 Reviewed-on: https://asterix-gerrit.ics.uci.edu/2492 Sonar-Qube: Jenkins <[email protected]> Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Contrib: Jenkins <[email protected]> Reviewed-by: Dmitry Lychagin <[email protected]> --- M asterixdb/asterix-app/src/test/resources/runtimets/queries/objects/ObjectsQueries.xml M asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableHelper.java M asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeDescriptor.java M asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordRemoveFieldsEvalFactory.java 4 files changed, 23 insertions(+), 15 deletions(-) Approvals: Anon. E. Moose #1000171: Jenkins: Verified; No violations found; ; Verified Dmitry Lychagin: Looks good to me, approved diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries/objects/ObjectsQueries.xml b/asterixdb/asterix-app/src/test/resources/runtimets/queries/objects/ObjectsQueries.xml index c07eb24..44eb244 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/queries/objects/ObjectsQueries.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries/objects/ObjectsQueries.xml @@ -162,11 +162,11 @@ <output-dir compare="Text">highly-nested-open</output-dir> </compilation-unit> </test-case> - <!--test-case FilePath="objects/object-remove-fields"> + <test-case FilePath="objects/object-remove-fields"> <compilation-unit name="documentation-example"> <output-dir compare="Text">documentation-example</output-dir> </compilation-unit> - </test-case--> + </test-case> <test-case FilePath="objects/object-remove-fields"> <compilation-unit name="tiny-social-example"> <output-dir compare="Text">tiny-social-example</output-dir> diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableHelper.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableHelper.java index 1c69d4c..004e50a 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableHelper.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/PointableHelper.java @@ -44,22 +44,26 @@ private static final byte[] NULL_BYTES = new byte[] { ATypeTag.SERIALIZED_NULL_TYPE_TAG }; - private static final IBinaryComparator STRING_BINARY_COMPARATOR = - PointableBinaryComparatorFactory.of(UTF8StringPointable.FACTORY).createBinaryComparator(); private final UTF8StringWriter utf8Writer; public PointableHelper() { utf8Writer = new UTF8StringWriter(); } - public static int compareStringBinValues(IValueReference a, IValueReference b) throws HyracksDataException { - // start+1 and len-1 due to type tag ignore (only interested in String value) - return STRING_BINARY_COMPARATOR.compare(a.getByteArray(), a.getStartOffset() + 1, a.getLength() - 1, - b.getByteArray(), b.getStartOffset() + 1, b.getLength() - 1); + public static IBinaryComparator createStringBinaryComparator() { + return PointableBinaryComparatorFactory.of(UTF8StringPointable.FACTORY).createBinaryComparator(); } - public static boolean isEqual(IValueReference a, IValueReference b) throws HyracksDataException { - return (compareStringBinValues(a, b) == 0); + public static int compareStringBinValues(IValueReference a, IValueReference b, IBinaryComparator comparator) + throws HyracksDataException { + // start+1 and len-1 due to type tag ignore (only interested in String value) + return comparator.compare(a.getByteArray(), a.getStartOffset() + 1, a.getLength() - 1, b.getByteArray(), + b.getStartOffset() + 1, b.getLength() - 1); + } + + public static boolean isEqual(IValueReference a, IValueReference b, IBinaryComparator comparator) + throws HyracksDataException { + return compareStringBinValues(a, b, comparator) == 0; } public static boolean byteArrayEqual(IValueReference valueRef1, IValueReference valueRef2) { diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeDescriptor.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeDescriptor.java index 9ff670f..7a4522f 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeDescriptor.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeDescriptor.java @@ -47,6 +47,7 @@ import org.apache.hyracks.algebricks.runtime.base.IScalarEvaluator; import org.apache.hyracks.algebricks.runtime.base.IScalarEvaluatorFactory; import org.apache.hyracks.api.context.IHyracksTaskContext; +import org.apache.hyracks.api.dataflow.value.IBinaryComparator; import org.apache.hyracks.api.exceptions.HyracksDataException; import org.apache.hyracks.data.std.api.IPointable; import org.apache.hyracks.data.std.primitive.VoidPointable; @@ -108,6 +109,7 @@ final List<RecordBuilder> rbStack = new ArrayList<>(); final ArrayBackedValueStorage tabvs = new ArrayBackedValueStorage(); + final IBinaryComparator stringBinaryComparator = PointableHelper.createStringBinaryComparator(); return new IScalarEvaluator() { @@ -158,7 +160,7 @@ IVisitablePointable rightValue = rightRecord.getFieldValues().get(j); IVisitablePointable rightType = rightRecord.getFieldTypeTags().get(j); // Check if same fieldname - if (PointableHelper.isEqual(leftName, rightName) + if (PointableHelper.isEqual(leftName, rightName, stringBinaryComparator) && !deepEqualAssesor.isEqual(leftValue, rightValue)) { //Field was found on the right and are subrecords, merge them if (PointableHelper.sameType(ATypeTag.OBJECT, rightType) diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordRemoveFieldsEvalFactory.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordRemoveFieldsEvalFactory.java index 192a14b..f10fb59 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordRemoveFieldsEvalFactory.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordRemoveFieldsEvalFactory.java @@ -27,8 +27,6 @@ import java.util.List; import org.apache.asterix.builders.RecordBuilder; -import org.apache.asterix.common.exceptions.AsterixException; -import org.apache.asterix.runtime.exceptions.TypeMismatchException; import org.apache.asterix.om.functions.BuiltinFunctions; import org.apache.asterix.om.pointables.AListVisitablePointable; import org.apache.asterix.om.pointables.ARecordVisitablePointable; @@ -40,9 +38,11 @@ import org.apache.asterix.om.types.ATypeTag; import org.apache.asterix.om.types.runtime.RuntimeRecordTypeInfo; import org.apache.asterix.runtime.evaluators.functions.PointableHelper; +import org.apache.asterix.runtime.exceptions.TypeMismatchException; import org.apache.hyracks.algebricks.runtime.base.IScalarEvaluator; import org.apache.hyracks.algebricks.runtime.base.IScalarEvaluatorFactory; import org.apache.hyracks.api.context.IHyracksTaskContext; +import org.apache.hyracks.api.dataflow.value.IBinaryComparator; import org.apache.hyracks.api.exceptions.HyracksDataException; import org.apache.hyracks.data.std.api.IPointable; import org.apache.hyracks.data.std.primitive.VoidPointable; @@ -78,6 +78,7 @@ final IPointable inputArg1 = new VoidPointable(); final IScalarEvaluator eval0 = inputRecordEvalFactory.createScalarEvaluator(ctx); final IScalarEvaluator eval1 = removeFieldPathsFactory.createScalarEvaluator(ctx); + final IBinaryComparator stringBinaryComparator = PointableHelper.createStringBinaryComparator(); return new IScalarEvaluator() { private final RuntimeRecordTypeInfo runtimeRecordTypeInfo = new RuntimeRecordTypeInfo(); @@ -197,7 +198,8 @@ boolean match = true; Iterator<IVisitablePointable> fpi = recordPath.iterator(); for (int j = inputPathItems.size() - 1; j >= 0; j--) { - match &= PointableHelper.isEqual(inputPathItems.get(j), fpi.next()); + match &= PointableHelper.isEqual(inputPathItems.get(j), fpi.next(), + stringBinaryComparator); if (!match) { break; } @@ -207,7 +209,7 @@ } } } else { - if (PointableHelper.isEqual(recordPath.getFirst(), item)) { + if (PointableHelper.isEqual(recordPath.getFirst(), item, stringBinaryComparator)) { return false; } } -- To view, visit https://asterix-gerrit.ics.uci.edu/2492 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4c5db58184235474f7aed7e5a4b91b6b5685fc06 Gerrit-PatchSet: 9 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Steven Jacobs <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Steven Jacobs <[email protected]> Gerrit-Reviewer: Xikui Wang <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]>
