drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j..@apache.org
Subject [4/5] drill git commit: DRILL-5104: Foreman should not set external sort memory for a physical plan
Date Tue, 24 Jan 2017 06:15:24 GMT
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 <progers@maprtech.com>
Authored: Tue Dec 13 14:36:42 2016 -0800
Committer: Jinfeng Ni <jni@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);
     }
 
   }


Mime
View raw message