Uroš Bojanić created SPARK-59439:
------------------------------------

             Summary: HistoryServerDiskManager double-releases the lease 
reservation when a failed commit rename triggers rollback
                 Key: SPARK-59439
                 URL: https://issues.apache.org/jira/browse/SPARK-59439
             Project: Spark
          Issue Type: Bug
          Components: Spark Core
    Affects Versions: 5.0.0
            Reporter: Uroš Bojanić


h3. Description

{{HistoryServerDiskManager.Lease.commit()}} releases the lease's reserved 
(uncommitted) usage with {{updateUsage(-leased)}} *before* it renames the 
temporary store into place. SPARK-58985 changed that rename to throw an 
{{IOException}} when it fails:

{code:scala}
updateUsage(-leased)                      // reservation released here
val newSize = sizeOf(tmpPath)
makeRoom(newSize)
active.synchronized {
  if (!tmpPath.renameTo(dst)) {
    throw new IOException(...)             // failure raised AFTER the release
  }
  ...
}
{code}

The caller reacts to that exception by rolling the lease back. For example, 
{{FsHistoryProvider.createDiskStore()}} wraps {{lease.commit()}} in a try/catch 
that calls {{lease.rollback()}} on both {{IOException}} and other exceptions, 
and {{Lease.rollback()}} releases the reservation a second time:

{code:scala}
def rollback(): Unit = {
  updateUsage(-leased)                    // reservation released a SECOND time
  Utils.deleteRecursively(tmpPath)
}
{code}

The reservation is added once (in {{lease()}}) but subtracted twice -- once by 
the failed {{commit()}} and once by {{rollback()}} -- so the current-usage 
tracker is under-counted by the leased amount. When it cannot absorb the extra 
deduction it goes negative and {{updateUsage}} throws:

{noformat}
java.lang.IllegalStateException: Disk usage tracker went negative (now = ..., 
delta = ...)
{noformat}

which is the same failure SPARK-58985 set out to prevent. In 
{{createDiskStore()}}'s retry loop the exception also escapes the loop, so the 
store is never rebuilt.

A failed {{renameTo}} is the trigger: an I/O error, the disk filling up, or the 
destination's parent directory being removed out of band.

This was introduced by SPARK-58985, which was backported to 4.0.x / 4.1.x / 
4.2.x / 4.3.x, so the fix should follow the same lines.

h3. Fix

Release the lease's reservation exactly once. Track whether it has already been 
returned and funnel both {{commit()}} (before the rename) and {{rollback()}} 
through an idempotent {{releaseLease()}}, so a failed commit followed by a 
rollback deducts it only once. A regression test forces the commit rename to 
fail, then rolls back, and asserts the usage tracker returns to zero (never 
negative) and that a subsequent lease/commit still succeeds.




--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to