diff options
author | Harald Musum <musum@yahooinc.com> | 2022-11-16 13:03:05 +0100 |
---|---|---|
committer | Harald Musum <musum@yahooinc.com> | 2022-11-16 13:03:05 +0100 |
commit | c4be6169c8412ab8e0c53aa35e9144f8f99acc3c (patch) | |
tree | eae5ca338ba1fa09b9276572bf4d48be1941af33 /configserver | |
parent | 4619fc010cf5cab1d9ffa16c2c128c0880eeb54f (diff) |
Use lock for adding file reference (controlled by feature flag)
Add method for deleting file references (not used yet)
Diffstat (limited to 'configserver')
15 files changed, 73 insertions, 41 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java index 91272f4e426..020a28b9ecd 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java @@ -4,10 +4,15 @@ package com.yahoo.vespa.config.server.filedistribution; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.component.AbstractComponent; import com.yahoo.component.annotation.Inject; +import com.yahoo.concurrent.Lock; +import com.yahoo.concurrent.Locks; import com.yahoo.config.FileReference; import com.yahoo.io.IOUtils; import com.yahoo.text.Utf8; import com.yahoo.vespa.defaults.Defaults; +import com.yahoo.vespa.flags.BooleanFlag; +import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; import net.jpountz.xxhash.XXHash64; import net.jpountz.xxhash.XXHashFactory; import java.io.File; @@ -21,6 +26,7 @@ import java.nio.channels.FileChannel; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; +import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -29,15 +35,20 @@ import static com.yahoo.yolean.Exceptions.uncheck; public class FileDirectory extends AbstractComponent { private static final Logger log = Logger.getLogger(FileDirectory.class.getName()); + + private final Locks<FileReference> locks = new Locks<>(1, TimeUnit.MINUTES); + private final File root; + private final BooleanFlag useLock; @Inject - public FileDirectory(ConfigserverConfig configserverConfig) { - this(new File(Defaults.getDefaults().underVespaHome(configserverConfig.fileReferencesDir()))); + public FileDirectory(ConfigserverConfig configserverConfig, FlagSource flagSource) { + this(new File(Defaults.getDefaults().underVespaHome(configserverConfig.fileReferencesDir())), flagSource); } - public FileDirectory(File rootDir) { - root = rootDir; + public FileDirectory(File rootDir, FlagSource flagSource) { + this.root = rootDir; + this.useLock = Flags.USE_LOCKS_IN_FILEDISTRIBUTION.bindTo(flagSource); try { ensureRootExist(); } catch (IllegalArgumentException e) { @@ -109,10 +120,21 @@ public class FileDirectory extends AbstractComponent { Long hash = computeHash(source); FileReference fileReference = fileReferenceFromHash(hash); - if (shouldAddFile(source, hash)) - return addFile(source, fileReference); + if (useLock.value()) + try (Lock lock = locks.lock(fileReference)) { + return addFile(source, fileReference, hash); + } + else + return addFile(source, fileReference, hash); + } - return fileReference; + public void delete(FileReference fileReference) { + if (useLock.value()) + try (Lock lock = locks.lock(fileReference)) { + IOUtils.recursiveDeleteDir(destinationDir(fileReference)); + } + else + IOUtils.recursiveDeleteDir(destinationDir(fileReference)); } // Check if we should add file, it might already exist @@ -143,7 +165,9 @@ public class FileDirectory extends AbstractComponent { } // Pre-condition: Destination dir does not exist - private FileReference addFile(File source, FileReference reference) { + private FileReference addFile(File source, FileReference reference, Long hash) throws IOException { + if ( ! shouldAddFile(source, hash)) return reference; + ensureRootExist(); Path tempDestinationDir = uncheck(() -> Files.createTempDirectory(root.toPath(), "writing")); try { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java index 319dc85b89c..754781e16f5 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java @@ -122,12 +122,14 @@ public class ApplicationRepositoryTest { .configDefinitionsDir(temporaryFolder.newFolder().getAbsolutePath()) .fileReferencesDir(temporaryFolder.newFolder().getAbsolutePath()) .build(); + InMemoryFlagSource flagSource = new InMemoryFlagSource(); tenantRepository = new TestTenantRepository.Builder() .withClock(clock) .withConfigserverConfig(configserverConfig) .withCurator(curator) - .withFileDistributionFactory(new MockFileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig))) - .withFlagSource(new InMemoryFlagSource()) + .withFileDistributionFactory( + new MockFileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig, flagSource))) + .withFlagSource(flagSource) .build(); tenantRepository.addTenant(TenantRepository.HOSTED_VESPA_TENANT); tenantRepository.addTenant(tenant1); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java index 0721477ade3..1ed4fac10dc 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java @@ -291,7 +291,8 @@ public class DeployTester { .withClock(clock) .withConfigserverConfig(configserverConfig) .withCurator(curator) - .withFileDistributionFactory(new MockFileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig))) + .withFileDistributionFactory( + new MockFileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig, flagSource))) .withMetrics(Optional.ofNullable(metrics).orElse(Metrics.createTestMetrics())) .withModelFactoryRegistry((new ModelFactoryRegistry(modelFactories))) .withZone(zone); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistryTestCase.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistryTestCase.java index 51345e50d71..4ea2e66d799 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistryTestCase.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistryTestCase.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.config.server.filedistribution; import com.yahoo.config.FileReference; import com.yahoo.config.application.api.FileRegistry; +import com.yahoo.vespa.flags.InMemoryFlagSource; import org.junit.Test; import org.junit.rules.TemporaryFolder; import java.io.File; @@ -43,7 +44,8 @@ public class FileDBRegistryTestCase { public void importAndExport() throws IOException { TemporaryFolder tmpDir = new TemporaryFolder(); tmpDir.create(); - AddFileInterface fileManager = new ApplicationFileManager(new File(APP), new FileDirectory(tmpDir.newFolder()), false); + AddFileInterface fileManager = + new ApplicationFileManager(new File(APP), new FileDirectory(tmpDir.newFolder(), new InMemoryFlagSource()), false); FileRegistry fileRegistry = new FileDBRegistry(fileManager); assertEquals(FOO_REF, fileRegistry.addFile(FOO_FILE)); try { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDirectoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDirectoryTest.java index e5ef893d135..b346ef65bba 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDirectoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDirectoryTest.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.config.server.filedistribution; import com.yahoo.config.FileReference; import com.yahoo.io.IOUtils; +import com.yahoo.vespa.flags.InMemoryFlagSource; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -30,7 +31,7 @@ public class FileDirectoryTest { @Before public void setup() { - fileDirectory = new FileDirectory(temporaryFolder.getRoot()); + fileDirectory = new FileDirectory(temporaryFolder.getRoot(), new InMemoryFlagSource()); } @Test @@ -46,8 +47,6 @@ public class FileDirectoryTest { @Test public void requireThatFileReferenceWithSubDirectoriesWorks() throws IOException { - FileDirectory fileDirectory = new FileDirectory(temporaryFolder.getRoot()); - String subdirName = "subdir"; File subDirectory = new File(temporaryFolder.getRoot(), subdirName); createFileInSubDir(subDirectory, "foo", "some content"); @@ -77,8 +76,6 @@ public class FileDirectoryTest { @Test public void requireThatExistingDirWithInvalidContentIsDeleted() throws IOException { - FileDirectory fileDirectory = new FileDirectory(temporaryFolder.getRoot()); - String subdirName = "subdir"; File subDirectory = new File(temporaryFolder.getRoot(), subdirName); createFileInSubDir(subDirectory, "foo", "some content"); @@ -108,8 +105,6 @@ public class FileDirectoryTest { @Test public void requireThatNothingIsDoneIfFileReferenceExists() throws IOException { - FileDirectory fileDirectory = new FileDirectory(temporaryFolder.getRoot()); - String subdirName = "subdir"; File subDirectory = new File(temporaryFolder.getRoot(), subdirName); createFileInSubDir(subDirectory, "foo", "some content"); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java index d4bc4016b29..cf86ff06333 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java @@ -41,7 +41,7 @@ public class FileServerTest { @Before public void setup() throws IOException { File rootDir = new File(temporaryFolder.newFolder("fileserver-root").getAbsolutePath()); - fileServer = new FileServer(new MockFileDownloader(rootDir), List.of(gzip, lz4), new FileDirectory(rootDir)); + fileServer = new FileServer(new MockFileDownloader(rootDir), List.of(gzip, lz4), new FileDirectory(rootDir, new InMemoryFlagSource())); } @Test @@ -86,7 +86,7 @@ public class FileServerTest { @Test public void requireThatWeCanReplayDirWithLz4() throws IOException, InterruptedException, ExecutionException { File rootDir = new File(temporaryFolder.newFolder("fileserver-root-3").getAbsolutePath()); - fileServer = new FileServer(new MockFileDownloader(rootDir), List.of(lz4, gzip), new FileDirectory(rootDir)); // prefer lz4 + fileServer = new FileServer(new MockFileDownloader(rootDir), List.of(lz4, gzip), new FileDirectory(rootDir, new InMemoryFlagSource())); // prefer lz4 File dir = getFileServerRootDir(); IOUtils.writeFile(dir + "/subdir/12z/f1", "dummy-data-2", true); CompletableFuture<byte []> content = new CompletableFuture<>(); @@ -140,7 +140,10 @@ public class FileServerTest { private FileServer createFileServer(ConfigserverConfig.Builder configBuilder) throws IOException { File fileReferencesDir = temporaryFolder.newFolder(); configBuilder.fileReferencesDir(fileReferencesDir.getAbsolutePath()); - return new FileServer(new ConfigserverConfig(configBuilder), new InMemoryFlagSource(), new FileDirectory(fileReferencesDir)); + InMemoryFlagSource flagSource = new InMemoryFlagSource(); + return new FileServer(new ConfigserverConfig(configBuilder), + flagSource, + new FileDirectory(fileReferencesDir, flagSource)); } private static class FileReceiver implements FileServer.Receiver { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileDistributionFactory.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileDistributionFactory.java index 994b59c417d..706a550a387 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileDistributionFactory.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileDistributionFactory.java @@ -17,7 +17,7 @@ public class MockFileDistributionFactory extends FileDistributionFactory { @Override public FileRegistry createFileRegistry(File applicationPackage) { - return new MockFileRegistry(applicationPackage, getFileReferencesDir()); + return new MockFileRegistry(applicationPackage, fileDirectory); } @Override diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileRegistry.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileRegistry.java index 5cda7521188..fc594d4f0cc 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileRegistry.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileRegistry.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.config.server.filedistribution; import com.yahoo.config.FileReference; import com.yahoo.config.application.api.FileRegistry; import com.yahoo.path.Path; - import java.io.File; import java.io.IOException; import java.nio.ByteBuffer; @@ -21,8 +20,7 @@ public class MockFileRegistry implements FileRegistry { private final List<Entry> entries = new ArrayList<>(); private final AddFileInterface addFileInterface; - public MockFileRegistry(File applicationDir, File rootPath) { - FileDirectory fileDirectory = new FileDirectory(rootPath); + public MockFileRegistry(File applicationDir, FileDirectory fileDirectory) { this.addFileInterface = new ApplicationFileManager(applicationDir, fileDirectory, false); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java index 1079b4b89cf..d543820fc68 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java @@ -43,6 +43,7 @@ import com.yahoo.vespa.config.server.session.PrepareParams; import com.yahoo.vespa.config.server.tenant.Tenant; import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.config.server.tenant.TestTenantRepository; +import com.yahoo.vespa.flags.InMemoryFlagSource; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -125,7 +126,7 @@ public class ApplicationHandlerTest { tenantRepository = new TestTenantRepository.Builder() .withClock(clock) .withConfigserverConfig(configserverConfig) - .withFileDistributionFactory(new MockFileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig))) + .withFileDistributionFactory(new MockFileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig, new InMemoryFlagSource()))) .withHostProvisionerProvider(HostProvisionerProvider.withProvisioner(provisioner, false)) .withModelFactoryRegistry(new ModelFactoryRegistry(modelFactories)) .build(); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpcServer.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpcServer.java index 046aa165943..8400f677124 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpcServer.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpcServer.java @@ -11,6 +11,7 @@ import com.yahoo.vespa.config.server.filedistribution.FileServer; import com.yahoo.vespa.config.server.host.HostRegistry; import com.yahoo.vespa.config.server.monitoring.Metrics; import com.yahoo.vespa.config.server.rpc.security.NoopRpcAuthorizer; +import com.yahoo.vespa.flags.InMemoryFlagSource; import java.io.File; import java.time.Duration; @@ -38,7 +39,7 @@ public class MockRpcServer extends RpcServer { null, Metrics.createTestMetrics(), new HostRegistry(), - new FileServer(new FileDirectory(tempDir)), + new FileServer(new FileDirectory(tempDir, new InMemoryFlagSource())), new NoopRpcAuthorizer(), new RpcRequestHandlerProvider()); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java index 582ff412c45..651a56dfb25 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java @@ -109,19 +109,20 @@ public class RpcTester implements AutoCloseable { } RpcServer createRpcServer(ConfigserverConfig config) throws IOException { + InMemoryFlagSource flagSource = new InMemoryFlagSource(); RpcServer rpcServer = new RpcServer(config, - new SuperModelRequestHandler(new TestConfigDefinitionRepo(), + new SuperModelRequestHandler(new TestConfigDefinitionRepo(), configserverConfig, new SuperModelManager( config, Zone.defaultZone(), new MemoryGenerationCounter(), - new InMemoryFlagSource())), - Metrics.createTestMetrics(), - hostRegistry, - new FileServer(new FileDirectory(temporaryFolder.newFolder())), - new NoopRpcAuthorizer(), - new RpcRequestHandlerProvider()); + flagSource)), + Metrics.createTestMetrics(), + hostRegistry, + new FileServer(new FileDirectory(temporaryFolder.newFolder(), flagSource)), + new NoopRpcAuthorizer(), + new RpcRequestHandlerProvider()); rpcServer.setUpGetConfigHandlers(); return rpcServer; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java index 8e209ef4cf3..eabcd5dce45 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java @@ -127,7 +127,7 @@ public class SessionPreparerTest { HostProvisionerProvider hostProvisionerProvider) { return new SessionPreparer( modelFactoryRegistry, - new MockFileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig)), + new MockFileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig, flagSource)), new InThreadExecutorService(), hostProvisionerProvider, new PermanentApplicationPackage(configserverConfig), diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java index 885f5117242..af7754fa995 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java @@ -98,7 +98,8 @@ public class SessionRepositoryTest { .withConfigserverConfig(configserverConfig) .withCurator(curator) .withFlagSource(flagSource) - .withFileDistributionFactory(new MockFileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig))) + .withFileDistributionFactory( + new MockFileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig, flagSource))) .withModelFactoryRegistry(modelFactoryRegistry) .build(); tenantRepository.addTenant(SessionRepositoryTest.tenantName); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java index a751c517f1e..89279ceea1e 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java @@ -30,6 +30,7 @@ import com.yahoo.vespa.config.server.monitoring.Metrics; import com.yahoo.vespa.config.server.provision.HostProvisionerProvider; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.VespaModelFactory; @@ -40,7 +41,6 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import org.junit.rules.TemporaryFolder; import org.xml.sax.SAXException; - import java.io.IOException; import java.time.Clock; import java.util.Arrays; @@ -207,14 +207,16 @@ public class TenantRepositoryTest { private static class FailingDuringBootstrapTenantRepository extends TenantRepository { + private static final FlagSource flagSource = new InMemoryFlagSource(); + public FailingDuringBootstrapTenantRepository(ConfigserverConfig configserverConfig) { super(new HostRegistry(), new MockCurator(), Metrics.createTestMetrics(), new StripedExecutor<>(new InThreadExecutorService()), new StripedExecutor<>(new InThreadExecutorService()), - new FileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig)), - new InMemoryFlagSource(), + new FileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig, flagSource)), + flagSource, new InThreadExecutorService(), new MockSecretStore(), HostProvisionerProvider.withProvisioner(new MockProvisioner(), false), diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TestTenantRepository.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TestTenantRepository.java index dd982ccbd72..f6935c25903 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TestTenantRepository.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TestTenantRepository.java @@ -144,7 +144,8 @@ public class TestTenantRepository extends TenantRepository { public TenantRepository build() { if (fileDistributionFactory == null) - fileDistributionFactory = new FileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig)); + fileDistributionFactory = new FileDistributionFactory(configserverConfig, + new FileDirectory(configserverConfig, flagSource)); return new TestTenantRepository(hostRegistry, curator, metrics, |