You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
When a workflow re-uploads an artifact whose previous content had already
expired (and whose storage blob was removed by the cleanup job), CreateArtifact
revives the existing DB row as a fresh pending upload. This PR fixes two problems
in that path and adds test coverage.
1. Revive path resets status/storage but leaves stale content pointers
Previously, a matching row in a terminal state (Expired / PendingDeletion / Deleted) was "revived" by only bumping expired_unix. That left the row stuck
in its terminal status while still pointing at a StoragePath/FileSize that no
longer correspond to any content. A re-upload now reinitializes the row in place
as a brand-new UploadPending record (the unique index on run_id/run_attempt_id/name/path requires updating the existing row rather than
inserting a second one).
2. created_unix was being clobbered on every re-upload
The revive path returned a struct whose CreatedUnix was 0. Both callers hand
that returned struct straight to UpdateArtifactByID, which uses AllCols() —
so the zero value overwrote the row's original creation timestamp. The fix
carries existing.CreatedUnix onto the returned struct so the subsequent AllCols() update preserves it.
Why
Re-running a job after its artifact expired is a normal case. Without this fix
the artifact row keeps a dead status and stale storage pointers, and its created_unix gets reset to "now" on every re-upload.
Wouldn’t there be a job attempt ID to prevent them from overriding each other?
Reruns don't override each other — cloneRunJobForAttempt assigns a new RunAttemptID, and the unique key includes run_attempt_id, so each attempt gets its own row. The re-upload this fixes is within the same run+attempt: upload-artifact@v4 with overwrite: true deletes the artifact (SetArtifactNeedDeleteByRunAttempt → PendingDeletion) and then re-creates the same name/path. The old code revived that dead row by only bumping expired_unix, leaving it in a terminal status pointing at a deleted blob. IsTerminal() catches that and reinitializes it as a fresh pending upload instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
backport/v1.27This PR should be backported to Gitea 1.26lgtm/need 2This PR needs two approvals by maintainers to be considered for merging.type/bug
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a workflow re-uploads an artifact whose previous content had already
expired (and whose storage blob was removed by the cleanup job),
CreateArtifactrevives the existing DB row as a fresh pending upload. This PR fixes two problems
in that path and adds test coverage.
1. Revive path resets status/storage but leaves stale content pointers
Previously, a matching row in a terminal state (
Expired/PendingDeletion/Deleted) was "revived" by only bumpingexpired_unix. That left the row stuckin its terminal status while still pointing at a
StoragePath/FileSizethat nolonger correspond to any content. A re-upload now reinitializes the row in place
as a brand-new
UploadPendingrecord (the unique index onrun_id/run_attempt_id/name/pathrequires updating the existing row rather thaninserting a second one).
2.
created_unixwas being clobbered on every re-uploadThe revive path returned a struct whose
CreatedUnixwas0. Both callers handthat returned struct straight to
UpdateArtifactByID, which usesAllCols()—so the zero value overwrote the row's original creation timestamp. The fix
carries
existing.CreatedUnixonto the returned struct so the subsequentAllCols()update preserves it.Why
Re-running a job after its artifact expired is a normal case. Without this fix
the artifact row keeps a dead status and stale storage pointers, and its
created_unixgets reset to "now" on every re-upload.