feat: add last_chunk_time to accurately clean up abandoned uploads without interrupting long transfers#2617
Merged
monkeyiq merged 3 commits intofilesender:development3from Mar 17, 2026
Conversation
added 2 commits
March 13, 2026 10:18
…thout interrupting long transfers
Author
Fix: "Forget it" button on resume-upload modalProblemWhen an upload failed and the user dismissed the resume modal with SolutionBefore clearing If the DELETE fails (e.g. the cron already cleaned it up), Modified file
|
monkeyiq
approved these changes
Mar 17, 2026
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.
Feature:
last_chunk_time— Precise Detection of Abandoned UploadsSummary
This PR introduces a new field
last_chunk_timeto thetransferstable and theTransferclass.Its purpose is to accurately track when the last file chunk was received for a transfer, enabling
the cleanup cron job to distinguish between genuinely abandoned uploads and large files that are
legitimately still uploading.
It also fixes an unrelated but critical bug in
allFailed()where date-only comparison caused aneffective cleanup window of 24–48 h instead of the configured value.
Motivation
Real-world context
This problem was encountered in a storage-constrained FileSender installation where:
whose transfer takes longer than 24 hours to complete. These must not be interrupted.
An incomplete transfer with status
uploadingis treated as "space in use", even if the userabandoned the upload days ago and no more chunks will ever arrive.
The combination of these three factors creates a compounding problem:
The original cleanup mechanism was too coarse
The cron job (
scripts/task/cron.php) callsTransfer::allFailed()to detect and delete stuckincomplete transfers. Before this change, the detection query was:
The
:datethreshold was computed asNOW() - failed_transfer_cleanup_days.This had two conflicting flaws that could not be resolved simultaneously:
failed_transfer_cleanup_daysto a small value (e.g. 1 day)failed_transfer_cleanup_daysto a large value (e.g. 7 days)There was no way to distinguish between a transfer that is actively uploading chunks and one
that was silently abandoned — both look identical from the
createdcolumn alone.The default value of
failed_transfer_cleanup_days = 7merely masked the problem for most users.Any installation with limited disk or with active large uploads was in an impossible bind.
What this PR enables
By tracking
last_chunk_time— the timestamp of the most recent chunk received — the system cannow make the correct distinction:
last_chunk_timestops advancing → cleaned up afterfailed_transfer_cleanup_daysof inactivity.last_chunk_timeadvances by at least one chunk per minute →never cleaned up, regardless of how long the transfer has been running.
This means
failed_transfer_cleanup_daysnow means exactly what its name says: days ofinactivity, not days since creation. A value of
1day is now safe and effective: it freesabandoned upload space within 24 hours while allowing a 10-day upload of a multi-TB file to
complete without any risk of premature deletion.
Changes
1.
classes/data/Transfer.class.php1a. New field in
dataMapAdds
last_chunk_timeas a nullable datetime column, mapped to the ORM.1b. New
FAILEDquery — useslast_chunk_timeinstead ofcreatedThe cleanup condition now checks inactivity (time since last chunk), not age (time since
creation). A transfer uploading a 500 GB file for 5 days is not abandoned — it last received a
chunk a minute ago.
1c. Property declaration
Required for PHP's magic method system. Without this explicit declaration,
__set()is triggeredon assignment and, since
last_chunk_timewas not in the whitelist, would throw aPropertyAccessException(HTTP 500 on everyPOST /rest.php/transfer).1d.
__get()whitelist, 'last_chunk_time'Added to the
in_arraylist inside__get()so$transfer->last_chunk_timecan be read withouttriggering
PropertyAccessException. Needed byFile::writeChunk().1e.
__set()handlerAllows writing
$transfer->last_chunk_time = time()safely from bothcreate()andFile::writeChunk().1f. Initialization in
create()Every new transfer gets
last_chunk_timeset to its creation timestamp. This ensures that if nochunk is ever uploaded (e.g. the user closes the browser before starting), the transfer will still
be cleaned up after
failed_transfer_cleanup_daysof inactivity.1g. Fix:
allFailed()— use full datetime, not date-onlyBefore:
After:
date('Y-m-d', ...)truncates to midnight, so the effective cleanup window is anywhere betweenNandN+1days depending on when the cron runs. Using'Y-m-d H:i:s'makes the thresholdexact: a transfer inactive for exactly
failed_transfer_cleanup_days × 24 hwill be eligible forcleanup regardless of the time of day.
2.
classes/data/File.class.php—writeChunk()updateEach time a chunk is successfully written, the transfer's
last_chunk_timeis updated. Thethrottle (
>= 60seconds) prevents excessiveUPDATEqueries: a 1 GB file with 5 MB chunksgenerates ~200 chunks — without throttling that would be 200
UPDATEstatements per file pertransfer. With throttling it is bounded to one per minute regardless of file count or chunk size.
3.
scripts/upgrade/database.php— schema migration + backfill3a. Column creation
The
updateTable()function (already present indatabase.php) detects and adds missing columns.It will automatically create
last_chunk_timeas aDATETIME NULLcolumn on thetransferstable for existing installations.
3b. Backfill for pre-existing rows
This backfill runs every time
database.phpis executed, but is idempotent: it only touches rowswhere
last_chunk_time IS NULL, so subsequent runs are no-ops.The distinction between active and non-active statuses during backfill is intentional:
created/started/uploading): settingNOW()grants them a freshfailed_transfer_cleanup_dayswindow, preventing immediate deletion on the next cron run.available/closed/etc.): the value doesn't matter for cleanup purposes, butcreatedis a semantically accurate fallback.Files changed
classes/data/Transfer.class.phpdataMap,FAILED, property,__get,__set,create(),allFailed()classes/data/File.class.phpwriteChunk()throttled update oflast_chunk_timescripts/upgrade/database.phpTesting
Manual test procedure
available).last_chunk_timeis populated:failed, deleting itin output) and the row is gone.last_chunk_time = NOW()is not deleted by the cron.Throttle verification
Upload a large file and observe the database during upload:
The timestamp should advance roughly once per minute, not on every chunk.
Backwards compatibility
failed_transfer_cleanup_dayssemantics are preserved; only the reference timestamp changesfrom "when the transfer was created" to "when the last chunk was received".