[
https://issues.apache.org/jira/browse/SPARK-59439?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated SPARK-59439:
-----------------------------------
Labels: pull-request-available (was: )
> 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ć
> 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: [email protected]
For additional commands, e-mail: [email protected]
