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

   ### What changes were proposed in this pull request?
   This PR adds read-write lock to `TaskMetrics#registerAccumulator` and  
`TaskMetrics#accumulators` to ensure that errors described in 
SPARK-39696(`java.util.ConcurrentModificationException: mutation occurred 
during iteration`) will not occur when these two methods are called 
concurrently.
   
   
   ### Why are the changes needed?
   Bug fix.
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   
   Pass GitHub Actions and add a new test case.
   
   The new case will fail regardless of the Scala version without changes of 
`TaskMetrics` as follows:
   
   Scala 2.12:
   
   ```
   mvn clean test -pl core -am -Dtest=none 
-DwildcardSuites=org.apache.spark.executor.TaskMetricsSuite 
   ```
   
   **Before**
   
   ```
   TaskMetricsSuite:
   - mutating values
   - mutating shuffle read metrics values
   - mutating shuffle write metrics values
   - mutating input metrics values
   - mutating output metrics values
   - merging multiple shuffle read metrics
   - additional accumulables
   - SPARK-39696: Concurrent r/w of TaskMetrics should not throw Exception *** 
FAILED ***
     2 did not equal 0 (TaskMetricsSuite.scala:274)
   Run completed in 6 seconds, 980 milliseconds.
   Total number of tests run: 8
   Suites: completed 2, aborted 0
   Tests: succeeded 7, failed 1, canceled 0, ignored 0, pending 0
   *** 1 TEST FAILED ***
   ```
   
   **After**
   
   ```
   TaskMetricsSuite:
   - mutating values
   - mutating shuffle read metrics values
   - mutating shuffle write metrics values
   - mutating input metrics values
   - mutating output metrics values
   - merging multiple shuffle read metrics
   - additional accumulables
   - SPARK-39696: Concurrent r/w of TaskMetrics should not throw Exception
   Run completed in 7 seconds, 516 milliseconds.
   Total number of tests run: 8
   Suites: completed 2, aborted 0
   Tests: succeeded 8, failed 0, canceled 0, ignored 0, pending 0
   All tests passed.
   
   ```
   
   Scala 2.13
   
   ```
   mvn clean test -pl core -am -Dtest=none 
-DwildcardSuites=org.apache.spark.executor.TaskMetricsSuite  -Pscala-2.13
   ```
   
   **Before** 
   
   ```
   TaskMetricsSuite:
   - mutating values
   - mutating shuffle read metrics values
   - mutating shuffle write metrics values
   - mutating input metrics values
   - mutating output metrics values
   - merging multiple shuffle read metrics
   - additional accumulables
   - SPARK-39696: Concurrent r/w of TaskMetrics should not throw Exception *** 
FAILED ***
     1339 did not equal 0 (TaskMetricsSuite.scala:275)
   Run completed in 6 seconds, 714 milliseconds.
   Total number of tests run: 8
   Suites: completed 2, aborted 0
   Tests: succeeded 7, failed 1, canceled 0, ignored 0, pending 0
   *** 1 TEST FAILED ***
   ```
   
   **After**
   
   ```
   Discovery starting.
   Discovery completed in 6 seconds, 369 milliseconds.
   Run starting. Expected test count is: 8
   TaskMetricsSuite:
   - mutating values
   - mutating shuffle read metrics values
   - mutating shuffle write metrics values
   - mutating input metrics values
   - mutating output metrics values
   - merging multiple shuffle read metrics
   - additional accumulables
   - SPARK-39696: Concurrent r/w of TaskMetrics should not throw Exception
   Run completed in 8 seconds, 434 milliseconds.
   Total number of tests run: 8
   Suites: completed 2, aborted 0
   Tests: succeeded 8, 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to