pan3793 commented on code in PR #58741:
URL: https://github.com/apache/spark/pull/58741#discussion_r4000648080
##########
core/src/main/scala/org/apache/spark/deploy/history/HistoryServerDiskManager.scala:
##########
@@ -332,6 +332,19 @@ private class HistoryServerDiskManager(
private[history] class Lease(val tmpPath: File, private val leased: Long) {
+ // The leased (reserved, uncommitted) usage must be returned exactly once,
whether the lease
+ // is committed or rolled back. commit() releases it before moving the
store into place, so a
+ // failure after that point (e.g. a failed rename) sends the caller
through rollback(); guard
+ // against releasing it a second time there, which would drive the usage
tracker negative.
+ private var released = false
+
+ private def releaseLease(): Unit = {
+ if (!released) {
+ updateUsage(-leased)
+ released = true
Review Comment:
`updateUsage` applies `currentUsage.addAndGet(delta)` before its negative
check throws, but `released` is set only after it returns. If the release
throws here, the reservation is already deducted while the flag stays `false`,
so the caller's `rollback()` deducts it a second time and skips
`deleteRecursively(tmpPath)`. Set the flag first so the guard also holds on the
exception path.
```suggestion
released = true
updateUsage(-leased)
```
--
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]
