Skip to content

session: a job's stop waits for the log-retention sweep, and publishing its end first races folder teardown #1636

Closed

Description

@AbirAbbas

What happened

Since #1603 (log retention, in #1627), a job's ending runs the whole log-folder retention sweep before the job's status leaves running. Under load, a stopped job reads as running for 47–130 ms, against about 2 ms before. TestStopWorkDoesNotWakeAndOnlyFreshSubmissionRestarts fails 5 of 20 runs under load on #1627's head 6cc872c8e, and 0 of 20 on dev 837b2b06a.

The obvious fix, publishing the final status first and then sweeping, was tried on #1627 (8b0edfd6d) and reverted (2e007e33b). Anything that watches running() then removes the job's folder while the sweep is still writing in it. CI:

--- FAIL: TestAParkedWorkerIsHandedBackWithARecordBeforeItsWholeAllowance (1.05s)
    testing.go:1464: TempDir RemoveAll cleanup: unlinkat …/003/.codeaf/jobs: directory not empty

waitSettled in internal/session/task_job_park_test.go polls !one.running(). Giving a task's copy back after its jobs end has the same shape in the product.

Replication

Deterministic (no model). On #1627's head, loaded (run beside another package's tests, or with GOMAXPROCS=2):

go test -run '^TestStopWorkDoesNotWakeAndOnlyFreshSubmissionRestarts$' -count=20 ./internal/session

A few runs fail. With 8b0edfd6d applied instead, run
-run '^TestAParkedWorkerIsHandedBackWithARecordBeforeItsWholeAllowance$' -count=50 under the same load. It fails on the TempDir cleanup.

Where

  • job.settle and jobSink.close in internal/session/jobs.go (search retention)
  • the sweep in internal/session/jobretention.go

The fix

A job's stop is not slowed by retention, and no observer of a job's end can race the sweep. One way to get both: the sweep stops being part of any single job's ending. It runs as one registry-owned pass, which shutdown and the registry's close join, and which tolerates its directory disappearing. Test helpers that wait for a job's end wait on done, not on running().

Acceptance

  • Unit: TestStopWorkDoesNotWakeAndOnlyFreshSubmissionRestarts passes 50/50 under GOMAXPROCS=2 with another package's tests running beside it.
  • Unit: TestAParkedWorkerIsHandedBackWithARecordBeforeItsWholeAllowance passes 50/50 under the same load.
  • Unit: a new test ends a job and immediately removes its jobs folder, as a copy's give-back does, and nothing is written into it afterwards.
  • Unit: a new test proves retention still bounds the folder (the Job logs grow without bound: recursive search of runtime logs consumed 152 GiB and filled disk #1599 law) and that shutdown still joins a sweep in progress.

Activity

  1. added
    bugSomething the code does that it should not
    area:sessionThe engine — turns, tasks, the toolbelt, checkpoints
    on Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:sessionThe engine — turns, tasks, the toolbelt, checkpointsbugSomething the code does that it should not

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions