From f199b4ef9cd335f2a8042ccf3adab88c2867799d Mon Sep 17 00:00:00 2001 From: dzikoysk Date: Fri, 10 Nov 2023 01:32:56 +0100 Subject: [PATCH] GH-1975 Retry `Files.put` in `FileSystemStorageProvider` (Fix #1975) --- ...FileSystemStorageProviderIntegrationTest.kt | 1 + .../S3StorageProviderIntegrationTest.kt | 1 + .../com/reposilite/maven/RepositoryFactory.kt | 3 +++ .../com/reposilite/maven/RepositoryProvider.kt | 1 + .../com/reposilite/storage/StorageFacade.kt | 17 +++++++++++++++-- .../storage/StorageProviderFactory.kt | 9 ++++++++- .../filesystem/FileSystemQuotaProviders.kt | 10 ++++++++-- .../filesystem/FileSystemStorageProvider.kt | 16 ++++++++++++++-- .../FileSystemStorageProviderFactory.kt | 18 ++++++++++-------- .../storage/s3/S3StorageProviderFactory.kt | 12 ++++++++---- 10 files changed, 69 insertions(+), 19 deletions(-) diff --git a/reposilite-backend/src/integration/kotlin/com/reposilite/storage/infrastructure/FileSystemStorageProviderIntegrationTest.kt b/reposilite-backend/src/integration/kotlin/com/reposilite/storage/infrastructure/FileSystemStorageProviderIntegrationTest.kt index 896bc943b..3f763fb73 100644 --- a/reposilite-backend/src/integration/kotlin/com/reposilite/storage/infrastructure/FileSystemStorageProviderIntegrationTest.kt +++ b/reposilite-backend/src/integration/kotlin/com/reposilite/storage/infrastructure/FileSystemStorageProviderIntegrationTest.kt @@ -37,6 +37,7 @@ internal class FileSystemStorageProviderIntegrationTest : StorageProviderIntegra val storageFacade = StorageFacade() super.storageProvider = storageFacade.createStorageProvider( + journalist = logger, failureFacade = failureFacade, workingDirectory = rootDirectory.toPath(), repository = "test-storage", diff --git a/reposilite-backend/src/integration/kotlin/com/reposilite/storage/infrastructure/S3StorageProviderIntegrationTest.kt b/reposilite-backend/src/integration/kotlin/com/reposilite/storage/infrastructure/S3StorageProviderIntegrationTest.kt index a6e012edb..5cd16e562 100644 --- a/reposilite-backend/src/integration/kotlin/com/reposilite/storage/infrastructure/S3StorageProviderIntegrationTest.kt +++ b/reposilite-backend/src/integration/kotlin/com/reposilite/storage/infrastructure/S3StorageProviderIntegrationTest.kt @@ -47,6 +47,7 @@ internal class S3StorageProviderIntegrationTest : StorageProviderIntegrationTest val storageFacade = StorageFacade() this.storageProvider = storageFacade.createStorageProvider( + journalist = logger, failureFacade = failureFacade, workingDirectory = rootDirectory.toPath(), repository = "test-repository", diff --git a/reposilite-backend/src/main/kotlin/com/reposilite/maven/RepositoryFactory.kt b/reposilite-backend/src/main/kotlin/com/reposilite/maven/RepositoryFactory.kt index c0f59d8e9..280bf7b68 100644 --- a/reposilite-backend/src/main/kotlin/com/reposilite/maven/RepositoryFactory.kt +++ b/reposilite-backend/src/main/kotlin/com/reposilite/maven/RepositoryFactory.kt @@ -17,6 +17,7 @@ package com.reposilite.maven import com.reposilite.auth.AuthenticationFacade +import com.reposilite.journalist.Journalist import com.reposilite.maven.application.MirroredRepositorySettings import com.reposilite.maven.application.RepositorySettings import com.reposilite.shared.http.RemoteClientProvider @@ -28,6 +29,7 @@ import java.nio.file.Paths import java.util.UUID internal class RepositoryFactory( + private val journalist: Journalist, private val workingDirectory: Path, private val authenticationFacade: AuthenticationFacade, private val remoteClientProvider: RemoteClientProvider, @@ -49,6 +51,7 @@ internal class RepositoryFactory( storageProvider = storageFacade .createStorageProvider( + journalist = journalist, failureFacade = failureFacade, workingDirectory = workingDirectory.resolve(repositoriesDirectory), repository = repositoryName, diff --git a/reposilite-backend/src/main/kotlin/com/reposilite/maven/RepositoryProvider.kt b/reposilite-backend/src/main/kotlin/com/reposilite/maven/RepositoryProvider.kt index ae83ffeff..df40254bb 100644 --- a/reposilite-backend/src/main/kotlin/com/reposilite/maven/RepositoryProvider.kt +++ b/reposilite-backend/src/main/kotlin/com/reposilite/maven/RepositoryProvider.kt @@ -65,6 +65,7 @@ internal class RepositoryProvider( private fun createRepositories(repositoriesConfiguration: List): Map { val factory = RepositoryFactory( + journalist = journalist, workingDirectory = workingDirectory, authenticationFacade = authenticationFacade, remoteClientProvider = remoteClientProvider, diff --git a/reposilite-backend/src/main/kotlin/com/reposilite/storage/StorageFacade.kt b/reposilite-backend/src/main/kotlin/com/reposilite/storage/StorageFacade.kt index 1535657b6..2c1d4cd65 100644 --- a/reposilite-backend/src/main/kotlin/com/reposilite/storage/StorageFacade.kt +++ b/reposilite-backend/src/main/kotlin/com/reposilite/storage/StorageFacade.kt @@ -16,6 +16,7 @@ package com.reposilite.storage +import com.reposilite.journalist.Journalist import com.reposilite.plugin.api.Facade import com.reposilite.status.FailureFacade import java.nio.file.Path @@ -28,7 +29,19 @@ class StorageFacade : Facade { .associateBy { it.type } .mapValues { (_, factory) -> factory as StorageProviderFactory<*, StorageProviderSettings> } - fun createStorageProvider(failureFacade: FailureFacade, workingDirectory: Path, repository: String, storageSettings: StorageProviderSettings): StorageProvider? = - storageProviderFactories[storageSettings.type]?.create(failureFacade, workingDirectory, repository, storageSettings) + fun createStorageProvider( + journalist: Journalist, + failureFacade: FailureFacade, + workingDirectory: Path, + repository: String, + storageSettings: StorageProviderSettings + ): StorageProvider? = + storageProviderFactories[storageSettings.type]?.create( + journalist = journalist, + failureFacade = failureFacade, + workingDirectory = workingDirectory, + repositoryName = repository, + settings = storageSettings + ) } diff --git a/reposilite-backend/src/main/kotlin/com/reposilite/storage/StorageProviderFactory.kt b/reposilite-backend/src/main/kotlin/com/reposilite/storage/StorageProviderFactory.kt index 6aab5418c..9d8883498 100644 --- a/reposilite-backend/src/main/kotlin/com/reposilite/storage/StorageProviderFactory.kt +++ b/reposilite-backend/src/main/kotlin/com/reposilite/storage/StorageProviderFactory.kt @@ -16,6 +16,7 @@ package com.reposilite.storage +import com.reposilite.journalist.Journalist import com.reposilite.status.FailureFacade import java.nio.file.Path @@ -24,6 +25,12 @@ interface StorageProviderFactory - fun create(failureFacade: FailureFacade, workingDirectory: Path, repositoryName: String, settings: SETTINGS): PROVIDER + fun create( + journalist: Journalist, + failureFacade: FailureFacade, + workingDirectory: Path, + repositoryName: String, + settings: SETTINGS + ): PROVIDER } diff --git a/reposilite-backend/src/main/kotlin/com/reposilite/storage/filesystem/FileSystemQuotaProviders.kt b/reposilite-backend/src/main/kotlin/com/reposilite/storage/filesystem/FileSystemQuotaProviders.kt index 36e143aa6..13d052c4e 100644 --- a/reposilite-backend/src/main/kotlin/com/reposilite/storage/filesystem/FileSystemQuotaProviders.kt +++ b/reposilite-backend/src/main/kotlin/com/reposilite/storage/filesystem/FileSystemQuotaProviders.kt @@ -16,6 +16,7 @@ package com.reposilite.storage.filesystem +import com.reposilite.journalist.Journalist import com.reposilite.shared.ErrorResponse import com.reposilite.shared.toErrorResponse import io.javalin.http.HttpStatus.INSUFFICIENT_STORAGE @@ -27,7 +28,11 @@ import java.nio.file.Path * @param rootDirectory root directory of storage space * @param maxSize the largest amount of storage available for use, in bytes */ -internal class FixedQuota(rootDirectory: Path, private val maxSize: Long) : FileSystemStorageProvider(rootDirectory) { +internal class FixedQuota( + journalist: Journalist, + rootDirectory: Path, + private val maxSize: Long +) : FileSystemStorageProvider(journalist, rootDirectory) { init { if (maxSize <= 0) { @@ -47,9 +52,10 @@ internal class FixedQuota(rootDirectory: Path, private val maxSize: Long) : File * @param maxPercentage the maximum percentage of the disk available for use */ internal class PercentageQuota( + journalist: Journalist, rootDirectory: Path, private val maxPercentage: Double -) : FileSystemStorageProvider(rootDirectory) { +) : FileSystemStorageProvider(journalist, rootDirectory) { init { if (maxPercentage > 1 || maxPercentage <= 0) { diff --git a/reposilite-backend/src/main/kotlin/com/reposilite/storage/filesystem/FileSystemStorageProvider.kt b/reposilite-backend/src/main/kotlin/com/reposilite/storage/filesystem/FileSystemStorageProvider.kt index 547612cc5..e0bbbcb9d 100644 --- a/reposilite-backend/src/main/kotlin/com/reposilite/storage/filesystem/FileSystemStorageProvider.kt +++ b/reposilite-backend/src/main/kotlin/com/reposilite/storage/filesystem/FileSystemStorageProvider.kt @@ -16,6 +16,7 @@ package com.reposilite.storage.filesystem +import com.reposilite.journalist.Journalist import com.reposilite.shared.ErrorResponse import com.reposilite.shared.badRequest import com.reposilite.shared.notFound @@ -43,15 +44,18 @@ import java.io.File import java.io.IOException import java.io.InputStream import java.nio.file.Files +import java.nio.file.NoSuchFileException import java.nio.file.Path import java.nio.file.StandardCopyOption.REPLACE_EXISTING import java.nio.file.attribute.FileTime +import kotlin.io.path.absolutePathString import kotlin.streams.asSequence /** * @param rootDirectory root directory of storage space */ abstract class FileSystemStorageProvider protected constructor( + val journalist: Journalist, val rootDirectory: Path ) : StorageProvider { @@ -78,8 +82,16 @@ abstract class FileSystemStorageProvider protected constructor( data.copyTo(destination) } - Files.move(temporaryFile.toPath(), file, REPLACE_EXISTING) - Unit + do { + try { + Files.move(temporaryFile.toPath(), file, REPLACE_EXISTING) + } catch (e: NoSuchFileException) { + // Concurrent Files.move calls may throw + // ~ https://github.com/dzikoysk/reposilite/issues/1975 + journalist.logger.debug("[FS] Cannot move file ${temporaryFile.absolutePath} to ${file.absolutePathString()}, retrying...") + Thread.sleep(100) // probably good enough for now + } + } while (Files.exists(temporaryFile.toPath())) } } diff --git a/reposilite-backend/src/main/kotlin/com/reposilite/storage/filesystem/FileSystemStorageProviderFactory.kt b/reposilite-backend/src/main/kotlin/com/reposilite/storage/filesystem/FileSystemStorageProviderFactory.kt index 4ce91713c..c50527402 100644 --- a/reposilite-backend/src/main/kotlin/com/reposilite/storage/filesystem/FileSystemStorageProviderFactory.kt +++ b/reposilite-backend/src/main/kotlin/com/reposilite/storage/filesystem/FileSystemStorageProviderFactory.kt @@ -16,6 +16,7 @@ package com.reposilite.storage.filesystem +import com.reposilite.journalist.Journalist import com.reposilite.status.FailureFacade import com.reposilite.storage.StorageProviderFactory import java.nio.file.Files @@ -34,26 +35,26 @@ class FileSystemStorageProviderFactory : StorageProviderFactory = diff --git a/reposilite-backend/src/main/kotlin/com/reposilite/storage/s3/S3StorageProviderFactory.kt b/reposilite-backend/src/main/kotlin/com/reposilite/storage/s3/S3StorageProviderFactory.kt index 8f3185fa3..c77efe5d1 100644 --- a/reposilite-backend/src/main/kotlin/com/reposilite/storage/s3/S3StorageProviderFactory.kt +++ b/reposilite-backend/src/main/kotlin/com/reposilite/storage/s3/S3StorageProviderFactory.kt @@ -16,23 +16,27 @@ package com.reposilite.storage.s3 +import com.reposilite.journalist.Journalist import com.reposilite.status.FailureFacade import com.reposilite.storage.StorageProviderFactory import software.amazon.awssdk.auth.credentials.AwsBasicCredentials import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider -import software.amazon.awssdk.core.exception.SdkClientException import software.amazon.awssdk.regions.Region import software.amazon.awssdk.services.s3.S3Client import software.amazon.awssdk.services.s3.S3Configuration -import software.amazon.awssdk.services.s3.model.S3Exception import java.net.URI import java.nio.file.Path class S3StorageProviderFactory : StorageProviderFactory { - override fun create(failureFacade: FailureFacade, workingDirectory: Path, repositoryName: String, settings: S3StorageProviderSettings): S3StorageProvider { + override fun create( + journalist: Journalist, + failureFacade: FailureFacade, + workingDirectory: Path, + repositoryName: String, + settings: S3StorageProviderSettings + ): S3StorageProvider { val client = S3Client.builder() - val pathStyleAccessEnabled = System.getProperty("reposilite.s3.pathStyleAccessEnabled") == "true" if (pathStyleAccessEnabled) {