Skip to content
Snippets Groups Projects
Commit b1581ac2 authored by Josh Rosen's avatar Josh Rosen
Browse files

[SPARK-9854] [SQL] RuleExecutor.timeMap should be thread-safe

`RuleExecutor.timeMap` is currently a non-thread-safe mutable HashMap; this can lead to infinite loops if multiple threads are concurrently modifying the map.  I believe that this is responsible for some hangs that I've observed in HiveQuerySuite.

This patch addresses this by using a Guava `AtomicLongMap`.

Author: Josh Rosen <joshrosen@databricks.com>

Closes #8120 from JoshRosen/rule-executor-time-map-fix.
parent c3e9a120
No related branches found
No related tags found
No related merge requests found
......@@ -17,22 +17,25 @@
package org.apache.spark.sql.catalyst.rules
import scala.collection.JavaConverters._
import com.google.common.util.concurrent.AtomicLongMap
import org.apache.spark.Logging
import org.apache.spark.sql.catalyst.trees.TreeNode
import org.apache.spark.sql.catalyst.util.sideBySide
import scala.collection.mutable
object RuleExecutor {
protected val timeMap = new mutable.HashMap[String, Long].withDefault(_ => 0)
protected val timeMap = AtomicLongMap.create[String]()
/** Resets statistics about time spent running specific rules */
def resetTime(): Unit = timeMap.clear()
/** Dump statistics about time spent running specific rules. */
def dumpTimeSpent(): String = {
val maxSize = timeMap.keys.map(_.toString.length).max
timeMap.toSeq.sortBy(_._2).reverseMap { case (k, v) =>
val map = timeMap.asMap().asScala
val maxSize = map.keys.map(_.toString.length).max
map.toSeq.sortBy(_._2).reverseMap { case (k, v) =>
s"${k.padTo(maxSize, " ").mkString} $v"
}.mkString("\n")
}
......@@ -79,7 +82,7 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging {
val startTime = System.nanoTime()
val result = rule(plan)
val runTime = System.nanoTime() - startTime
RuleExecutor.timeMap(rule.ruleName) = RuleExecutor.timeMap(rule.ruleName) + runTime
RuleExecutor.timeMap.addAndGet(rule.ruleName, runTime)
if (!result.fastEquals(plan)) {
logTrace(
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment