HeartSaVioR commented on a change in pull request #27208: [SPARK-30481][CORE]
Integrate event log compactor into Spark History Server
URL: https://github.com/apache/spark/pull/27208#discussion_r370981444
##########
File path:
core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala
##########
@@ -1175,6 +1237,21 @@ private[history] class FsHistoryProvider(conf:
SparkConf, clock: Clock)
}
deleted
}
+
+ /** NOTE: 'task' should ensure it executes 'endProcessing' at the end */
Review comment:
`finally` statement in `mergeApplicationListing` makes it be complicated,
because we should handle the reentrance of lock. (We're submitting another task
in the task.)
If we move endProcessing from `finally` statement in
`mergeApplicationListing` to the end of task here, `processing` on the
compaction task would be executed earlier than calling `endProcessing` on the
listing task. Marking lock from compaction task would succeed but effectively
no-op, and releasing lock from listing task would remove the mark for
compaction task as well, which makes the compaction task run without proper
lock.
So either we need to make lock much smarter, or document the requirement on
caller side. I'm feeling that former one is more complicated than latter one.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
