Skip to content

Commit 151b954

Browse files
wypoonsquito
authored andcommitted
[SPARK-28770][CORE][TEST] Fix ReplayListenerSuite tests that sometimes fail
### What changes were proposed in this pull request? `ReplayListenerSuite` depends on a listener class to listen for replayed events. This class was implemented by extending `EventLoggingListener`. `EventLoggingListener` does not log executor metrics update events, but uses them to update internal state; on a stage completion event, it then logs stage executor metrics events using this internal state. As executor metrics update events do not get written to the event log, they do not get replayed. The internal state of the replay listener can therefore be different from the original listener, leading to different stage completion events being logged. We reimplement the replay listener to simply buffer each and every event it receives. This makes it a simpler yet better tool for verifying the events that get sent through the ReplayListenerBus. ### Why are the changes needed? As explained above. Tests sometimes fail due to events being received by the `EventLoggingListener` that do not get logged (and thus do not get replayed) but influence other events that get logged. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Existing unit tests. Closes #25673 from wypoon/SPARK-28770. Authored-by: Wing Yew Poon <[email protected]> Signed-off-by: Imran Rashid <[email protected]>
1 parent 0647906 commit 151b954

File tree

1 file changed

+16
-20
lines changed

1 file changed

+16
-20
lines changed

core/src/test/scala/org/apache/spark/scheduler/ReplayListenerSuite.scala

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@ import java.io._
2121
import java.net.URI
2222
import java.util.concurrent.atomic.AtomicInteger
2323

24+
import scala.collection.mutable.ArrayBuffer
25+
2426
import org.apache.hadoop.fs.Path
2527
import org.json4s.JsonAST.JValue
2628
import org.json4s.jackson.JsonMethods._
2729
import org.scalatest.BeforeAndAfter
2830

29-
import org.apache.spark.{LocalSparkContext, SparkConf, SparkContext, SparkFunSuite}
31+
import org.apache.spark._
3032
import org.apache.spark.deploy.SparkHadoopUtil
3133
import org.apache.spark.io.{CompressionCodec, LZ4CompressionCodec}
3234
import org.apache.spark.util.{JsonProtocol, JsonProtocolSuite, Utils}
@@ -62,7 +64,7 @@ class ReplayListenerSuite extends SparkFunSuite with BeforeAndAfter with LocalSp
6264

6365
val conf = EventLoggingListenerSuite.getLoggingConf(logFilePath)
6466
val logData = fileSystem.open(logFilePath)
65-
val eventMonster = new EventMonster(conf)
67+
val eventMonster = new EventBufferingListener
6668
try {
6769
val replayer = new ReplayListenerBus()
6870
replayer.addListener(eventMonster)
@@ -108,7 +110,7 @@ class ReplayListenerSuite extends SparkFunSuite with BeforeAndAfter with LocalSp
108110
val conf = EventLoggingListenerSuite.getLoggingConf(logFilePath)
109111
val replayer = new ReplayListenerBus()
110112

111-
val eventMonster = new EventMonster(conf)
113+
val eventMonster = new EventBufferingListener
112114
replayer.addListener(eventMonster)
113115

114116
// Verify the replay returns the events given the input maybe truncated.
@@ -145,7 +147,7 @@ class ReplayListenerSuite extends SparkFunSuite with BeforeAndAfter with LocalSp
145147

146148
val conf = EventLoggingListenerSuite.getLoggingConf(logFilePath)
147149
val logData = fileSystem.open(logFilePath)
148-
val eventMonster = new EventMonster(conf)
150+
val eventMonster = new EventBufferingListener
149151
try {
150152
val replayer = new ReplayListenerBus()
151153
replayer.addListener(eventMonster)
@@ -207,7 +209,7 @@ class ReplayListenerSuite extends SparkFunSuite with BeforeAndAfter with LocalSp
207209

208210
// Replay events
209211
val logData = EventLoggingListener.openEventLog(eventLog.getPath(), fileSystem)
210-
val eventMonster = new EventMonster(conf)
212+
val eventMonster = new EventBufferingListener
211213
try {
212214
val replayer = new ReplayListenerBus()
213215
replayer.addListener(eventMonster)
@@ -219,11 +221,11 @@ class ReplayListenerSuite extends SparkFunSuite with BeforeAndAfter with LocalSp
219221
// Verify the same events are replayed in the same order
220222
assert(sc.eventLogger.isDefined)
221223
val originalEvents = sc.eventLogger.get.loggedEvents
224+
.map(JsonProtocol.sparkEventFromJson(_))
222225
val replayedEvents = eventMonster.loggedEvents
226+
.map(JsonProtocol.sparkEventFromJson(_))
223227
originalEvents.zip(replayedEvents).foreach { case (e1, e2) =>
224-
// Don't compare the JSON here because accumulators in StageInfo may be out of order
225-
JsonProtocolSuite.assertEquals(
226-
JsonProtocol.sparkEventFromJson(e1), JsonProtocol.sparkEventFromJson(e2))
228+
JsonProtocolSuite.assertEquals(e1, e1)
227229
}
228230
}
229231

@@ -235,21 +237,15 @@ class ReplayListenerSuite extends SparkFunSuite with BeforeAndAfter with LocalSp
235237

236238
/**
237239
* A simple listener that buffers all the events it receives.
238-
*
239-
* The event buffering functionality must be implemented within EventLoggingListener itself.
240-
* This is because of the following race condition: the event may be mutated between being
241-
* processed by one listener and being processed by another. Thus, in order to establish
242-
* a fair comparison between the original events and the replayed events, both functionalities
243-
* must be implemented within one listener (i.e. the EventLoggingListener).
244-
*
245-
* This child listener inherits only the event buffering functionality, but does not actually
246-
* log the events.
247240
*/
248-
private class EventMonster(conf: SparkConf)
249-
extends EventLoggingListener("test", None, new URI("testdir"), conf) {
241+
private class EventBufferingListener extends SparkFirehoseListener {
250242

251-
override def start() { }
243+
private[scheduler] val loggedEvents = new ArrayBuffer[JValue]
252244

245+
override def onEvent(event: SparkListenerEvent) {
246+
val eventJson = JsonProtocol.sparkEventToJson(event)
247+
loggedEvents += eventJson
248+
}
253249
}
254250

255251
/*

0 commit comments

Comments
 (0)