Add configurable upload timeout for parcel uploads
Problem
The access logs uploader was using context.TODO() when uploading parcels to object storage, which meant there was no timeout for upload operations. This could cause uploads to hang indefinitely if the storage backend became unresponsive or experienced network issues.
The issue was identified at: https://github.com/storj/common/blob/b9952c61d22febe124585cb2529231854d493a61/accesslogs/uploader.go#L195-L197
Solution
This PR adds a configurable UploadTimeout option that sets a maximum duration for each individual parcel upload operation:
-
New configuration field:
UploadTimeoutinOptions.UploadingOptionswith a default of 30 seconds -
Context with timeout: Replaced
context.TODO()withcontext.WithTimeout()in the upload loop - Per-parcel timeout: Each upload carries its own timeout value, allowing for future per-parcel configuration
- User-configurable: Users can adjust the timeout via the config struct to match their storage backend characteristics
Changes
- Added
uploadTimeoutfield touploadstruct to make timeout scoped to individual parcels - Added
uploadTimeoutfield tosequentialUploaderOptionsandsequentialUploaderstructs - Updated
Options.UploadingOptionsto includeUploadTimeout time.Durationwith a default of 30 seconds - Modified
sequentialUploader.run()to use the timeout from each individual upload - Updated both
queueUploadandqueueUploadWithoutQueueLimitto pass the timeout when creating uploads - Updated all existing tests to include the new timeout parameter
- Added
TestUploadTimeoutto verify that uploads properly respect the configured timeout
Testing
All existing tests pass, and a new test specifically validates timeout behavior by:
- Creating a storage mock that delays longer than the configured timeout
- Verifying the upload is cancelled with
context.DeadlineExceeded - Confirming the retry mechanism handles timeouts appropriately
Example usage:
processor := accesslogs.NewProcessor(log, accesslogs.Options{
UploadingOptions: struct {
UploadTimeout: 2 * time.Minute, // Custom timeout
},
})
Fixes the TODO comment and provides robust timeout handling for production deployments.
Fixes storj/edge#555
Original prompt
This section details on the original issue you should resolve
<issue_title>add timeouts for the parcel uploads</issue_title> <issue_description>In the current server access logging implementation, we haven't set a robust timeout for the parcel uploads:
https://github.com/storj/common/blob/b9952c61d22febe124585cb2529231854d493a61/accesslogs/uploader.go#L195-L197
We need to add one; perhaps make it configurable.</issue_description>
Comments on the Issue (you are @copilot in this section)
Fixes storj/edge#555
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.
@copilot could you make the timeout not be global but scoped to an individual parcel?
Made the timeout scoped to individual parcels in commit 9ef9a2c. Each upload struct now carries its own uploadTimeout field, and the timeout is read from the upload when creating the context in the run loop.