[SPARK-8968] [SQL] external sort by the partition clomns when dynamic partitioning to optimize the memory overhead #7336

scwf · 2015-07-10T01:48:48Z

Now the hash based writer dynamic partitioning show the bad performance for big data and cause many small files and high GC. This patch we do external sort first so that each time we only need open one writer.

before this patch:

after this patch:

scwf · 2015-07-10T01:51:49Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/InsertIntoHiveTable.scala

maybe we can add a config here to control whether to shuffle before insert

Yeah, please add a SQLConf option, and probably make it off by default.

scwf · 2015-07-10T01:52:40Z

/cc @liancheng

liancheng · 2015-07-10T02:12:50Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/InsertIntoHiveTable.scala

Please make this private.

liancheng · 2015-07-10T03:04:17Z

I'm thinking maybe we should make the change in InsertIntoHiveTable.sideEffectResult, where the writer container gets created. So that you don't bother doing a pattern match later.

Another high level comment is that, although this change does work for your workload, the following statement made in the PR description isn't correct:

This patch we shuffle data by the partition columns firstly so that each partition will have ony one partition file and this also reduce the gc overhead.

By repartitioning the dataset by dynamic partition columns, you potentially reduce the number of dynamic partitions handled per task (that's why it reduces GC overhead), but the number can't be guaranteed to be reduced to 1.

Actually we are also considering to improve dynamic partitioning insertion via local sorting (sort by partition columns with the spillable ExternalSorter). Because when writing sorted data, a task only need to open a single writer, and local sorting doesn't require shuffling.

SparkQA · 2015-07-10T03:25:25Z

Test build #36989 has finished for PR 7336 at commit 10d9f6c.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2015-07-10T03:36:55Z

+1 for local sorting. I met OOM when writing parquet with many partitions, and my first idea was also shuffle by partition column. But shuffle is expensive, local sort seems better, we may need to profile these 2 approaches.

scwf · 2015-07-10T05:15:51Z

This patch we shuffle data by the partition columns firstly so that each partition will have ony one partition file and this also reduce the gc overhead.

@liancheng here i am not mean the partition of spark rdd, i mean for each partition dir for the table such as .../a=2/b=5 there will be only one file. before this patch there maybe many small files under the partition dir.

@cloud-fan @liancheng I have tested this patch and it shows that the performance not become bad(in my situation, it improved 20%-30%).
Anyway i will also have a try to use ExternalSorter to do a local sorting and will report the performance later.

liancheng · 2015-07-10T17:10:30Z

@scwf Ah I see, these two "partition" concepts are really confusing sometimes... Although I mentioned local sorting, I do tend to also include this repartitioning optimization. But we need to add a SQLConf configuration for it first.

SparkQA · 2015-07-13T03:13:11Z

Test build #37110 has finished for PR 7336 at commit cb797de.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-07-13T05:22:00Z

Test build #37121 has finished for PR 7336 at commit b8b30d5.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

scwf · 2015-07-13T06:14:07Z

retest this please

SparkQA · 2015-07-13T07:55:04Z

Test build #37126 has finished for PR 7336 at commit b8b30d5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

liancheng · 2015-07-14T08:44:38Z

@scwf Could you please help verifying whether the HiveQL CLUSTER BY clause helps in your scenario? Essentially what CLUSTER BY does is just adding an Exchange operator, similar to the changes you made in this PR. Sorry that I didn't think of this earlier.

SparkQA · 2015-07-16T05:26:30Z

Test build #37459 has finished for PR 7336 at commit b5ada0a.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-07-16T15:02:50Z

Test build #37503 has finished for PR 7336 at commit ef4cc65.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-07-16T17:12:35Z

Test build #37506 has finished for PR 7336 at commit df19e87.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-07-16T18:16:10Z

Test build #37519 has finished for PR 7336 at commit 5420868.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

rxin · 2015-07-18T19:16:04Z

Is this related to https://issues.apache.org/jira/browse/SPARK-8890 ?

liancheng · 2015-07-19T12:06:28Z

@rxin Sort of. This PR tries to fix the same issue on the Hive support side.

scwf · 2015-07-20T07:21:53Z

@rxin, yes we found this issue when do dynamic partitioning in our case, so here i do local sort data on the partition columns to reduce the gc overhead.

scwf · 2015-07-23T06:34:18Z

@rxin any comments here?

SparkQA · 2015-08-08T10:19:36Z

Test build #40231 has finished for PR 7336 at commit c75abcb.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-08-08T17:26:24Z

Test build #40239 has finished for PR 7336 at commit aab3983.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

scwf · 2015-08-12T01:01:06Z

/cc @marmbrus can you take a look at this?

scwf · 2015-08-12T14:05:42Z

retest this please

marmbrus · 2015-08-12T17:56:47Z

This looks good, but i'd like to wait until 1.6 since we are now past the deadline and this is a pretty big change.

SparkQA · 2015-11-21T01:58:27Z

Test build #46449 timed out for PR 7336 at commit aab3983 after a configured wait of 175m.

scwf · 2016-01-10T15:19:52Z

Back to update, @marmbrus @rxin please help review this when you have time.

SparkQA · 2016-01-10T17:17:48Z

Test build #49057 has finished for PR 7336 at commit 7766247.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

rxin · 2016-01-10T19:14:04Z

Is this covering a different code path from #10638 ?

scwf · 2016-01-11T01:11:30Z

@rxin, yes, This PR try to fix the same issue on the Hive support side.

scwf · 2016-01-16T01:45:17Z

Ping @rxin

rxin · 2016-01-16T02:17:31Z

cc @cloud-fan

cloud-fan · 2016-01-19T18:11:44Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveWriterContainers.scala

is the above code duplicated in parent class?

yes, extracted a common method for it.

cloud-fan · 2016-01-19T18:12:01Z

retest this please

SparkQA · 2016-01-19T18:36:48Z

Test build #49690 has finished for PR 7336 at commit 7766247.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

…e-basedon-apachespark

scwf · 2016-01-20T15:37:05Z

retest this please

SparkQA · 2016-01-20T17:28:55Z

Test build #49789 has finished for PR 7336 at commit 7adbcca.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2016-01-21T00:57:07Z

LGTM, can you update the statistics pictures in your PR description?

rxin · 2016-01-21T01:11:51Z

I'm going to merge this. The pictures won't really show up in the commit log so it is not that big of a deal, although in the future we should make sure we update it.

yhuai · 2016-01-21T01:43:10Z

@scwf It breaks scala 2.11 build. I am going to fix it.

yhuai · 2016-01-21T01:49:58Z

Fixed by d60f8d7.

scwf · 2016-01-21T02:53:48Z

@yhuai thanks

scwf reviewed Jul 10, 2015
View reviewed changes

liancheng reviewed Jul 10, 2015
View reviewed changes

sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/InsertIntoHiveTable.scala Outdated

Copy link

Contributor

liancheng Jul 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this private.

scwf changed the title ~~[SPARK-8968] [SQL] shuffled by the partition clomns when dynamic partitioning to optimize the memory overhead~~ [WIP][SPARK-8968] [SQL] shuffled by the partition clomns when dynamic partitioning to optimize the memory overhead Jul 10, 2015

scwf changed the title ~~[WIP][SPARK-8968] [SQL] shuffled by the partition clomns when dynamic partitioning to optimize the memory overhead~~ [SPARK-8968] [SQL] shuffled by the partition clomns when dynamic partitioning to optimize the memory overhead Jul 13, 2015

scwf changed the title ~~[SPARK-8968] [SQL] shuffled by the partition clomns when dynamic partitioning to optimize the memory overhead~~ [SPARK-8968] [SQL] external sort by the partition clomns when dynamic partitioning to optimize the memory overhead Jul 16, 2015

scwf force-pushed the dynamic-optimize-basedon-apachespark branch from 5420868 to c75abcb Compare August 8, 2015 08:12

scwf added 2 commits January 9, 2016 20:22

hive side: fallback on sorting when writing many dynamic partitions

984626f

fix spark-3810 test failure

3d6338f

scwf force-pushed the dynamic-optimize-basedon-apachespark branch from aab3983 to b137a0b Compare January 10, 2016 15:13

always do sort based dynamic patition writing

7766247

scwf force-pushed the dynamic-optimize-basedon-apachespark branch from b137a0b to 7766247 Compare January 10, 2016 15:26

cloud-fan reviewed Jan 19, 2016
View reviewed changes

scwf added 3 commits January 20, 2016 23:20

extract a common method

5da7fdc

Merge branch 'master' of github.com:apache/spark into dynamic-optimiz…

3596f74

…e-basedon-apachespark

style fix

7adbcca

asfgit closed this in 015c8ef Jan 21, 2016

[SPARK-8968] [SQL] external sort by the partition clomns when dynamic partitioning to optimize the memory overhead #7336

[SPARK-8968] [SQL] external sort by the partition clomns when dynamic partitioning to optimize the memory overhead #7336

Uh oh!

Conversation

scwf commented Jul 10, 2015

Uh oh!

scwf Jul 10, 2015

Choose a reason for hiding this comment

Uh oh!

liancheng Jul 10, 2015

Choose a reason for hiding this comment

Uh oh!

scwf commented Jul 10, 2015

Uh oh!

liancheng Jul 10, 2015

Choose a reason for hiding this comment

Uh oh!

liancheng commented Jul 10, 2015

Uh oh!

SparkQA commented Jul 10, 2015

Uh oh!

cloud-fan commented Jul 10, 2015

Uh oh!

scwf commented Jul 10, 2015

Uh oh!

liancheng commented Jul 10, 2015

Uh oh!

SparkQA commented Jul 13, 2015

Uh oh!

SparkQA commented Jul 13, 2015

Uh oh!

scwf commented Jul 13, 2015

Uh oh!

SparkQA commented Jul 13, 2015

Uh oh!

liancheng commented Jul 14, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

rxin commented Jul 18, 2015

Uh oh!

liancheng commented Jul 19, 2015

Uh oh!

scwf commented Jul 20, 2015

Uh oh!

scwf commented Jul 23, 2015

Uh oh!

SparkQA commented Aug 8, 2015

Uh oh!

SparkQA commented Aug 8, 2015

Uh oh!

scwf commented Aug 12, 2015

Uh oh!

scwf commented Aug 12, 2015

Uh oh!

marmbrus commented Aug 12, 2015

Uh oh!

SparkQA commented Nov 21, 2015

Uh oh!

scwf commented Jan 10, 2016

Uh oh!

SparkQA commented Jan 10, 2016

Uh oh!

rxin commented Jan 10, 2016

Uh oh!

scwf commented Jan 11, 2016

Uh oh!

scwf commented Jan 16, 2016

Uh oh!

rxin commented Jan 16, 2016

Uh oh!

cloud-fan Jan 19, 2016

Choose a reason for hiding this comment

Uh oh!