Repository navigation
Conversation
Port of owncloud#692, adapted to our diverged decomposedfs copies: - Add an Orphaned upload session filter to storage.UploadSessionFilter, evaluated last in ListUploadSessions of both copies because it reads the node metadata of every session. - Add IsOrphaned and syntheticNode to OcisSession and DecomposedFsSession. - Extract the node reverting from upload.Cleanup into revertNode and run it before the bin and info files are removed, so a failure leaves a recoverable session instead of a deleted one. The readable-node branch keeps our local RevertUpload/Purge logic (upstream has RevertCurrentRevision). When the node metadata is unreadable, revert the optimistic size propagation from a node built out of the session's SpaceID/NodeID/NodeParentID and remove the orphaned node, so the space quota is released; the upload is kept when the quota cannot be released. In the pkg fork the orphaned node is removed via lu.PurgeNode and its metadata via the MetadataNode-based backend Purge. - Clean up orphaned sessions on PostprocessingFinished instead of returning early and leaving them to be retried forever. - Add the regression test to both upload_async_test.go copies. Changelog is maintained in the ready-release-go PR, not per commit. Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
The pkg decomposedfs fork gets PrepareUpload in the next commit.
The pkg decomposedfs fork snapshots the previous version through the tree's CreateRevision, like the legacy upload path does, so posix keeps a copy of the current content as a revision while decomposedfs only copies the metadata. On failure it removes that revision without retaking the node lock and restores the old mtime in the same batch as the other attributes. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
CommitUpload wrote the staged bytes through a bare node without a space root, which the posix blobstore rejects, and always onto the live node. On posix the content lives in the node itself, so: - commit against the real, locked node - if a newer upload took over the node, write into the revision that upload created for this session; without one (versioning disabled) discard the bytes, nothing references them - add UploadSource.Path so a staged file can be moved into place instead of copied from a reader - stage posix reader uploads in the space's .oc-tmp dir, which the watcher ignores, and don't re-stage a file that already is there Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Hunks that conflict with our revert code (RevertUpload) keep our side; the next commit adapts RollbackUpload to it.
Both decomposedfs copies get Node.RevertLatestRevision, which restores the latest revision onto the node and removes it, without purging the node or unmarking processing. RollbackUpload uses it. Without a revision (versioning disabled) it returns PreconditionFailed and leaves the node alone instead of purging it, as the previous metadata can't be restored. Adds RollbackUpload to the pkg fork, with specs for both copies and posix. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
RollbackUpload keeps our PreconditionFailed when there is no revision to restore. The pkg decomposedfs fork gets the fixes in the next commit.
- PrepareUpload checks the quota for new files too - CommitUpload stores the scan result on the node that holds the content: the node, or the revision when a newer upload took over - RollbackUpload purges a node the upload created Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Every driver copies options.DisableVersioning into the aspect, and only the legacy upload session store reads it; PrepareUpload reads the option. It can go when the upload coordinator replaces that store. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
The legacy decomposedfs copy stays a snapshot of the original driver, including its known flaws; our own upload behaviour lives in the pkg fork. So this undoes what the earlier commits added there: - CommitUpload takes a body only; a staged file path is for the pkg fork - RollbackUpload purges an existing node when there is no revision to restore, like upstream, instead of returning PreconditionFailed - Node.RevertCurrentRevision provides upstream's API on top of our RevertUpload; DeleteRevision is back as it was - the rollback specs are upstream's Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
- Add node.ErrMissingParentID and treat a session as orphaned only when reading its node fails with it. Other read errors, e.g. an unavailable cache, are logged and leave the session untouched. - On PostprocessingFinished, only drop the bytes and info of an orphaned session when postprocessing asked to delete the upload. - Clear and persist the session's sizeDiff before reverting it, so a repeated cleanup does not release the quota twice. - Remove the bytes even if reverting the node failed, so infected files do not stay on disk. Only the info is kept for a retry. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Upload sessions carry an explicit status (uploading, processing, failed) with a message, as the decomposedfs sessions do. Listing uploads shows it, and IsProcessing reports the processing status instead of guessing from the offset and the scan result. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
TouchFile only returns an error here, so the coordinator reads the touched node back to learn its id and space owner or manager.
The coordinator persists the session after PrepareUpload, so it records the processing status there. Listing uploads then reports the session as processing while postprocessing runs. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
The fake driver's TouchFile and Delete only return an error, as ours do; the coordinator reads the touched node back through GetMD.
The processing filter follows the session status, so the spec sets it instead of relying on all bytes having arrived. A new spec checks that a session handed to postprocessing is persisted as processing. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
The postprocessing events gain the ResourceID the consumer uses to recognise its own storage. Events without one are accepted, so producers that do not set it keep working.
The coordinator's consumer now does what the decomposedfs one does: - each event runs in the trace context and with the initiator it carries - StartPostprocessingStep and the antivirus PostprocessingStepFinished record the step in the session status, and an aborting or deleting verdict marks the session failed - UploadReady names the parent of the uploaded file Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Co-authored-by: Firas Frikha <firas.frikha@kiteworks.com>
RollbackUpload takes the RollbackInfo the coordinator records in the session. When the node's metadata can no longer be read, the pkg fork falls back to the recorded node and parent ids: it reverts the size propagation and removes the node through the lookup, so posix drops the file and its id cache entry, and decomposedfs the parent's child entry. If the propagation fails, the node is kept so the rollback can be retried. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
The pkg fork implements OrphanChecker, and posix forwards it, so the coordinator can list orphaned upload sessions on both. The coordinator spec selects the processing session by its status. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
The decomposedfs half of this change is already in place; this adds the mapping to the coordinator's tus adapter.
Both trees honour the context flag the coordinator sets, since PrepareUpload propagates the new file anyway. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
MarkProcessing in the pkg fork no longer refuses a node another session marked: the last writer wins, and only the owning session can clear the mark. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
The posix changes are left to a follow-up commit, since our posix is built on the other decomposedfs copy.
…nto it ReadNode caches the node's attributes, not the revision's, when it reads a revision. CommitUpload wrote the scan result onto the revision a newer upload had kept for it, and on posix that write stored the cached attributes: the revision then claimed the newer upload's blob id and size. CommitUpload now drops the cache of the revision it writes into. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
…ploads The pkg fork's PrepareUpload creates a new file's node under the id the coordinator minted, marks the node as processing in the same attribute batch, reports the space owner of a new file, and purges a node it created when it fails. Posix and the decomposedfs tree remove what InitNewNode made when it fails; posix caches the new id only once the name is its own, and keeps the storage path out of the AlreadyExists error. When two uploads to the same file overlap, the older one survives whatever happens to the newer one, also with versioning disabled: - PrepareUpload snapshots a node another upload is still processing, and records that upload's processing mark on the revision - RollbackUpload restores the revision through the tree, so posix gets the content back too, and hands the mark back to the older upload - an upload that commits into such a revision clears the mark there - with versioning disabled, a commit to the live file drops the revisions that only served a rollback - rolling back the older upload drops the revision the newer one kept of it Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
…xtcloud driver TouchFile only returns an error here; the coordinator reads the new node back itself.
…the driver
Decomposedfs keeps consuming only the DeleteRevision event, in a
consumer group of its own ("dcfs-revisions"), since the coordinator
takes the "dcfs" group. The tus handler still resolves a finished
upload through the driver for the etag and permission headers.
…g fork The pkg fork no longer consumes postprocessing events. Like the legacy copy it only keeps a DeleteRevision consumer in the "dcfs-revisions" group, since the coordinator takes the "dcfs" group. - the coordinator resolves a finished tus upload for the etag and permission headers, so this keeps working once the drivers drop their session stores - the coordinator passes the staged file's path to CommitUpload, so posix can move it into place instead of copying it - pkg/upload uses go-viper/mapstructure like the rest of the repo The async upload specs of the pkg fork now drive uploads through the coordinator against the real driver. When the newer of two overlapping uploads is rolled back, the file is handed back to the older one, and when both fail, the file the older one created is removed and its quota released. For that, rolling back an upload that created the file while a newer one is processing marks the revision the newer upload kept, so its rollback purges the file; a commit removes such revisions. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
… retries are exhausted This also adds the reva_upload_sessions_commit_failed counter the retry reports, which came with the driver-side retry that the coordinator replaces.
…once When the coordinator rolls back a node but keeps the upload (an abort, a commit that failed after all retries, a cleanup that keeps the bytes), the session is now marked failed with the reason and its size diff is cleared. Listing uploads shows why the upload stopped, and a later cleanup of the kept session cannot release the quota a second time, which matters for an orphaned node whose rollback works from the session alone. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
The UploadSessionResolver interface stays: the coordinator implements it for the tus handler. The posix driver and the other decomposedfs copy are adapted in a follow-up commit.
The coordinator now owns the whole upload flow, so the pkg fork and posix drop their own session store, the tus data store methods, InitiateUpload, Upload, ListUploadSessions and ResolveUpload. The DisableVersioning aspect goes too: only that session store read it. The pkg fork's upload specs drive the uploads through a coordinator, including the over-quota upload that finishes through tus with a 507. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
… gateway OpenCloud signs data URLs in the proxy, at the edge, so its storages configure only the download endpoint. The session then signed a token against an empty data gateway endpoint, and postprocessing got a bare "/<token>" it could not download the staged bytes from: the virus scan never ran. Return the data server URL itself in that case, as the driver sessions did. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
The header was left out for new files, because their id used to be decided by the commit. A driver whose PrepareUpload creates the node does so under the id minted at initiate, so that id is the file's, and COPY, which reads the id from the upload response, needs it. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
With versioning disabled, a live commit removed every revision of the node, to clean up the snapshots an overlapping upload had kept for a rollback. That also removed the versions created before versioning was disabled. Mark those snapshots instead, and remove only revisions kept for a rollback. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
The coordinator checks the lock of a file before an upload, and stats the file when it finishes the upload. GetLock reported an unlocked file as locked by an empty token, so every upload to an existing file failed with 423. And the stat at finish runs with the upload's executant but without a token, so looking up the share failed and the upload with it. Report an unlocked file as unlocked, and stat via the service account when there is no token. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Antivirus downloads only the first bytes of a file larger than its scan size, with a Range header. The decomposedfs upload sessions served the staged bytes as a tus content server for that; the coordinator did not, so tusd sent the whole file and a file with a virus past the scan size got scanned and deleted anyway. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
The decomposedfs upload sessions synced every chunk right away, so an upload could be resumed after a crash and slow disks were spared one large flush at the end of the upload. Do the same in the coordinator's sessions, and log failed chunk writes as they did. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
… commit fails The decomposedfs upload sessions finished an upload without the request's cancellation, so a client going away could not leave it half done, and kept the staged bytes when a synchronous commit failed, as they are the only copy of the upload. Restore both in the coordinator: a failed synchronous commit now rolls back the node but keeps the session, marked failed, like a failed asynchronous one. Log why tusd could not finish an upload, as the sessions did. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
gowebdav formats a lock property the remote does not report as "<nil>", not as an empty string, so uploads to existing files in received shares still failed with 423. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
…ions The workers merged each child's permissions into the parent's permission set they all shared, so siblings leaked permissions into each other and the race detector flagged the concurrent writes. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
… mock The coordinator reads the node it touched back without metadata keys, so the request carries "mdKeys":null. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
The coordinator stages uploads in an upload directory, which the ocmreceived driver cannot derive, and commits them as the service account, so the cesnet storage and data providers share an upload directory and get a service account. UploadGateway announces the upload length like ocdav does: without it the coordinator takes the upload as empty and finishes it at initiate. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I'm trying to verify the changes are ultimately compatible with our posix driver, which is why this pr contains all coordinator related PRs. It also contains #906 and #896
Uploads now go through a driver-independent coordinator in
pkg/upload: storageprovider and dataprovider initiate, receive, postprocess and commit uploads through it. The drivers only implement PrepareUpload, CommitUpload, RollbackUpload and MarkProcessing. The legacy per-driver upload path and session stores are removed.Requires the corresponding OpenCloud changes (wiring the coordinator, listing upload sessions through it) before the reva bump.
This PR was created with the help of AI (Claude Code).