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/09/08 02:59:33 UTC

[GitHub] [spark] LuciferYang commented on a diff in pull request #37610: [SPARK-38888][BUILD][CORE][YARN][DOCS] Add `RocksDB` support for shuffle service state store

LuciferYang commented on code in PR #37610:
URL: https://github.com/apache/spark/pull/37610#discussion_r965457952


##########
resource-managers/yarn/src/test/scala/org/apache/spark/network/yarn/YarnShuffleServiceSuite.scala:
##########
@@ -687,7 +687,8 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
     s1.stop()
   }
 
-  test("Finalized merged shuffle are written into DB and cleaned up after application stopped") {
+  // TODO: should enable this test after SPARK-40186 is resolved.
+  ignore("Finalized merged shuffle are written into DB and cleaned up after application stopped") {

Review Comment:
   will re-enable this due to it should pass after SPARK-40186 merged
   
   ```
   mvn clean test -pl resource-managers/yarn -Pyarn -DwildcardSuites=org.apache.spark.network.yarn.YarnShuffleServiceWithRocksDBBackendSuite
   ```
   
   ```
   YarnShuffleServiceWithRocksDBBackendSuite:
   - executor and merged shuffle state kept across NM restart
   - removed applications should not be in registered executor file and merged shuffle file
   - shuffle service should be robust to corrupt registered executor file
   - get correct recovery path
   - moving recovery file from NM local dir to recovery path
   - service throws error if cannot start
   - Consistency in AppPathInfo between in-memory hashmap and the DB
   - Finalized merged shuffle are written into DB and cleaned up after application stopped
   - SPARK-40186: shuffleMergeManager should have been shutdown before db closed
   - Dangling finalized merged partition info in DB will be removed during restart
   - Dangling application path or shuffle information in DB will be removed during restart
   - Cleanup for former attempts local path info should be triggered in applicationRemoved
   - recovery db should not be created if NM recovery is not enabled
   - SPARK-31646: metrics should be registered into Node Manager's metrics system
   - SPARK-34828: metrics should be registered with configured name
   - create default merged shuffle file manager instance
   - create remote block push resolver instance
   - invalid class name of merge manager will use noop instance
   Run completed in 9 seconds, 454 milliseconds.
   Total number of tests run: 18
   Suites: completed 2, aborted 0
   Tests: succeeded 18, failed 0, canceled 0, ignored 0, pending 0
   All tests passed.
   ```



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