You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/11/08 22:26:12 UTC
[GitHub] [spark] gengliangwang opened a new pull request, #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
gengliangwang opened a new pull request, #38567:
URL: https://github.com/apache/spark/pull/38567
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'core/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Support using RocksDB as the KVStore in live UI. The location of RocksDB can be set via configuration `spark.ui.store.path`. The configuration is optional. The default KV store will still be in memory.
Note: let's make it ROCKSDB only for now instead of supporting both LevelDB and RocksDB. RocksDB is built based on LevelDB with improvements on writes and reads. Furthermore, we can reuse the RocksDBFileManager in streaming for replicating the local RocksDB file to DFS. The replication in DFS can be used for the Spark history server.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
The current architecture of Spark live UI and Spark history server(SHS) is too simple to serve large clusters and heavy workloads:
- Spark stores all the live UI date in memory. The size can be a few GBs and affects the driver's stability (OOM).
- There is a limitation of storing 1000 queries only. Note that we can’t simply increase the limitation under the current Architecture. I did a memory profiling. Storing one query execution detail can take 800KB while storing one task requires 0.3KB. So for 1000 SQL queries with 1000* 2000 tasks, the memory usage for query execution and task data will be 1.4GB. Spark UI stores UI data for jobs/stages/executors as well. So to store 10k queries, it may take more than 14GB.
- SHS has to parse JSON format event log for the initial start. The uncompressed event logs can be as big as a few GBs, and the parse can be quite slow. Some users reported they had to wait for more than half an hour.
With RocksDB as KVStore, we can improve the stability of Spark driver and fasten the startup of SHS.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
Yes, supporting RocksDB as the KVStore in live UI. The location of RocksDB can be set via configuration `spark.ui.store.path`. The configuration is optional. The default KV store will still be in memory.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
New UT
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1028627008
##########
core/src/main/scala/org/apache/spark/internal/config/Status.scala:
##########
@@ -70,4 +70,11 @@ private[spark] object Status {
.version("3.0.0")
.booleanConf
.createWithDefault(false)
+
+ val LIVE_UI_LOCAL_STORE_DIR = ConfigBuilder("spark.ui.store.path")
+ .doc("Local directory where to cache application information for live UI. By default this is " +
+ "not set, meaning all application information will be kept in memory.")
Review Comment:
@tgravescs Thanks, I have added the docs in `docs/configuration.md`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1319148903
Sorry for the delay, I will try to review this later this week ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1324085444
@mridulm @LuciferYang @tgravescs Thanks for the review.
I am merging this one to the master branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1024721571
##########
core/src/main/scala/org/apache/spark/status/KVUtils.scala:
##########
@@ -80,6 +89,44 @@ private[spark] object KVUtils extends Logging {
db
}
+ def createKVStore(
+ storePath: Option[File],
+ diskBackend: HybridStoreDiskBackend.Value,
+ conf: SparkConf): KVStore = {
+ storePath.map { path =>
+ val dir = diskBackend match {
+ case LEVELDB => "listing.ldb"
+ case ROCKSDB => "listing.rdb"
+ }
+
+ val dbPath = Files.createDirectories(new File(path, dir).toPath()).toFile()
+ Utils.chmod700(dbPath)
+
+
+ val metadata = FsHistoryProviderMetadata(
+ FsHistoryProvider.CURRENT_LISTING_VERSION,
+ AppStatusStore.CURRENT_VERSION,
+ conf.get(History.HISTORY_LOG_DIR))
+
+ try {
+ open(dbPath, metadata, conf, Some(diskBackend))
+ } catch {
+ // If there's an error, remove the listing database and any existing UI database
+ // from the store directory, since it's extremely likely that they'll all contain
+ // incompatible information.
+ case _: UnsupportedStoreVersionException | _: MetadataMismatchException =>
Review Comment:
For Live UI, are these two exception scenarios possible?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1024721724
##########
core/src/main/scala/org/apache/spark/status/KVUtils.scala:
##########
@@ -80,6 +89,44 @@ private[spark] object KVUtils extends Logging {
db
}
+ def createKVStore(
+ storePath: Option[File],
+ diskBackend: HybridStoreDiskBackend.Value,
+ conf: SparkConf): KVStore = {
+ storePath.map { path =>
+ val dir = diskBackend match {
+ case LEVELDB => "listing.ldb"
+ case ROCKSDB => "listing.rdb"
+ }
+
+ val dbPath = Files.createDirectories(new File(path, dir).toPath()).toFile()
+ Utils.chmod700(dbPath)
+
+
+ val metadata = FsHistoryProviderMetadata(
+ FsHistoryProvider.CURRENT_LISTING_VERSION,
+ AppStatusStore.CURRENT_VERSION,
+ conf.get(History.HISTORY_LOG_DIR))
+
+ try {
+ open(dbPath, metadata, conf, Some(diskBackend))
+ } catch {
Review Comment:
For Live UI, are these two exception scenarios possible?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1030446796
##########
core/src/main/scala/org/apache/spark/status/AppStatusStore.scala:
##########
@@ -769,7 +772,12 @@ private[spark] object AppStatusStore {
def createLiveStore(
conf: SparkConf,
appStatusSource: Option[AppStatusSource] = None): AppStatusStore = {
- val store = new ElementTrackingStore(new InMemoryStore(), conf)
+ val storePath = conf.get(LIVE_UI_LOCAL_STORE_DIR).map(new File(_))
+ // For the disk-based KV store of live UI, let's simply make it ROCKSDB only for now,
+ // instead of supporting both LevelDB and RocksDB. RocksDB is built based on LevelDB with
Review Comment:
not related to this PR, I'm wondering why did we provide these two choices in the first place. RocksDB only seems sufficient.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1308103370
This pr is mostly adding ability to use a disk backed store in addition to in memory store.
It seems to not match what is described in the pr description - is this wip ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1312210749
@mridulm FYI I have sent the SPIP to the dev list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1308227478
> Or part of a larger set of changes
Yes, I created Jira https://issues.apache.org/jira/browse/SPARK-41053, and this one is just the beginning.
> Maintaining state at driver on disk backed store and copying that to dfs has a few things which impact it - particularly for larger applications.
Yes, but it worthies starting the effort. We can keep developing it until it is ready. I run a benchmark with Protobuf serializer, and the results are promising.
For large applications, the current in-memory live UI can bring memory pressure which affects the driver stability as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1030848858
##########
core/src/main/scala/org/apache/spark/status/AppStatusStore.scala:
##########
@@ -769,7 +772,12 @@ private[spark] object AppStatusStore {
def createLiveStore(
conf: SparkConf,
appStatusSource: Option[AppStatusSource] = None): AppStatusStore = {
- val store = new ElementTrackingStore(new InMemoryStore(), conf)
+ val storePath = conf.get(LIVE_UI_LOCAL_STORE_DIR).map(new File(_))
+ // For the disk-based KV store of live UI, let's simply make it ROCKSDB only for now,
+ // instead of supporting both LevelDB and RocksDB. RocksDB is built based on LevelDB with
Review Comment:
According to the context, I thought Reynold mentioned that the legal issue was resolved. Do you mean that Apache Spark still have legal issue with RocksDB, @gengliangwang ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1308663093
A newbie question: Does this design prefer the `driver` instance to monopolize a disk to ensure performance? If the `driver` shares a disk with other `executors`, will disk r/w caused by `executors` degrade the performance of the `driver`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1027625306
##########
core/src/main/scala/org/apache/spark/status/KVUtils.scala:
##########
@@ -80,6 +89,44 @@ private[spark] object KVUtils extends Logging {
db
}
+ def createKVStore(
+ storePath: Option[File],
+ diskBackend: HybridStoreDiskBackend.Value,
+ conf: SparkConf): KVStore = {
+ storePath.map { path =>
+ val dir = diskBackend match {
+ case LEVELDB => "listing.ldb"
+ case ROCKSDB => "listing.rdb"
+ }
+
+ val dbPath = Files.createDirectories(new File(path, dir).toPath()).toFile()
+ Utils.chmod700(dbPath)
+
+
+ val metadata = FsHistoryProviderMetadata(
+ FsHistoryProvider.CURRENT_LISTING_VERSION,
+ AppStatusStore.CURRENT_VERSION,
+ conf.get(History.HISTORY_LOG_DIR))
+
+ try {
+ open(dbPath, metadata, conf, Some(diskBackend))
+ } catch {
Review Comment:
If unexpected data cleaning doesn't occur in the live UI scenario, I think it is OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1030867941
##########
core/src/main/scala/org/apache/spark/status/AppStatusStore.scala:
##########
@@ -769,7 +772,12 @@ private[spark] object AppStatusStore {
def createLiveStore(
conf: SparkConf,
appStatusSource: Option[AppStatusSource] = None): AppStatusStore = {
- val store = new ElementTrackingStore(new InMemoryStore(), conf)
+ val storePath = conf.get(LIVE_UI_LOCAL_STORE_DIR).map(new File(_))
+ // For the disk-based KV store of live UI, let's simply make it ROCKSDB only for now,
+ // instead of supporting both LevelDB and RocksDB. RocksDB is built based on LevelDB with
Review Comment:
@dongjoon-hyun I was explaining why @vanzin chose LevelDB. Maybe I was wrong about his reason.
> Do you mean that Apache Spark still have legal issue with RocksDB
I don't think so. If so, I wouldn't choose it as the only disk backend of live UI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang closed pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang closed pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
URL: https://github.com/apache/spark/pull/38567
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1027640551
##########
core/src/main/scala/org/apache/spark/status/AppStatusStore.scala:
##########
@@ -769,7 +772,14 @@ private[spark] object AppStatusStore {
def createLiveStore(
conf: SparkConf,
appStatusSource: Option[AppStatusSource] = None): AppStatusStore = {
- val store = new ElementTrackingStore(new InMemoryStore(), conf)
+ val storePath = conf.get(LIVE_UI_LOCAL_STORE_DIR).map(new File(_))
+ // For the disk-based KV store of live UI, let's simply make it ROCKSDB only for now,
+ // instead of supporting both LevelDB and RocksDB. RocksDB is built based on LevelDB with
+ // improvements on writes and reads. Furthermore, we can reuse the RocksDBFileManager in
+ // streaming for replicating the local RocksDB file to DFS. The replication in DFS can be
+ // used for Spark history server.
Review Comment:
Thanks, I have removed the comment about replication.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
tgravescs commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1028174857
##########
core/src/main/scala/org/apache/spark/internal/config/Status.scala:
##########
@@ -70,4 +70,11 @@ private[spark] object Status {
.version("3.0.0")
.booleanConf
.createWithDefault(false)
+
+ val LIVE_UI_LOCAL_STORE_DIR = ConfigBuilder("spark.ui.store.path")
+ .doc("Local directory where to cache application information for live UI. By default this is " +
+ "not set, meaning all application information will be kept in memory.")
Review Comment:
also how does this or will this interact with spark.history.store.path? do we want to explicitly put live in the name of the config to make it clear if they are totally separate?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
tgravescs commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1028172049
##########
core/src/main/scala/org/apache/spark/internal/config/Status.scala:
##########
@@ -70,4 +70,11 @@ private[spark] object Status {
.version("3.0.0")
.booleanConf
.createWithDefault(false)
+
+ val LIVE_UI_LOCAL_STORE_DIR = ConfigBuilder("spark.ui.store.path")
+ .doc("Local directory where to cache application information for live UI. By default this is " +
+ "not set, meaning all application information will be kept in memory.")
Review Comment:
this needs to be documented in the .md docs unless you have a followup issue to document all at once?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1309116975
@mridulm yes I can start a SPIP since you have doubts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1308050103
+CC @shardulm94, @thejdeep
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1308113372
To comment on proposal in description, based on past prototypes I have worked on/seen:
Maintaining state at driver on disk backed store and copying that to dfs has a few things which impact it - particularly for larger applications.
They are not very robust to application crashes, interact in nontrivial ways with shutdown hook (hdfs failures) and increase application termination time during graceful shutdown.
Depending on application characteristics, the impact of disk backed store can positively or negatively impact driver performance (positively - as updates are faster due to index, which was lacking in in memory store (when I added index, memory requirements increased :-( ), negatively due to increased disk activity): was difficult to predict.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1308118224
Also note that we cannot avoid parsing event files at history server with db generated at driver - unless the configs match for both (retained stages, tasks, queries, etc): particularly as these can be specified by users (for example, to aggressively cleanup for perf sensitive apps).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1309119625
> A newbie question: Does this design prefer the driver instance to monopolize a disk to ensure performance? If the driver shares a disk with other executors, will disk r/w caused by executors degrade the performance of the driver?
@LuciferYang I don't expect the RocksDB file to be large. This is an optional solution, it won't become the default at least for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1028629241
##########
core/src/main/scala/org/apache/spark/status/KVUtils.scala:
##########
@@ -80,6 +89,44 @@ private[spark] object KVUtils extends Logging {
db
}
+ def createKVStore(
+ storePath: Option[File],
+ diskBackend: HybridStoreDiskBackend.Value,
+ conf: SparkConf): KVStore = {
+ storePath.map { path =>
+ val dir = diskBackend match {
+ case LEVELDB => "listing.ldb"
+ case ROCKSDB => "listing.rdb"
+ }
+
+ val dbPath = Files.createDirectories(new File(path, dir).toPath()).toFile()
+ Utils.chmod700(dbPath)
+
+
+ val metadata = FsHistoryProviderMetadata(
+ FsHistoryProvider.CURRENT_LISTING_VERSION,
+ AppStatusStore.CURRENT_VERSION,
+ conf.get(History.HISTORY_LOG_DIR))
+
+ try {
+ open(dbPath, metadata, conf, Some(diskBackend))
+ } catch {
Review Comment:
@tgravescs If there is exception from RocksDB, it will delete the target directory and open the KV store again. Otherwise if the error is not from RocksDB, it will fall back to the in-memory store.
It follows the same logic in SHS here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1028977314
##########
core/src/main/scala/org/apache/spark/status/KVUtils.scala:
##########
@@ -80,6 +89,44 @@ private[spark] object KVUtils extends Logging {
db
}
+ def createKVStore(
+ storePath: Option[File],
+ diskBackend: HybridStoreDiskBackend.Value,
+ conf: SparkConf): KVStore = {
+ storePath.map { path =>
+ val dir = diskBackend match {
+ case LEVELDB => "listing.ldb"
+ case ROCKSDB => "listing.rdb"
+ }
+
+ val dbPath = Files.createDirectories(new File(path, dir).toPath()).toFile()
+ Utils.chmod700(dbPath)
+
+
Review Comment:
Thanks, removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
tgravescs commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1028178491
##########
core/src/main/scala/org/apache/spark/status/KVUtils.scala:
##########
@@ -80,6 +89,44 @@ private[spark] object KVUtils extends Logging {
db
}
+ def createKVStore(
+ storePath: Option[File],
+ diskBackend: HybridStoreDiskBackend.Value,
+ conf: SparkConf): KVStore = {
+ storePath.map { path =>
+ val dir = diskBackend match {
+ case LEVELDB => "listing.ldb"
+ case ROCKSDB => "listing.rdb"
+ }
+
+ val dbPath = Files.createDirectories(new File(path, dir).toPath()).toFile()
+ Utils.chmod700(dbPath)
+
+
+ val metadata = FsHistoryProviderMetadata(
+ FsHistoryProvider.CURRENT_LISTING_VERSION,
+ AppStatusStore.CURRENT_VERSION,
+ conf.get(History.HISTORY_LOG_DIR))
+
+ try {
+ open(dbPath, metadata, conf, Some(diskBackend))
+ } catch {
Review Comment:
If something goes wrong and the kv store can't be created for live ui then the entire application fails?
##########
core/src/main/scala/org/apache/spark/status/KVUtils.scala:
##########
@@ -80,6 +89,44 @@ private[spark] object KVUtils extends Logging {
db
}
+ def createKVStore(
+ storePath: Option[File],
+ diskBackend: HybridStoreDiskBackend.Value,
+ conf: SparkConf): KVStore = {
+ storePath.map { path =>
+ val dir = diskBackend match {
+ case LEVELDB => "listing.ldb"
+ case ROCKSDB => "listing.rdb"
+ }
+
+ val dbPath = Files.createDirectories(new File(path, dir).toPath()).toFile()
+ Utils.chmod700(dbPath)
+
+
Review Comment:
nit extra new line
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1031012684
##########
core/src/main/scala/org/apache/spark/status/AppStatusStore.scala:
##########
@@ -769,7 +772,12 @@ private[spark] object AppStatusStore {
def createLiveStore(
conf: SparkConf,
appStatusSource: Option[AppStatusSource] = None): AppStatusStore = {
- val store = new ElementTrackingStore(new InMemoryStore(), conf)
+ val storePath = conf.get(LIVE_UI_LOCAL_STORE_DIR).map(new File(_))
+ // For the disk-based KV store of live UI, let's simply make it ROCKSDB only for now,
+ // instead of supporting both LevelDB and RocksDB. RocksDB is built based on LevelDB with
Review Comment:
Got it. Thank you for confirming that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
mridulm commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1027618122
##########
core/src/main/scala/org/apache/spark/status/AppStatusStore.scala:
##########
@@ -769,7 +772,14 @@ private[spark] object AppStatusStore {
def createLiveStore(
conf: SparkConf,
appStatusSource: Option[AppStatusSource] = None): AppStatusStore = {
- val store = new ElementTrackingStore(new InMemoryStore(), conf)
+ val storePath = conf.get(LIVE_UI_LOCAL_STORE_DIR).map(new File(_))
+ // For the disk-based KV store of live UI, let's simply make it ROCKSDB only for now,
+ // instead of supporting both LevelDB and RocksDB. RocksDB is built based on LevelDB with
+ // improvements on writes and reads. Furthermore, we can reuse the RocksDBFileManager in
+ // streaming for replicating the local RocksDB file to DFS. The replication in DFS can be
+ // used for Spark history server.
Review Comment:
nit: Can we remove the part about replication for now ? When we add support for it, we can suitably modify the comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1025583467
##########
core/src/main/scala/org/apache/spark/status/KVUtils.scala:
##########
@@ -80,6 +89,44 @@ private[spark] object KVUtils extends Logging {
db
}
+ def createKVStore(
+ storePath: Option[File],
+ diskBackend: HybridStoreDiskBackend.Value,
+ conf: SparkConf): KVStore = {
+ storePath.map { path =>
+ val dir = diskBackend match {
+ case LEVELDB => "listing.ldb"
+ case ROCKSDB => "listing.rdb"
+ }
+
+ val dbPath = Files.createDirectories(new File(path, dir).toPath()).toFile()
+ Utils.chmod700(dbPath)
+
+
+ val metadata = FsHistoryProviderMetadata(
+ FsHistoryProvider.CURRENT_LISTING_VERSION,
+ AppStatusStore.CURRENT_VERSION,
+ conf.get(History.HISTORY_LOG_DIR))
+
+ try {
+ open(dbPath, metadata, conf, Some(diskBackend))
+ } catch {
Review Comment:
No. The LevelDB exception is not possible either. There are only two error-handling branches, and seems too much to distinguish whether the caller is for live UI or SHS.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1030747593
##########
core/src/main/scala/org/apache/spark/status/AppStatusStore.scala:
##########
@@ -769,7 +772,12 @@ private[spark] object AppStatusStore {
def createLiveStore(
conf: SparkConf,
appStatusSource: Option[AppStatusSource] = None): AppStatusStore = {
- val store = new ElementTrackingStore(new InMemoryStore(), conf)
+ val storePath = conf.get(LIVE_UI_LOCAL_STORE_DIR).map(new File(_))
+ // For the disk-based KV store of live UI, let's simply make it ROCKSDB only for now,
+ // instead of supporting both LevelDB and RocksDB. RocksDB is built based on LevelDB with
Review Comment:
https://issues.apache.org/jira/browse/SPARK-18085?focusedCommentId=16088758&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16088758
There were legal concerns.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1030848858
##########
core/src/main/scala/org/apache/spark/status/AppStatusStore.scala:
##########
@@ -769,7 +772,12 @@ private[spark] object AppStatusStore {
def createLiveStore(
conf: SparkConf,
appStatusSource: Option[AppStatusSource] = None): AppStatusStore = {
- val store = new ElementTrackingStore(new InMemoryStore(), conf)
+ val storePath = conf.get(LIVE_UI_LOCAL_STORE_DIR).map(new File(_))
+ // For the disk-based KV store of live UI, let's simply make it ROCKSDB only for now,
+ // instead of supporting both LevelDB and RocksDB. RocksDB is built based on LevelDB with
Review Comment:
According to the context, Reynold mentioned that the legal issue was resolved. Do you mean that Apache Spark still have legal issue with RocksDB, @gengliangwang ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a diff in pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #38567:
URL: https://github.com/apache/spark/pull/38567#discussion_r1028627819
##########
core/src/main/scala/org/apache/spark/internal/config/Status.scala:
##########
@@ -70,4 +70,11 @@ private[spark] object Status {
.version("3.0.0")
.booleanConf
.createWithDefault(false)
+
+ val LIVE_UI_LOCAL_STORE_DIR = ConfigBuilder("spark.ui.store.path")
+ .doc("Local directory where to cache application information for live UI. By default this is " +
+ "not set, meaning all application information will be kept in memory.")
Review Comment:
> how does this or will this interact with spark.history.store.path? do we want to explicitly put live in the name of the config to make it clear if they are totally separate?
If the RocksDB instance is gracefully closed, SHS can set the backend as RocksDB and start with the instance from the live UI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1308897570
If this is part of a larger effort, would it be better to make this an spip instead @gengliangwang ?
I have described some of the concerns seen in past/similar efforts - and if is not clear how this effort is approaching the solution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on pull request #38567: [SPARK-41054][UI][CORE] Support RocksDB as KVStore in live UI
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on PR #38567:
URL: https://github.com/apache/spark/pull/38567#issuecomment-1308222974
> It seems to not match what is described in the pr description or add equivalent functionality which was reverted
I believe it covers the reverted PR https://github.com/apache/spark/pull/38542. The previous approach is to create an optional disk store for storing a subset of the `SQLExecutionUIData`.
There are also two shortages:
* There are no evictions for the UI data on the disk
* It requires extra memory for reading/writing LevelDB/RocksDB
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org