[
https://issues.apache.org/jira/browse/DRILL-5104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15768720#comment-15768720
]
ASF GitHub Bot commented on DRILL-5104:
---------------------------------------
GitHub user paul-rogers opened a pull request:
https://github.com/apache/drill/pull/703
DRILL-5104: Foreman should not set 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.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/paul-rogers/drill DRILL-5104
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/703.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #703
----
commit 25a7f9ed45b97f9be5971fa979c1b408a6311d8e
Author: Paul Rogers <[email protected]>
Date: 2016-12-13T22:36:42Z
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.
----
> Foreman sets external sort memory allocation even for a physical plan
> ---------------------------------------------------------------------
>
> Key: DRILL-5104
> URL: https://issues.apache.org/jira/browse/DRILL-5104
> Project: Apache Drill
> Issue Type: Sub-task
> Affects Versions: 1.8.0
> Reporter: Paul Rogers
> Assignee: Paul Rogers
>
> Consider the (disabled) unit test
> {{TestSimpleExternalSort.outOfMemoryExternalSort}} which uses the physical
> plan {{xsort/oom_sort_test.json}} that contains a setting for the amount of
> memory to allocate:
> {code}
> {
> ...
> pop:"external-sort",
> ...
> initialAllocation: 1000000,
> maxAllocation: 30000000
> },
> {code}
> When run, the amount of memory is set to 715827882. The reason is that code
> was added to {{Foreman}} to compute the memory to allocate to the external
> sort:
> {code}
> private void runPhysicalPlan(final PhysicalPlan plan) throws
> ExecutionSetupException {
> validatePlan(plan);
> MemoryAllocationUtilities.setupSortMemoryAllocations(plan, queryContext);
> {code}
> The problem is that a physical plan should execute as provided to enable
> detailed testing.
> To solve this problem, move the sort memory setup to the path taken by SQL
> queries, but not via physical plans.
> This change is necessary to re-enable the previously-disabled external sort
> tests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
