You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "siying (via GitHub)" <gi...@apache.org> on 2023/10/11 23:02:30 UTC

[PR] [SPARK-45504][SS] Lower CPU Priority Of RocksDB Background Threads [spark]

siying opened a new pull request, #43339:
URL: https://github.com/apache/spark/pull/43339

   
   ### What changes were proposed in this pull request?
   Lower RocksDB background threads' CPU priority.
   
   
   ### Why are the changes needed?
   RocksDB background threads execute background tasks such as compactions and flushes. It should be in general lower priority than Spark tasks excution. For the case where a task may wait for some RocksDB background task to finish, such as checkpointing, or waiting async checkpointing to finish, the task slot is waiting so we are likely to have enough CPU for low pri CPU anyway.
   
   ### Does this PR introduce _any_ user-facing change?
   No.
   
   ### How was this patch tested?
   Run benchmarks and see the results.
   
   
   ### Was this patch authored or co-authored using generative AI tooling?
   No.
   


-- 
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

Re: [PR] [SPARK-45504][SS] Lower CPU Priority Of RocksDB Background Threads [spark]

Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on code in PR #43339:
URL: https://github.com/apache/spark/pull/43339#discussion_r1359983633


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/RocksDB.scala:
##########
@@ -109,6 +109,8 @@ class RocksDB(
   dbOptions.setCreateIfMissing(true)
   dbOptions.setTableFormatConfig(tableFormatConfig)
   dbOptions.setMaxOpenFiles(conf.maxOpenFiles)
+  dbOptions.getEnv().lowerThreadPoolCPUPriority(Priority.HIGH)

Review Comment:
   Just a curiosity, should we set this as up and down? Or unnecessary 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

Re: [PR] [SPARK-45504][SS] Lower CPU Priority Of RocksDB Background Threads [spark]

Posted by "siying (via GitHub)" <gi...@apache.org>.
siying commented on PR #43339:
URL: https://github.com/apache/spark/pull/43339#issuecomment-1778049354

   The benchmark results not decisive. Sometimes it does show P99 improvements, but P50 tends to regress. Closing it for now. Will reopen if things change.


-- 
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

Re: [PR] [SPARK-45504][SS] Lower CPU Priority Of RocksDB Background Threads [spark]

Posted by "siying (via GitHub)" <gi...@apache.org>.
siying commented on code in PR #43339:
URL: https://github.com/apache/spark/pull/43339#discussion_r1370819296


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/RocksDB.scala:
##########
@@ -109,6 +109,8 @@ class RocksDB(
   dbOptions.setCreateIfMissing(true)
   dbOptions.setTableFormatConfig(tableFormatConfig)
   dbOptions.setMaxOpenFiles(conf.maxOpenFiles)
+  dbOptions.getEnv().lowerThreadPoolCPUPriority(Priority.HIGH)

Review Comment:
   This cannot set up or down. Once priority for a thread is lowered, there is no way to move it up.



-- 
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

Re: [PR] [SPARK-45504][SS] Lower CPU Priority Of RocksDB Background Threads [spark]

Posted by "siying (via GitHub)" <gi...@apache.org>.
siying closed pull request #43339: [SPARK-45504][SS] Lower CPU Priority Of RocksDB Background Threads
URL: https://github.com/apache/spark/pull/43339


-- 
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