[SPARK-32350][CORE] Add batch-write on LevelDB to improve performance of HybridStore by baohe-zhang · Pull Request #29149 · apache/spark

baohe-zhang · 2020-07-17T17:10:48Z

What changes were proposed in this pull request?

The idea is to improve the performance of HybridStore by adding batch write support to LevelDB. #28412 introduces HybridStore. HybridStore will write data to InMemoryStore at first and use a background thread to dump data to LevelDB once the writing to InMemoryStore is completed. In the comments section of #28412 , @mridulm mentioned using batch writing can improve the performance of this dumping process and he wrote the code of writeAll().

Why are the changes needed?

I did the comparison of the HybridStore switching time between one-by-one write and batch write on an HDD disk. When the disk is free, the batch-write has around 25% improvement, and when the disk is 100% busy, the batch-write has 7x - 10x improvement.

when the disk is at 0% utilization:

log size, jobs and tasks per job	original switching time, with write()	switching time with writeAll()
133m, 400 jobs, 100 tasks per job	16s	13s
265m, 400 jobs, 200 tasks per job	30s	23s
1.3g, 1000 jobs, 400 tasks per job	136s	108s

when the disk is at 100% utilization:

log size, jobs and tasks per job	original switching time, with write()	switching time with writeAll()
133m, 400 jobs, 100 tasks per job	116s	17s
265m, 400 jobs, 200 tasks per job	251s	26s

I also ran some write related benchmarking tests on LevelDBBenchmark.java and measured the total time of writing 1024 objects. The tests were conducted when the disk is at 0% utilization.

Benchmark test	with write(), ms	with writeAll(), ms
randomUpdatesIndexed	213.06	157.356
randomUpdatesNoIndex	57.869	35.439
randomWritesIndexed	298.854	229.274
randomWritesNoIndex	66.764	38.361
sequentialUpdatesIndexed	87.019	56.219
sequentialUpdatesNoIndex	61.851	41.942
sequentialWritesIndexed	94.044	56.534
sequentialWritesNoIndex	118.345	66.483

Does this PR introduce any user-facing change?

No

How was this patch tested?

Manually tested.

baohe-zhang · 2020-07-17T17:16:53Z

cc @HeartSaVioR @mridulm @tgravescs ^^

HeartSaVioR · 2020-07-18T00:58:04Z

common/kvstore/src/main/java/org/apache/spark/util/kvstore/LevelDB.java

