Skip to content

feat(objectstore): add configurable S3 retry attempts #56502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kesselb merged 1 commit into from
Nov 19, 2025
+23 −1
Merged

feat(objectstore): add configurable S3 retry attempts #56502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions config/config.sample.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1964,6 +1964,25 @@
],
],

/**
* To use S3 object storage
*/
'objectstore' => [
'class' => 'OC\\Files\\ObjectStore\\S3',
'arguments' => [
'bucket' => 'nextcloud',
'key' => 'your-access-key',
'secret' => 'your-secret-key',
'hostname' => 's3.example.com',
'port' => 443,
'use_ssl' => true,
'region' => 'us-east-1',
// optional: Maximum number of retry attempts for failed S3 requests
// Default: 5
'retriesMaxAttempts' => 5,
Comment on lines +1980 to +1982
Copy link
Contributor

@artonge artonge Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we add that to the existing entry line 1905?

server/config/config.sample.php

Lines 1905 to 1936 in f2f9cb7

'objectstore' => [
'class' => 'OC\\Files\\ObjectStore\\Swift',
'arguments' => [
// trystack will use your Facebook ID as the username
'username' => 'facebook100000123456789',
// in the trystack dashboard, go to user -> settings -> API Password to
// generate a password
'password' => 'Secr3tPaSSWoRdt7',
// must already exist in the objectstore, name can be different
'container' => 'nextcloud',
// prefix to prepend to the fileid, default is 'oid:urn:'
'objectPrefix' => 'oid:urn:',
// create the container if it does not exist. default is false
'autocreate' => true,
// required, dev-/trystack defaults to 'RegionOne'
'region' => 'RegionOne',
// The Identity / Keystone endpoint
'url' => 'http://8.21.28.222:5000/v2.0',
// uploadPartSize: size of the uploaded chunks, defaults to 524288000
'uploadPartSize' => 524288000,
// required on dev-/trystack
'tenantName' => 'facebook100000123456789',
// dev-/trystack uses swift by default, the lib defaults to 'cloudFiles'
// if omitted
'serviceName' => 'swift',
// The Interface / URL Type, optional
'urlType' => 'internal',
// Maximum amount of data that can be uploaded
'totalSizeLimit' => 1024 * 1024 * 1024,
],
],

Copy link
Contributor Author

@nfebe nfebe Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that is for swift not s3?

Copy link
Contributor

@artonge artonge Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, weird that we have an entry for swift only.
We should also add some documentation here then: https://docs.nextcloud.com/server/latest/admin_manual/configuration_files/primary_storage.html#simple-storage-service-s3

Copy link
Contributor

@artonge artonge Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep your change then :)

Copy link
Member

@tcitworld tcitworld Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to document all supported options here too then: for instance totalSizeLimit, objectPrefix, use_path_style, concurrency, proxy, connect_timeout, timeout, uploadPartSize, putSizeLimit, useMultipartCopy, copySizeLimit, legacy_auth, version, verify_bucket_exists, autocreate, storageClass (and there's even more in S3ConnectionTrait).

Maybe just a bunch of common ones?

Some of there are not in the documentation as well.

Copy link
Contributor

@kesselb kesselb Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I've overlooked the comment 🙈
Sounds good to me 👍

],
],

/**
* If this is set to true and a multibucket object store is configured, then
* newly created previews are put into 256 dedicated buckets.
Expand Down
2 changes: 2 additions & 0 deletions lib/private/Files/ObjectStore/S3ConfigTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ trait S3ConfigTrait {
private int|float $copySizeLimit;

private bool $useMultipartCopy = true;

protected int $retriesMaxAttempts;
}
3 changes: 2 additions & 1 deletion lib/private/Files/ObjectStore/S3ConnectionTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ protected function parseParams($params) {
$this->putSizeLimit = $params['putSizeLimit'] ?? 104857600;
$this->copySizeLimit = $params['copySizeLimit'] ?? 5242880000;
$this->useMultipartCopy = (bool)($params['useMultipartCopy'] ?? true);
$this->retriesMaxAttempts = $params['retriesMaxAttempts'] ?? 5;
$params['region'] = empty($params['region']) ? 'eu-west-1' : $params['region'];
$params['hostname'] = empty($params['hostname']) ? 's3.' . $params['region'] . '.amazonaws.com' : $params['hostname'];
$params['s3-accelerate'] = $params['hostname'] === 's3-accelerate.amazonaws.com' || $params['hostname'] === 's3-accelerate.dualstack.amazonaws.com';
Expand Down Expand Up @@ -109,7 +110,7 @@ public function getConnection() {
'use_aws_shared_config_files' => false,
'retries' => [
'mode' => 'standard',
'max_attempts' => 5,
'max_attempts' => $this->retriesMaxAttempts,
],
];

Expand Down
Loading