[jira] [Assigned] (SPARK-59439) HistoryServerDiskManager double-releases the lease reservation when a failed commit rename triggers rollback

["Cheng Pan (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/SPARK-59439?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Cheng Pan reassigned SPARK-59439:
---------------------------------

    Assignee: Uroš Bojanić

> 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ć
>            Assignee: Uroš Bojanić
>            Priority: Major
>              Labels: pull-request-available
>
> 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: issues-unsubscribe@spark.apache.org
For additional commands, e-mail: issues-help@spark.apache.org