[SPARK-54353][SQL] Make CollectMetricsExec.collectedMetrics thread-safe

[heyihong]

What changes were proposed in this pull request?

This PR makes the CollectMetricsExec.collectedMetrics method thread-safe by adding proper synchronization. The changes include:

1. Modified CollectMetricsExec.collectedMetrics method: Added synchronization blocks around both the accumulator.value access and the toRowConverter call to prevent race conditions when multiple threads access the collected metrics concurrently.

2. Added concurrency test: Added a new test case SPARK-54353: concurrent CollectMetricsExec.collect() in DatasetSuite that verifies the thread-safety by spawning multiple threads that concurrently access CollectMetricsExec.collect().

3. Removed the coarse-grained lock in QueryExecution.observedMetrics

The implementation ensures that:

• The accumulator value is read in a thread-safe manner
• The row converter (which is not thread-safe) is protected from concurrent access
• Both operations are properly synchronized to avoid data races

Why are the changes needed?

The collectedMetrics method in CollectMetricsExec can be accessed concurrently by multiple threads, particularly when using the Observation API. The previous implementation had two thread-safety issues:

1. Accumulator access: The accumulator.value call could be accessed by multiple threads simultaneously
2. Row converter: The toRowConverter (an InternalRow => Row converter generated by code generation) is not thread-safe and can cause race conditions when accessed concurrently

Without proper synchronization, concurrent access could lead to:

• Incorrect metric values being returned
• Potential data corruption
• Race conditions in the row conversion process

This is especially problematic in scenarios where multiple threads query the execution plan or access observations simultaneously.

Does this PR introduce any user-facing change?

No. This is an internal bug fix that improves thread-safety. Users should not observe any behavioral changes except for the elimination of potential race conditions and incorrect results when accessing metrics concurrently.

How was this patch tested?

build/sbt "sql/testOnly *DatasetSuite -- -z SPARK-54353"

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Cursor 1.7.54

[hvanhovell]

Why does this need to be thread safe? None of the code in Spark Core has to be thread safe.

[hvanhovell]

@heyihong please fix the locking in CollectMetricsExec. Get the value out of the accumulator and move into a a synchronized field. This change as is will cause performance regression throughout the engine.

[heyihong]

@heyihong please fix the locking in CollectMetricsExec. Get the value out of the accumulator and move into a a synchronized field. This change as is will cause performance regression throughout the engine.

The accumulator also needs synchronization because accumulator.value (for example, when using ArrayBasedMapBuilder) is not thread-safe.