Skip to content

Commit 67d0307

Browse files
committed
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 <[email protected]>
1 parent fcf1406 commit 67d0307

File tree

1 file changed

+32
-15
lines changed

1 file changed

+32
-15
lines changed

lib/private/Files/ObjectStore/S3ConnectionTrait.php

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
use GuzzleHttp\Promise\Create;
1616
use GuzzleHttp\Promise\RejectedPromise;
1717
use OCP\Files\StorageNotAvailableException;
18+
use OCP\ICache;
19+
use OCP\ICacheFactory;
1820
use OCP\ICertificateManager;
1921
use OCP\Server;
2022
use Psr\Log\LoggerInterface;
@@ -28,6 +30,8 @@ trait S3ConnectionTrait {
2830

2931
protected ?S3Client $connection = null;
3032

33+
private ?ICache $existingBucketsCache = null;
34+
3135
protected function parseParams($params) {
3236
if (empty($params['bucket'])) {
3337
throw new \Exception('Bucket has to be configured.');
@@ -81,6 +85,11 @@ public function getConnection() {
8185
return $this->connection;
8286
}
8387

88+
if ($this->existingBucketsCache === null) {
89+
$this->existingBucketsCache = Server::get(ICacheFactory::class)
90+
->createLocal('s3-bucket-exists-cache');
91+
}
92+
8493
$scheme = (isset($this->params['use_ssl']) && $this->params['use_ssl'] === false) ? 'http' : 'https';
8594
$base_url = $scheme . '://' . $this->params['hostname'] . ':' . $this->params['port'] . '/';
8695

@@ -142,22 +151,30 @@ public function getConnection() {
142151
['app' => 'objectstore']);
143152
}
144153

145-
if ($this->params['verify_bucket_exists'] && !$this->connection->doesBucketExist($this->bucket)) {
146-
try {
147-
$logger->info('Bucket "' . $this->bucket . '" does not exist - creating it.', ['app' => 'objectstore']);
148-
if (!$this->connection::isBucketDnsCompatible($this->bucket)) {
149-
throw new StorageNotAvailableException('The bucket will not be created because the name is not dns compatible, please correct it: ' . $this->bucket);
150-
}
151-
$this->connection->createBucket(['Bucket' => $this->bucket]);
152-
$this->testTimeout();
153-
} catch (S3Exception $e) {
154-
$logger->debug('Invalid remote storage.', [
155-
'exception' => $e,
156-
'app' => 'objectstore',
157-
]);
158-
if ($e->getAwsErrorCode() !== 'BucketAlreadyOwnedByYou') {
159-
throw new StorageNotAvailableException('Creation of bucket "' . $this->bucket . '" failed. ' . $e->getMessage());
154+
if ($this->params['verify_bucket_exists']) {
155+
$cacheKey = $this->params['hostname'] . $this->bucket;
156+
$exist = $this->existingBucketsCache->get($cacheKey) === 1;
157+
158+
if (!$exist && !$this->connection->doesBucketExist($this->bucket)) {
159+
try {
160+
$logger->info('Bucket "' . $this->bucket . '" does not exist - creating it.', ['app' => 'objectstore']);
161+
if (!$this->connection::isBucketDnsCompatible($this->bucket)) {
162+
throw new StorageNotAvailableException('The bucket will not be created because the name is not dns compatible, please correct it: ' . $this->bucket);
163+
}
164+
$this->connection->createBucket(['Bucket' => $this->bucket]);
165+
$this->testTimeout();
166+
$this->existingBucketsCache->set($cacheKey, 1);
167+
} catch (S3Exception $e) {
168+
$logger->debug('Invalid remote storage.', [
169+
'exception' => $e,
170+
'app' => 'objectstore',
171+
]);
172+
if ($e->getAwsErrorCode() !== 'BucketAlreadyOwnedByYou') {
173+
throw new StorageNotAvailableException('Creation of bucket "' . $this->bucket . '" failed. ' . $e->getMessage());
174+
}
160175
}
176+
} elseif (!$exist) {
177+
$this->existingBucketsCache->set($cacheKey, 1);
161178
}
162179
}
163180

0 commit comments

Comments
 (0)