From 67d0307e66d4f55050ddafe1a8ee1b9e5642c9f0 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 12 Nov 2025 14:14:06 +0100 Subject: [PATCH 1/2] perf(s3): Cache whether bucket exists Otherwise, we call doesBucketExist all the time which does a network request to the S3 server adding some non-trivial latency when creating a S3 connection object. Signed-off-by: Carl Schwan --- .../Files/ObjectStore/S3ConnectionTrait.php | 47 +++++++++++++------ 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/lib/private/Files/ObjectStore/S3ConnectionTrait.php b/lib/private/Files/ObjectStore/S3ConnectionTrait.php index 71fa2b456275f..6f9d6ac7a97c4 100644 --- a/lib/private/Files/ObjectStore/S3ConnectionTrait.php +++ b/lib/private/Files/ObjectStore/S3ConnectionTrait.php @@ -15,6 +15,8 @@ use GuzzleHttp\Promise\Create; use GuzzleHttp\Promise\RejectedPromise; use OCP\Files\StorageNotAvailableException; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\ICertificateManager; use OCP\Server; use Psr\Log\LoggerInterface; @@ -28,6 +30,8 @@ trait S3ConnectionTrait { protected ?S3Client $connection = null; + private ?ICache $existingBucketsCache = null; + protected function parseParams($params) { if (empty($params['bucket'])) { throw new \Exception('Bucket has to be configured.'); @@ -81,6 +85,11 @@ public function getConnection() { return $this->connection; } + if ($this->existingBucketsCache === null) { + $this->existingBucketsCache = Server::get(ICacheFactory::class) + ->createLocal('s3-bucket-exists-cache'); + } + $scheme = (isset($this->params['use_ssl']) && $this->params['use_ssl'] === false) ? 'http' : 'https'; $base_url = $scheme . '://' . $this->params['hostname'] . ':' . $this->params['port'] . '/'; @@ -142,22 +151,30 @@ public function getConnection() { ['app' => 'objectstore']); } - if ($this->params['verify_bucket_exists'] && !$this->connection->doesBucketExist($this->bucket)) { - try { - $logger->info('Bucket "' . $this->bucket . '" does not exist - creating it.', ['app' => 'objectstore']); - if (!$this->connection::isBucketDnsCompatible($this->bucket)) { - throw new StorageNotAvailableException('The bucket will not be created because the name is not dns compatible, please correct it: ' . $this->bucket); - } - $this->connection->createBucket(['Bucket' => $this->bucket]); - $this->testTimeout(); - } catch (S3Exception $e) { - $logger->debug('Invalid remote storage.', [ - 'exception' => $e, - 'app' => 'objectstore', - ]); - if ($e->getAwsErrorCode() !== 'BucketAlreadyOwnedByYou') { - throw new StorageNotAvailableException('Creation of bucket "' . $this->bucket . '" failed. ' . $e->getMessage()); + if ($this->params['verify_bucket_exists']) { + $cacheKey = $this->params['hostname'] . $this->bucket; + $exist = $this->existingBucketsCache->get($cacheKey) === 1; + + if (!$exist && !$this->connection->doesBucketExist($this->bucket)) { + try { + $logger->info('Bucket "' . $this->bucket . '" does not exist - creating it.', ['app' => 'objectstore']); + if (!$this->connection::isBucketDnsCompatible($this->bucket)) { + throw new StorageNotAvailableException('The bucket will not be created because the name is not dns compatible, please correct it: ' . $this->bucket); + } + $this->connection->createBucket(['Bucket' => $this->bucket]); + $this->testTimeout(); + $this->existingBucketsCache->set($cacheKey, 1); + } catch (S3Exception $e) { + $logger->debug('Invalid remote storage.', [ + 'exception' => $e, + 'app' => 'objectstore', + ]); + if ($e->getAwsErrorCode() !== 'BucketAlreadyOwnedByYou') { + throw new StorageNotAvailableException('Creation of bucket "' . $this->bucket . '" failed. ' . $e->getMessage()); + } } + } elseif (!$exist) { + $this->existingBucketsCache->set($cacheKey, 1); } } From a228ffa641cabc666de9021acb903f0fd10b8250 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 20 Nov 2025 16:50:14 +0100 Subject: [PATCH 2/2] refactor(s3): Readability Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com> Signed-off-by: Carl Schwan --- .../Files/ObjectStore/S3ConnectionTrait.php | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/private/Files/ObjectStore/S3ConnectionTrait.php b/lib/private/Files/ObjectStore/S3ConnectionTrait.php index 6f9d6ac7a97c4..c01e54e1057a6 100644 --- a/lib/private/Files/ObjectStore/S3ConnectionTrait.php +++ b/lib/private/Files/ObjectStore/S3ConnectionTrait.php @@ -155,25 +155,25 @@ public function getConnection() { $cacheKey = $this->params['hostname'] . $this->bucket; $exist = $this->existingBucketsCache->get($cacheKey) === 1; - if (!$exist && !$this->connection->doesBucketExist($this->bucket)) { - try { - $logger->info('Bucket "' . $this->bucket . '" does not exist - creating it.', ['app' => 'objectstore']); - if (!$this->connection::isBucketDnsCompatible($this->bucket)) { - throw new StorageNotAvailableException('The bucket will not be created because the name is not dns compatible, please correct it: ' . $this->bucket); - } - $this->connection->createBucket(['Bucket' => $this->bucket]); - $this->testTimeout(); - $this->existingBucketsCache->set($cacheKey, 1); - } catch (S3Exception $e) { - $logger->debug('Invalid remote storage.', [ - 'exception' => $e, - 'app' => 'objectstore', - ]); - if ($e->getAwsErrorCode() !== 'BucketAlreadyOwnedByYou') { - throw new StorageNotAvailableException('Creation of bucket "' . $this->bucket . '" failed. ' . $e->getMessage()); + if (!$exist) { + if (!$this->connection->doesBucketExist($this->bucket)) { + try { + $logger->info('Bucket "' . $this->bucket . '" does not exist - creating it.', ['app' => 'objectstore']); + if (!$this->connection::isBucketDnsCompatible($this->bucket)) { + throw new StorageNotAvailableException('The bucket will not be created because the name is not dns compatible, please correct it: ' . $this->bucket); + } + $this->connection->createBucket(['Bucket' => $this->bucket]); + $this->testTimeout(); + } catch (S3Exception $e) { + $logger->debug('Invalid remote storage.', [ + 'exception' => $e, + 'app' => 'objectstore', + ]); + if ($e->getAwsErrorCode() !== 'BucketAlreadyOwnedByYou') { + throw new StorageNotAvailableException('Creation of bucket "' . $this->bucket . '" failed. ' . $e->getMessage()); + } } } - } elseif (!$exist) { $this->existingBucketsCache->set($cacheKey, 1); } }