Skip to content

[SPARK-40404][DOCS] Add precondition description for spark.shuffle.service.db.backend in running-on-yarn.md #37853

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
LuciferYang wants to merge 4 commits into from
Closed

[SPARK-40404][DOCS] Add precondition description for spark.shuffle.service.db.backend in running-on-yarn.md #37853

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d8fbfbf
fix
LuciferYang Sep 12, 2022
d8b39ef
fix
LuciferYang Sep 12, 2022
b92cf5c
revert change of docs/spark-standalone.md
LuciferYang Sep 14, 2022
88a253e
add more
LuciferYang Sep 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/running-on-yarn.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -852,8 +852,8 @@ The following extra configuration options are available when the shuffle service
<td><code>spark.shuffle.service.db.backend</code></td>
<td>LEVELDB</td>
<td>
To specify the kind of disk-base store used in shuffle service state store, supports `LEVELDB` and `ROCKSDB` now
and `LEVELDB` as default value.
When Yarn NodeManager recovery is enabled, this use to specify the kind of disk-base store used in shuffle
service state store, supports `LEVELDB` and `ROCKSDB` now and `LEVELDB` as default value.

@mridulm mridulm Sep 12, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
When Yarn NodeManager recovery is enabled, this use to specify the kind of disk-base store used in shuffle
service state store, supports `LEVELDB` and `ROCKSDB` now and `LEVELDB` as default value.
When work-preserving restart is enabled in YARN, this is used to specify the disk-base store used in shuffle
service state store, supports `LEVELDB` and `ROCKSDB` with `LEVELDB` as default value.

@LuciferYang LuciferYang Sep 12, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

d8b39ef fix this

@LuciferYang LuciferYang Sep 12, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your correction

The original data store in `LevelDB/RocksDB` will not be automatically convert to another kind of storage now.

@mridulm mridulm Sep 14, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

convert -> converted ?

@dongjoon-hyun dongjoon-hyun Sep 15, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 for @mridulm comment. And, could you add additional description about what happens at the runtime when the the store types are mismatched. It's deleted and recreated, right?

@LuciferYang LuciferYang Sep 15, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The old one will not be deleted, but the new one will be created. When the store type is switched, the directory name will change, for example, from registeredExecutors.ldb to registeredExecutors.rdb, YarnShuffleService will create registeredExecutors.rdb if it not exists, but YarnShuffleService did not know that registeredExecutors.ldb existed, so it will not be deleted

@LuciferYang LuciferYang Sep 15, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Add The original data store will be retained and the new type data store will be created when switching storage types. Is that ok ?

@LuciferYang LuciferYang Sep 16, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@dongjoon-hyun @mridulm Automatic data format conversion may be a useful feature. I think it is more friendly for migrating stock users to use new features. I have filed a Jira SPARK-40464 and will promote its completion if necessary.

@LuciferYang LuciferYang Sep 16, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@dongjoon-hyun @mridulm Automatic data format conversion may be a useful feature. I think it is more friendly for migrating stock users to use new features. I have filed a Jira SPARK-40464 and will promote its completion if necessary.

also cc @panbingkun , what I discussed with you offline yesterday

</td>
<td>3.4.0</td>
Expand Down
6 changes: 3 additions & 3 deletions docs/spark-standalone.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,9 @@ SPARK_WORKER_OPTS supports the following system properties:
<td>true</td>
<td>
Store External Shuffle service state on local disk so that when the external shuffle service is restarted, it will
automatically reload info on current executors. This only affects standalone mode (yarn always has this behavior
enabled). You should also enable <code>spark.worker.cleanup.enabled</code>, to ensure that the state
eventually gets cleaned up. This config may be removed in the future.
automatically reload info on current executors. This only affects standalone mode. You should also enable
<code>spark.worker.cleanup.enabled</code>, to ensure that the state eventually gets cleaned up.
This config may be removed in the future.

@mridulm mridulm Sep 12, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are we removing the yarn related blurb from here ? Essentially, this boolean does not control the behavior in yarn - for yarn, that is configured for the cluster, and inherits the behavior for spark

@LuciferYang LuciferYang Sep 12, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm... does yarn always has this behavior enabled mean that YarnShuffleService will always persist data into Level/RocksDB?

Is that incorrect?

@LuciferYang LuciferYang Sep 12, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • when yarn.nodemanager.recovery.enabled is true, _recoveryPath and registeredExecutorFile in YarnShuffleService will not null, then YarnShuffleService persist data into Level/RocksDB
  • when yarn.nodemanager.recovery.enabled is false, _recoveryPath and registeredExecutorFile in YarnShuffleService will null, then YarnShuffleService not persist data into diskstore

The persist behavior of YarnShuffleService is controlled by Yarn's configuration. It seems that it not related to spark.shuffle.service.db.enabled, so I don't think it is necessary to mention yarn always has this behavior enabled in this configuration description.

If we need add yarn related descriptions here, should we also need mention mesos always has this behavior disabled here...

@LuciferYang LuciferYang Sep 12, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Or can we change

yarn always has this behavior enabled

to

The behavior of yarn (and mesos) not depend on this configuration

?

@LuciferYang LuciferYang Sep 13, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

friendly ping @weixiuli @squito to help check this change

@LuciferYang LuciferYang Sep 14, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Have an offline discussion with the author @weixiuli of this configuration description, b92cf5c revert change in this pr

</td>
<td>3.0.0</td>
</tr>
Expand Down