+
+        try (WriteBatch batch = db().createWriteBatch()) {
+          while (valueIter.hasNext()) {
+            final Object value = valueIter.next();


Adding one value (L204-L219) looks to be same with write() - let's extract and deduplicate.

HeartSaVioR · 2020-07-18T01:36:42Z

core/src/main/scala/org/apache/spark/deploy/history/HybridStore.scala

-          while (it.hasNext()) {
-            levelDB.write(it.next())
-          }
+          val values = Lists.newArrayList(


This would be OK, given all entries are from inMemoryStore which are already materialized into memory.

HeartSaVioR

Looks OK in general. Just a minor comment. I'd like to wait for others to review as well if it doesn't hold too long.

HeartSaVioR · 2020-07-19T10:05:52Z

ok to test

HeartSaVioR · 2020-07-19T10:10:50Z

add to whitelist

SparkQA · 2020-07-19T12:26:25Z

Test build #126130 has finished for PR 29149 at commit a12e178.

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

SparkQA · 2020-07-19T12:31:05Z

Test build #126129 has finished for PR 29149 at commit a12e178.

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

HeartSaVioR · 2020-07-21T01:19:30Z

@mridulm @tgravescs
I'm planning to merge this in tomorrow. Please comment if you'd like to have time to review. Thanks!

tgravescs · 2020-07-21T14:43:43Z

I likely won't have time for a review so go ahead without mine

HeartSaVioR · 2020-07-21T20:40:32Z

OK I'll go ahead merging. To be sure I'll trigger test once again.

HeartSaVioR · 2020-07-21T20:40:39Z

retest this, please

SparkQA · 2020-07-21T22:53:50Z

Test build #126278 has finished for PR 29149 at commit a12e178.

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

HeartSaVioR · 2020-07-21T23:06:37Z

retest this, please

SparkQA · 2020-07-22T01:20:40Z

Test build #126284 has finished for PR 29149 at commit a12e178.

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

HeartSaVioR · 2020-07-22T01:26:28Z

retest this, please

SparkQA · 2020-07-22T04:24:42Z

Test build #126290 has finished for PR 29149 at commit a12e178.

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

HeartSaVioR · 2020-07-22T04:26:54Z

Thanks! Merging to master.

baohe-zhang · 2020-07-22T05:03:53Z

Thanks for the review!

mridulm · 2020-08-10T19:44:38Z

Sorry for the delay in getting to this.
The only additional change I had made in my version was to "batch the batch update".
For example, if the list is very large, the write will block all other threads for a very long time - which has fairness issues; in addition to the serialization consuming too much memory, etc.
Instead, I batched updates to some K values (In my case K was 128).

This can be trivially done with an inner loop doing for (List<?> batchList : Iterables.partition(entry.getValue(), batchSize)) { ... }

The performance actually improves for larger list sizes (due to memory pressure reducing - particularly in SHS), while the smaller lists suffer from minimal impact

baohe-zhang · 2020-08-10T20:51:33Z

This seems an important improvement. Should I put up a followup PR to include this change?

mridulm · 2020-08-11T07:01:46Z

That would be great, thanks @baohe-zhang !

Add writeAll() on LevelDB and use it on HybridStore

58023ff

probot-autolabeler bot added the CORE label Jul 17, 2020

baohe-zhang changed the title ~~[SPARK-32350] Add batch-write on LevelDB to improve performance of HybridStore~~ [SPARK-32350][CORE] Add batch-write on LevelDB to improve performance of HybridStore Jul 17, 2020

HeartSaVioR reviewed Jul 18, 2020

View reviewed changes

Extract the duplicate code to a new method

a12e178

HeartSaVioR closed this in 7b9d755 Jul 22, 2020

baohe-zhang mentioned this pull request Aug 13, 2020

[SPARK-32350][FOLLOW-UP] Fix count update issue and partition the value list to a set of small batches for LevelDB writeAll #29425

Closed

mridulm mentioned this pull request Jan 14, 2025

[SPARK-50808][CORE] Fix issue in writeAll with mixed types not getting written properly #49479

Closed

Conversation

baohe-zhang commented Jul 17, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

baohe-zhang commented Jul 17, 2020

Uh oh!

HeartSaVioR Jul 18, 2020

Choose a reason for hiding this comment

Uh oh!

baohe-zhang Jul 19, 2020

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR Jul 18, 2020

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR left a comment

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR commented Jul 19, 2020

Uh oh!

HeartSaVioR commented Jul 19, 2020

Uh oh!

SparkQA commented Jul 19, 2020

Uh oh!

SparkQA commented Jul 19, 2020

Uh oh!

HeartSaVioR commented Jul 21, 2020

Uh oh!

tgravescs commented Jul 21, 2020

Uh oh!

HeartSaVioR commented Jul 21, 2020

Uh oh!

HeartSaVioR commented Jul 21, 2020

Uh oh!

SparkQA commented Jul 21, 2020

Uh oh!

HeartSaVioR commented Jul 21, 2020

Uh oh!

SparkQA commented Jul 22, 2020

Uh oh!

HeartSaVioR commented Jul 22, 2020

Uh oh!

SparkQA commented Jul 22, 2020

Uh oh!

HeartSaVioR commented Jul 22, 2020

Uh oh!

baohe-zhang commented Jul 22, 2020

Uh oh!

mridulm commented Aug 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

baohe-zhang commented Aug 10, 2020

Uh oh!

mridulm commented Aug 11, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mridulm commented Aug 10, 2020 •

edited

Loading