[
https://issues.apache.org/jira/browse/DRILL-6435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16489869#comment-16489869
]
ASF GitHub Bot commented on DRILL-6435:
---------------------------------------
Ben-Zvi closed pull request #1286: DRILL-6435: MappingSet is stateful, so it
can't be shared between threads
URL: https://github.com/apache/drill/pull/1286
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java
index 39297144cd..366e4e8888 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java
@@ -72,9 +72,9 @@
public class TopNBatch extends AbstractRecordBatch<TopN> {
static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(TopNBatch.class);
- public final MappingSet mainMapping = createMainMappingSet();
- public final MappingSet leftMapping = createLeftMappingSet();
- public final MappingSet rightMapping = createRightMappingSet();
+ private final MappingSet mainMapping = createMainMappingSet();
+ private final MappingSet leftMapping = createLeftMappingSet();
+ private final MappingSet rightMapping = createRightMappingSet();
private final int batchPurgeThreshold;
private final boolean codegenDump;
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java
index 9713b70d29..a5c2ae7318 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java
@@ -73,23 +73,23 @@
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(MergeJoinBatch.class);
- public final MappingSet setupMapping =
+ private final MappingSet setupMapping =
new MappingSet("null", "null",
GM("doSetup", "doSetup", null, null),
GM("doSetup", "doSetup", null, null));
- public final MappingSet copyLeftMapping =
+ private final MappingSet copyLeftMapping =
new MappingSet("leftIndex", "outIndex",
GM("doSetup", "doSetup", null, null),
GM("doSetup", "doCopyLeft", null, null));
- public final MappingSet copyRightMappping =
+ private final MappingSet copyRightMappping =
new MappingSet("rightIndex", "outIndex",
GM("doSetup", "doSetup", null, null),
GM("doSetup", "doCopyRight", null, null));
- public final MappingSet compareMapping =
+ private final MappingSet compareMapping =
new MappingSet("leftIndex", "rightIndex",
GM("doSetup", "doSetup", null, null),
GM("doSetup", "doCompare", null, null));
- public final MappingSet compareRightMapping =
+ private final MappingSet compareRightMapping =
new MappingSet("rightIndex", "null",
GM("doSetup", "doSetup", null, null),
GM("doSetup", "doCompare", null, null));
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java
index 03bf58b36b..ae14fb3ec9 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java
@@ -111,17 +111,17 @@
// Mapping set for the right side
- private static final MappingSet emitRightMapping =
+ private final MappingSet emitRightMapping =
new MappingSet("rightCompositeIndex" /* read index */, "outIndex" /*
write index */, "rightContainer" /* read container */,
"outgoing" /* write container */, EMIT_RIGHT_CONSTANT, EMIT_RIGHT);
// Mapping set for the left side
- private static final MappingSet emitLeftMapping = new MappingSet("leftIndex"
/* read index */, "outIndex" /* write index */,
+ private final MappingSet emitLeftMapping = new MappingSet("leftIndex" /*
read index */, "outIndex" /* write index */,
"leftBatch" /* read container */,
"outgoing" /* write container */,
EMIT_LEFT_CONSTANT, EMIT_LEFT);
- private static final MappingSet SETUP_LEFT_MAPPING = new
MappingSet("leftIndex" /* read index */, "outIndex" /* write index */,
+ private final MappingSet SETUP_LEFT_MAPPING = new MappingSet("leftIndex" /*
read index */, "outIndex" /* write index */,
"leftBatch" /* read container */,
"outgoing" /* write container */,
ClassGenerator.DEFAULT_CONSTANT_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java
index 090ca58c05..a9cbfdf60f 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java
@@ -17,36 +17,15 @@
*/
package org.apache.drill.exec.physical.impl.mergereceiver;
-import static org.apache.drill.exec.compile.sig.GeneratorMapping.GM;
-
import org.apache.drill.exec.compile.TemplateClassDefinition;
-import org.apache.drill.exec.compile.sig.MappingSet;
import org.apache.drill.exec.exception.SchemaChangeException;
import org.apache.drill.exec.ops.FragmentContext;
import org.apache.drill.exec.record.VectorAccessible;
public interface MergingReceiverGeneratorBase {
+ TemplateClassDefinition<MergingReceiverGeneratorBase> TEMPLATE_DEFINITION =
new TemplateClassDefinition<>(MergingReceiverGeneratorBase.class,
MergingReceiverTemplate.class);
- public abstract void doSetup(FragmentContext context,
- VectorAccessible incoming,
- VectorAccessible outgoing) throws
SchemaChangeException;
-
- public abstract int doEval(int leftIndex,
- int rightIndex) throws SchemaChangeException;
-
- public abstract void doCopy(int inIndex, int outIndex) throws
SchemaChangeException;
-
- public static TemplateClassDefinition<MergingReceiverGeneratorBase>
TEMPLATE_DEFINITION =
- new TemplateClassDefinition<>(MergingReceiverGeneratorBase.class,
MergingReceiverTemplate.class);
-
- public final MappingSet compareMapping =
- new MappingSet("leftIndex", "rightIndex",
- GM("doSetup", "doCompare", null, null),
- GM("doSetup", "doCompare", null, null));
-
- public final MappingSet copyMapping =
- new MappingSet("inIndex", "outIndex",
- GM("doSetup", "doCopy", null, null),
- GM("doSetup", "doCopy", null, null));
-
+ void doSetup(FragmentContext context, VectorAccessible incoming,
VectorAccessible outgoing) throws SchemaChangeException;
+ int doEval(int leftIndex, int rightIndex) throws SchemaChangeException;
+ void doCopy(int inIndex, int outIndex) throws SchemaChangeException;
}
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
index 9087757b0a..b9291cb07e 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
@@ -738,11 +738,11 @@ private MergingReceiverGeneratorBase createMerger()
throws SchemaChangeException
}
}
- public final MappingSet MAIN_MAPPING = new MappingSet( (String) null, null,
ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
- public final MappingSet LEFT_MAPPING = new MappingSet("leftIndex", null,
ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
- public final MappingSet RIGHT_MAPPING = new MappingSet("rightIndex", null,
ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
- GeneratorMapping COPIER_MAPPING = new GeneratorMapping("doSetup", "doCopy",
null, null);
- public final MappingSet COPIER_MAPPING_SET = new MappingSet(COPIER_MAPPING,
COPIER_MAPPING);
+ private final MappingSet MAIN_MAPPING = new MappingSet( (String) null, null,
ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
+ private final MappingSet LEFT_MAPPING = new MappingSet("leftIndex", null,
ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
+ private final MappingSet RIGHT_MAPPING = new MappingSet("rightIndex", null,
ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
+ private GeneratorMapping COPIER_MAPPING = new GeneratorMapping("doSetup",
"doCopy", null, null);
+ private final MappingSet COPIER_MAPPING_SET = new MappingSet(COPIER_MAPPING,
COPIER_MAPPING);
private void generateComparisons(final ClassGenerator<?> g, final
VectorAccessible batch) throws SchemaChangeException {
g.setMappingSet(MAIN_MAPPING);
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java
index 9e82af8bb8..8c5c0a4ccc 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java
@@ -101,11 +101,11 @@
.mode(SerializationMode.DRILL_SERIALIZIABLE) //
.build();
- public final MappingSet mainMapping = new MappingSet( (String) null, null,
ClassGenerator.DEFAULT_CONSTANT_MAP,
+ private final MappingSet mainMapping = new MappingSet( (String) null, null,
ClassGenerator.DEFAULT_CONSTANT_MAP,
ClassGenerator.DEFAULT_SCALAR_MAP);
- public final MappingSet incomingMapping = new MappingSet("inIndex", null,
"incoming", null,
+ private final MappingSet incomingMapping = new MappingSet("inIndex", null,
"incoming", null,
ClassGenerator.DEFAULT_CONSTANT_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
- public final MappingSet partitionMapping = new MappingSet("partitionIndex",
null, "partitionVectors", null,
+ private final MappingSet partitionMapping = new MappingSet("partitionIndex",
null, "partitionVectors", null,
ClassGenerator.DEFAULT_CONSTANT_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
private final int recordsToSample; // How many records must be received
before analyzing
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java
index f38b62e3ad..fc49d43832 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java
@@ -50,9 +50,9 @@
public class SortBatch extends AbstractRecordBatch<Sort> {
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(SortBatch.class);
- public final MappingSet mainMapping = new MappingSet( (String) null, null,
ClassGenerator.DEFAULT_CONSTANT_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
- public final MappingSet leftMapping = new MappingSet("leftIndex", null,
ClassGenerator.DEFAULT_CONSTANT_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
- public final MappingSet rightMapping = new MappingSet("rightIndex", null,
ClassGenerator.DEFAULT_CONSTANT_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
+ private final MappingSet mainMapping = new MappingSet( (String) null, null,
ClassGenerator.DEFAULT_CONSTANT_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
+ private final MappingSet leftMapping = new MappingSet("leftIndex", null,
ClassGenerator.DEFAULT_CONSTANT_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
+ private final MappingSet rightMapping = new MappingSet("rightIndex", null,
ClassGenerator.DEFAULT_CONSTANT_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
private final RecordBatch incoming;
private final SortRecordBatchBuilder builder;
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
index 9cde1a552f..0f3f8a3434 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
@@ -88,10 +88,10 @@
private static final ControlsInjector injector =
ControlsInjectorFactory.getInjector(ExternalSortBatch.class);
private static final GeneratorMapping COPIER_MAPPING = new
GeneratorMapping("doSetup", "doCopy", null, null);
- private static final MappingSet MAIN_MAPPING = new MappingSet( (String)
null, null, ClassGenerator.DEFAULT_SCALAR_MAP,
ClassGenerator.DEFAULT_SCALAR_MAP);
- private static final MappingSet LEFT_MAPPING = new MappingSet("leftIndex",
null, ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
- private static final MappingSet RIGHT_MAPPING = new MappingSet("rightIndex",
null, ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
- private static final MappingSet COPIER_MAPPING_SET = new
MappingSet(COPIER_MAPPING, COPIER_MAPPING);
+ private final MappingSet MAIN_MAPPING = new MappingSet( (String) null, null,
ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
+ private final MappingSet LEFT_MAPPING = new MappingSet("leftIndex", null,
ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
+ private final MappingSet RIGHT_MAPPING = new MappingSet("rightIndex", null,
ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
+ private final MappingSet COPIER_MAPPING_SET = new MappingSet(COPIER_MAPPING,
COPIER_MAPPING);
private final int SPILL_BATCH_GROUP_SIZE;
private final int SPILL_THRESHOLD;
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/managed/BaseSortWrapper.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/managed/BaseSortWrapper.java
index 338462ec69..d24008045c 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/managed/BaseSortWrapper.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/managed/BaseSortWrapper.java
@@ -41,9 +41,9 @@
public abstract class BaseSortWrapper extends BaseWrapper {
- protected static final MappingSet MAIN_MAPPING = new MappingSet((String)
null, null, ClassGenerator.DEFAULT_SCALAR_MAP,
ClassGenerator.DEFAULT_SCALAR_MAP);
- protected static final MappingSet LEFT_MAPPING = new MappingSet("leftIndex",
null, ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
- protected static final MappingSet RIGHT_MAPPING = new
MappingSet("rightIndex", null, ClassGenerator.DEFAULT_SCALAR_MAP,
ClassGenerator.DEFAULT_SCALAR_MAP);
+ protected final MappingSet MAIN_MAPPING = new MappingSet((String) null,
null, ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
+ protected final MappingSet LEFT_MAPPING = new MappingSet("leftIndex", null,
ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
+ protected final MappingSet RIGHT_MAPPING = new MappingSet("rightIndex",
null, ClassGenerator.DEFAULT_SCALAR_MAP, ClassGenerator.DEFAULT_SCALAR_MAP);
public BaseSortWrapper(OperatorContext opContext) {
super(opContext);
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/managed/PriorityQueueCopierWrapper.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/managed/PriorityQueueCopierWrapper.java
index dda42a2d82..9e13923c45 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/managed/PriorityQueueCopierWrapper.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/managed/PriorityQueueCopierWrapper.java
@@ -56,7 +56,7 @@
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(PriorityQueueCopierWrapper.class);
private static final GeneratorMapping COPIER_MAPPING = new
GeneratorMapping("doSetup", "doCopy", null, null);
- private static final MappingSet COPIER_MAPPING_SET = new
MappingSet(COPIER_MAPPING, COPIER_MAPPING);
+ private final MappingSet COPIER_MAPPING_SET = new MappingSet(COPIER_MAPPING,
COPIER_MAPPING);
/**
* A single PriorityQueueCopier instance is used for 2 purposes:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> MappingSet is stateful, so it can't be shared between threads
> -------------------------------------------------------------
>
> Key: DRILL-6435
> URL: https://issues.apache.org/jira/browse/DRILL-6435
> Project: Apache Drill
> Issue Type: Bug
> Reporter: Vlad Rozov
> Assignee: Vlad Rozov
> Priority: Major
> Labels: ready-to-commit
> Fix For: 1.14.0
>
>
> There are several instances where static {{MappingSet}} instances are used
> (for example {{NestedLoopJoinBatch}} and {{BaseSortWrapper}}). This causes
> instance reuse across threads when queries are executed concurrently. As
> {{MappingSet}} is a stateful class with visitor design pattern, such reuse
> causes invalid state.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
