You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by jn...@apache.org on 2017/01/24 06:15:24 UTC
[4/5] drill git commit: DRILL-5104: Foreman should not set external
sort memory for a physical plan
DRILL-5104: Foreman should not set external sort memory for a physical plan
Physical plans include a plan for memory allocations. However, the code
path in Foreman replans external sort memory, even for a physical plan.
This makes it impossible to use a physical plan to test memory
configuration.
This change avoids changing memory settings in a physical plan; while
preserving the adjustments for logical plans or SQL queries.
Revised to put a property in the plan itself. Old plans, and those
generated from SQL, will have memory allocations applied. Plans
marked as already "resource management" planned will be used as-is.
Includes a unit test that demonstrates the new behavior.
close apache/drill#703
Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/82176976
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/82176976
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/82176976
Branch: refs/heads/master
Commit: 8217697647e7ed10c52cfa91b860730302a339e8
Parents: 08ca5e0
Author: Paul Rogers <pr...@maprtech.com>
Authored: Tue Dec 13 14:36:42 2016 -0800
Committer: Jinfeng Ni <jn...@apache.org>
Committed: Mon Jan 23 17:08:39 2017 -0800
----------------------------------------------------------------------
.../apache/drill/exec/physical/PhysicalPlan.java | 3 ---
.../exec/util/MemoryAllocationUtilities.java | 9 ++++++++-
.../apache/drill/exec/work/foreman/Foreman.java | 1 +
.../impl/xsort/TestSimpleExternalSort.java | 6 ++++--
.../resources/xsort/one_key_sort_descending.json | 3 ++-
.../drill/common/logical/PlanProperties.java | 18 ++++++++++++++++--
6 files changed, 31 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/drill/blob/82176976/exec/java-exec/src/main/java/org/apache/drill/exec/physical/PhysicalPlan.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/PhysicalPlan.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/PhysicalPlan.java
index 78b882b..e0902c8 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/PhysicalPlan.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/PhysicalPlan.java
@@ -62,10 +62,8 @@ public class PhysicalPlan {
}else{
return list;
}
-
}
-
@JsonProperty("head")
public PlanProperties getProperties() {
return properties;
@@ -89,5 +87,4 @@ public class PhysicalPlan {
throw new RuntimeException(e);
}
}
-
}
http://git-wip-us.apache.org/repos/asf/drill/blob/82176976/exec/java-exec/src/main/java/org/apache/drill/exec/util/MemoryAllocationUtilities.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/util/MemoryAllocationUtilities.java b/exec/java-exec/src/main/java/org/apache/drill/exec/util/MemoryAllocationUtilities.java
index 38dfcd0..678167f 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/util/MemoryAllocationUtilities.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/util/MemoryAllocationUtilities.java
@@ -41,6 +41,13 @@ public class MemoryAllocationUtilities {
* @param queryContext
*/
public static void setupSortMemoryAllocations(final PhysicalPlan plan, final QueryContext queryContext) {
+
+ // Test plans may already have a pre-defined memory plan.
+ // Otherwise, determine memory allocation.
+
+ if (plan.getProperties().hasResourcePlan) {
+ return;
+ }
// look for external sorts
final List<ExternalSort> sortList = new LinkedList<>();
for (final PhysicalOperator op : plan.getSortedOperators()) {
@@ -64,6 +71,6 @@ public class MemoryAllocationUtilities {
externalSort.setMaxAllocation(maxSortAlloc);
}
}
+ plan.getProperties().hasResourcePlan = true;
}
-
}
http://git-wip-us.apache.org/repos/asf/drill/blob/82176976/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java b/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
index c6a3104..30718b6 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
@@ -774,6 +774,7 @@ public class Foreman implements Runnable {
}
}
+ @SuppressWarnings("resource")
@Override
public void close() {
Preconditions.checkState(!isClosed);
http://git-wip-us.apache.org/repos/asf/drill/blob/82176976/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestSimpleExternalSort.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestSimpleExternalSort.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestSimpleExternalSort.java
index b34a466..85975cb 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestSimpleExternalSort.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestSimpleExternalSort.java
@@ -42,7 +42,6 @@ import org.junit.rules.TestRule;
import com.google.common.base.Charsets;
import com.google.common.io.Files;
-@Ignore
public class TestSimpleExternalSort extends BaseTestQuery {
static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestSimpleExternalSort.class);
DrillConfig c = DrillConfig.create();
@@ -50,6 +49,7 @@ public class TestSimpleExternalSort extends BaseTestQuery {
@Rule public final TestRule TIMEOUT = TestTools.getTimeoutRule(80000);
+ @Ignore
@Test
public void mergeSortWithSv2() throws Exception {
List<QueryDataBatch> results = testPhysicalFromFileWithResults("xsort/one_key_sort_descending_sv2.json");
@@ -109,7 +109,7 @@ public class TestSimpleExternalSort extends BaseTestQuery {
for (QueryDataBatch b : results) {
if (b.getHeader().getRowCount() == 0) {
- break;
+ continue;
}
batchCount++;
RecordBatchLoader loader = new RecordBatchLoader(allocator);
@@ -132,6 +132,7 @@ public class TestSimpleExternalSort extends BaseTestQuery {
}
@Test
+ @Ignore
public void sortOneKeyDescendingExternalSort() throws Throwable{
RemoteServiceSet serviceSet = RemoteServiceSet.getLocalServiceSet();
@@ -186,6 +187,7 @@ public class TestSimpleExternalSort extends BaseTestQuery {
}
@Test
+ @Ignore
public void outOfMemoryExternalSort() throws Throwable{
RemoteServiceSet serviceSet = RemoteServiceSet.getLocalServiceSet();
http://git-wip-us.apache.org/repos/asf/drill/blob/82176976/exec/java-exec/src/test/resources/xsort/one_key_sort_descending.json
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/xsort/one_key_sort_descending.json b/exec/java-exec/src/test/resources/xsort/one_key_sort_descending.json
index f4eab5d..b4794ad 100644
--- a/exec/java-exec/src/test/resources/xsort/one_key_sort_descending.json
+++ b/exec/java-exec/src/test/resources/xsort/one_key_sort_descending.json
@@ -4,7 +4,8 @@
version:"1",
generator:{
type:"manual"
- }
+ },
+ hasResourcePlan: true
},
graph:[
{
http://git-wip-us.apache.org/repos/asf/drill/blob/82176976/logical/src/main/java/org/apache/drill/common/logical/PlanProperties.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/logical/PlanProperties.java b/logical/src/main/java/org/apache/drill/common/logical/PlanProperties.java
index ce9603e..f4de0eb 100644
--- a/logical/src/main/java/org/apache/drill/common/logical/PlanProperties.java
+++ b/logical/src/main/java/org/apache/drill/common/logical/PlanProperties.java
@@ -35,6 +35,12 @@ public class PlanProperties {
public JSONOptions options;
public int queue;
+ /**
+ * Indicates if the plan has been planned for resource management
+ * (memory, etc.) or if this plan must still be computed.
+ */
+ public boolean hasResourcePlan;
+
// @JsonInclude(Include.NON_NULL)
public static class Generator {
public String type;
@@ -55,7 +61,8 @@ public class PlanProperties {
@JsonProperty("type") PlanType type,
@JsonProperty("mode") ResultMode resultMode,
@JsonProperty("options") JSONOptions options,
- @JsonProperty("queue") int queue
+ @JsonProperty("queue") int queue,
+ @JsonProperty("hasResourcePlan") boolean hasResourcePlan
) {
this.version = version;
this.queue = queue;
@@ -63,6 +70,7 @@ public class PlanProperties {
this.type = type;
this.resultMode = resultMode == null ? ResultMode.EXEC : resultMode;
this.options = options;
+ this.hasResourcePlan = hasResourcePlan;
}
public static PlanPropertiesBuilder builder() {
@@ -76,6 +84,7 @@ public class PlanProperties {
private ResultMode mode = ResultMode.EXEC;
private JSONOptions options;
private int queueNumber = 0;
+ private boolean hasResourcePlan = false;
public PlanPropertiesBuilder type(PlanType type) {
this.type = type;
@@ -112,8 +121,13 @@ public class PlanProperties {
return this;
}
+ public PlanPropertiesBuilder generator(boolean hasResourcePlan) {
+ this.hasResourcePlan = hasResourcePlan;
+ return this;
+ }
+
public PlanProperties build() {
- return new PlanProperties(version, generator, type, mode, options, queueNumber);
+ return new PlanProperties(version, generator, type, mode, options, queueNumber, hasResourcePlan);
}
}