From c92a1c7aece77cd47283b2711fce2025f06641b0 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Mon, 26 Nov 2018 16:38:17 -0700 Subject: [PATCH 01/20] Work RepositoriesService into CCR --- .../java/org/elasticsearch/node/Node.java | 7 +- .../plugins/RepositoryPlugin.java | 5 + .../repositories/RepositoriesModule.java | 8 + .../java/org/elasticsearch/xpack/ccr/Ccr.java | 137 +++++++++++++++- .../xpack/ccr/repository/CcrRepository.java | 148 ++++++++++++++++++ .../core/LocalStateCompositeXPackPlugin.java | 9 ++ 6 files changed, 312 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index f3433dfa1ba1a..c2691abcbfd60 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -132,6 +132,7 @@ import org.elasticsearch.plugins.ScriptPlugin; import org.elasticsearch.plugins.SearchPlugin; import org.elasticsearch.repositories.RepositoriesModule; +import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.rest.RestController; import org.elasticsearch.script.ScriptModule; import org.elasticsearch.script.ScriptService; @@ -396,7 +397,9 @@ protected Node( .flatMap(p -> p.getNamedXContent().stream()), ClusterModule.getNamedXWriteables().stream()) .flatMap(Function.identity()).collect(toList())); - modules.add(new RepositoriesModule(this.environment, pluginsService.filterPlugins(RepositoryPlugin.class), xContentRegistry)); + RepositoriesModule repositoriesModule = new RepositoriesModule(this.environment, pluginsService.filterPlugins(RepositoryPlugin.class), + xContentRegistry); + modules.add(repositoriesModule); final MetaStateService metaStateService = new MetaStateService(nodeEnvironment, xContentRegistry); // collect engine factory providers from server and from plugins @@ -570,6 +573,8 @@ protected Node( actionModule.initRestHandlers(() -> clusterService.state().nodes()); logger.info("initialized"); + repositoriesModule.initRepositoryPlugins(injector.getInstance(RepositoriesService.class)); + success = true; } catch (IOException ex) { throw new ElasticsearchException("failed to bind service", ex); diff --git a/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java b/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java index a3af52a9a4aca..3d1032206edda 100644 --- a/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java @@ -24,6 +24,7 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.env.Environment; +import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.repositories.Repository; /** @@ -42,4 +43,8 @@ public interface RepositoryPlugin { default Map getRepositories(Environment env, NamedXContentRegistry namedXContentRegistry) { return Collections.emptyMap(); } + + default void supplyRepositoriesService(RepositoriesService repositoriesService) { + + } } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java index 3073e1f0b0ef6..7fefde85bf944 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java @@ -41,8 +41,10 @@ public class RepositoriesModule extends AbstractModule { private final Map repositoryTypes; + private final List repoPlugins; public RepositoriesModule(Environment env, List repoPlugins, NamedXContentRegistry namedXContentRegistry) { + this.repoPlugins = repoPlugins; Map factories = new HashMap<>(); factories.put(FsRepository.TYPE, (metadata) -> new FsRepository(metadata, env, namedXContentRegistry)); @@ -67,4 +69,10 @@ protected void configure() { MapBinder typesBinder = MapBinder.newMapBinder(binder(), String.class, Repository.Factory.class); repositoryTypes.forEach((k, v) -> typesBinder.addBinding(k).toInstance(v)); } + + public void initRepositoryPlugins(RepositoriesService repositoriesService) { + for (RepositoryPlugin plugin : repoPlugins) { + plugin.supplyRepositoriesService(repositoriesService); + } + } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java index ec196d637e1a9..e1d49754997c8 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java @@ -6,10 +6,20 @@ package org.elasticsearch.xpack.ccr; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.apache.lucene.util.SetOnce; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateUpdateTask; +import org.elasticsearch.cluster.LocalNodeMasterListener; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.metadata.RepositoriesMetaData; +import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.ParseField; @@ -20,6 +30,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsFilter; import org.elasticsearch.common.settings.SettingsModule; +import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.env.Environment; import org.elasticsearch.env.NodeEnvironment; @@ -32,6 +43,8 @@ import org.elasticsearch.plugins.EnginePlugin; import org.elasticsearch.plugins.PersistentTaskPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.RepositoryPlugin; +import org.elasticsearch.repositories.Repository; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.script.ScriptService; @@ -39,10 +52,12 @@ import org.elasticsearch.threadpool.ExecutorBuilder; import org.elasticsearch.threadpool.FixedExecutorBuilder; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.RemoteClusterAware; import org.elasticsearch.watcher.ResourceWatcherService; import org.elasticsearch.xpack.ccr.action.AutoFollowCoordinator; import org.elasticsearch.xpack.ccr.action.TransportGetAutoFollowPatternAction; import org.elasticsearch.xpack.ccr.action.TransportUnfollowAction; +import org.elasticsearch.xpack.ccr.repository.CcrRepository; import org.elasticsearch.xpack.ccr.rest.RestGetAutoFollowPatternAction; import org.elasticsearch.xpack.ccr.action.TransportCcrStatsAction; import org.elasticsearch.xpack.ccr.rest.RestCcrStatsAction; @@ -77,12 +92,16 @@ import org.elasticsearch.xpack.core.ccr.action.PauseFollowAction; import org.elasticsearch.xpack.core.ccr.action.UnfollowAction; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.function.Supplier; import static java.util.Collections.emptyList; @@ -92,7 +111,7 @@ /** * Container class for CCR functionality. */ -public class Ccr extends Plugin implements ActionPlugin, PersistentTaskPlugin, EnginePlugin { +public class Ccr extends Plugin implements ActionPlugin, PersistentTaskPlugin, EnginePlugin, RepositoryPlugin { public static final String CCR_THREAD_POOL_NAME = "ccr"; public static final String CCR_CUSTOM_METADATA_KEY = "ccr"; @@ -104,6 +123,7 @@ public class Ccr extends Plugin implements ActionPlugin, PersistentTaskPlugin, E private final boolean enabled; private final Settings settings; private final CcrLicenseChecker ccrLicenseChecker; + private SetOnce repositoryManager = new SetOnce<>(); /** * Construct an instance of the CCR container with the specified settings. @@ -142,6 +162,8 @@ public Collection createComponents( return emptyList(); } + repositoryManager.set(new CCRRepositoryManager(settings, clusterService)); + return Arrays.asList( ccrLicenseChecker, new AutoFollowCoordinator(settings, client, threadPool, clusterService, ccrLicenseChecker) @@ -259,6 +281,119 @@ public List> getExecutorBuilders(Settings settings) { return Collections.singletonList(new FixedExecutorBuilder(settings, CCR_THREAD_POOL_NAME, 32, 100, "xpack.ccr.ccr_thread_pool")); } + @Override + public Map getRepositories(Environment env, NamedXContentRegistry namedXContentRegistry) { + Repository.Factory repositoryFactory = (metadata) -> new CcrRepository(metadata, settings); + return Collections.singletonMap(CcrRepository.TYPE, repositoryFactory); + } + protected XPackLicenseState getLicenseState() { return XPackPlugin.getSharedLicenseState(); } + private static class CCRRepositoryManager extends RemoteClusterAware implements LocalNodeMasterListener { + + private static final Logger LOGGER = LogManager.getLogger(CCRRepositoryManager.class); + private static final String SOURCE = "refreshing " + CcrRepository.TYPE + " repositories"; + + private final ClusterService clusterService; + private final Set clusters = ConcurrentCollections.newConcurrentSet(); + private volatile boolean isMasterNode = false; + + private CCRRepositoryManager(Settings settings, ClusterService clusterService) { + super(settings); + this.clusterService = clusterService; + clusters.addAll(buildRemoteClustersDynamicConfig(settings).keySet()); + clusterService.addLocalNodeMasterListener(this); + listenForUpdates(clusterService.getClusterSettings()); + } + + @Override + protected Set getRemoteClusterNames() { + return clusters; + } + + @Override + protected void updateRemoteCluster(String clusterAlias, List addresses, String proxyAddress) { + if (addresses.isEmpty()) { + if (clusters.remove(clusterAlias) && isMasterNode) { + refreshCCRRepositories(); + } + } else { + if (clusters.add(clusterAlias) && isMasterNode) { + refreshCCRRepositories(); + } + } + } + + @Override + public void onMaster() { + this.isMasterNode = true; + refreshCCRRepositories(); + } + + @Override + public void offMaster() { + this.isMasterNode = false; + } + + @Override + public String executorName() { + return ThreadPool.Names.SAME; + } + + private void refreshCCRRepositories() { + clusterService.submitStateUpdateTask(SOURCE, new ClusterStateUpdateTask() { + + @Override + public ClusterState execute(ClusterState currentState) throws Exception { + MetaData metaData = currentState.metaData(); + RepositoriesMetaData repositories = metaData.custom(RepositoriesMetaData.TYPE); + + if (repositories == null) { + List repositoriesMetaData = new ArrayList<>(clusters.size()); + for (String cluster : clusters) { + LOGGER.info("put [{}] repository [{}]", CcrRepository.TYPE, cluster); + repositoriesMetaData.add(new RepositoryMetaData(cluster, CcrRepository.TYPE, Settings.EMPTY)); + } + repositories = new RepositoriesMetaData(repositoriesMetaData); + } else { + List repositoriesMetaData = new ArrayList<>(repositories.repositories().size()); + + Set needToAdd = new HashSet<>(clusters); + + for (RepositoryMetaData repositoryMetaData : repositories.repositories()) { + String name = repositoryMetaData.name(); + if (CcrRepository.TYPE.equals(repositoryMetaData.type())) { + if (needToAdd.remove(name)) { + repositoriesMetaData.add(new RepositoryMetaData(name, CcrRepository.TYPE, Settings.EMPTY)); + } else { + LOGGER.info("delete [{}] repository [{}]", CcrRepository.TYPE, name); + } + } else { + if (needToAdd.remove(name)) { + throw new IllegalStateException("Repository name conflict. Cannot put [" + + CcrRepository.TYPE + "] repository [" + name + "]. A [" + + repositoryMetaData.type() + "] repository with the same name is already registered."); + } + repositoriesMetaData.add(repositoryMetaData); + } + } + for (String cluster : needToAdd) { + LOGGER.info("put [{}] repository [{}]", CcrRepository.TYPE, cluster); + repositoriesMetaData.add(new RepositoryMetaData(cluster, CcrRepository.TYPE, Settings.EMPTY)); + } + repositories = new RepositoriesMetaData(repositoriesMetaData); + } + + MetaData.Builder mdBuilder = MetaData.builder(currentState.metaData()); + mdBuilder.putCustom(RepositoriesMetaData.TYPE, repositories); + return ClusterState.builder(currentState).metaData(mdBuilder).build(); + } + + @Override + public void onFailure(String source, Exception e) { + LOGGER.warn(new ParameterizedMessage("failed to refresh [{}] repositories", CcrRepository.TYPE), e); + } + }); + } + } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java new file mode 100644 index 0000000000000..00646a8d06e1b --- /dev/null +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java @@ -0,0 +1,148 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.ccr.repository; + +import org.apache.lucene.index.IndexCommit; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.metadata.RepositoryMetaData; +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.common.component.AbstractLifecycleComponent; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.shard.IndexShard; +import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.index.snapshots.IndexShardSnapshotStatus; +import org.elasticsearch.index.store.Store; +import org.elasticsearch.indices.recovery.RecoveryState; +import org.elasticsearch.repositories.IndexId; +import org.elasticsearch.repositories.Repository; +import org.elasticsearch.repositories.RepositoryData; +import org.elasticsearch.snapshots.SnapshotId; +import org.elasticsearch.snapshots.SnapshotInfo; +import org.elasticsearch.snapshots.SnapshotShardFailure; + +import java.io.IOException; +import java.util.List; + +/** + * This repository relies on a remote cluster for Ccr restores. It is read-only so it can only be used to + * restore shards/indexes that exist on the remote cluster. + */ +public class CcrRepository extends AbstractLifecycleComponent implements Repository { + + public static final String TYPE = "_ccr_"; + + private final RepositoryMetaData metadata; + + public CcrRepository(RepositoryMetaData metadata, Settings settings) { + super(settings); + this.metadata = metadata; + } + + @Override + protected void doStart() { + + } + + @Override + protected void doStop() { + + } + + @Override + protected void doClose() throws IOException { + + } + + @Override + public RepositoryMetaData getMetadata() { + return metadata; + } + + @Override + public SnapshotInfo getSnapshotInfo(SnapshotId snapshotId) { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } + + @Override + public MetaData getSnapshotGlobalMetaData(SnapshotId snapshotId) { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } + + @Override + public IndexMetaData getSnapshotIndexMetaData(SnapshotId snapshotId, IndexId index) throws IOException { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } + + @Override + public RepositoryData getRepositoryData() { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } + + @Override + public void initializeSnapshot(SnapshotId snapshotId, List indices, MetaData metaData) { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } + + @Override + public SnapshotInfo finalizeSnapshot(SnapshotId snapshotId, List indices, long startTime, String failure, int totalShards, + List shardFailures, long repositoryStateId, boolean includeGlobalState) { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } + + @Override + public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId) { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } + + @Override + public long getSnapshotThrottleTimeInNanos() { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } + + @Override + public long getRestoreThrottleTimeInNanos() { + return 0; + } + + @Override + public String startVerification() { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } + + @Override + public void endVerification(String verificationToken) { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } + + @Override + public void verify(String verificationToken, DiscoveryNode localNode) { + } + + @Override + public boolean isReadOnly() { + return true; + } + + @Override + public void snapshotShard(IndexShard shard, Store store, SnapshotId snapshotId, IndexId indexId, IndexCommit snapshotIndexCommit, + IndexShardSnapshotStatus snapshotStatus) { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } + + @Override + public void restoreShard(IndexShard shard, SnapshotId snapshotId, Version version, IndexId indexId, ShardId snapshotShardId, + RecoveryState recoveryState) { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } + + @Override + public IndexShardSnapshotStatus getShardSnapshotStatus(SnapshotId snapshotId, Version version, IndexId indexId, ShardId shardId) { + throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE); + } +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java index 92427fb92c439..369ee5e2be016 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java @@ -53,7 +53,9 @@ import org.elasticsearch.plugins.NetworkPlugin; import org.elasticsearch.plugins.PersistentTaskPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.RepositoryPlugin; import org.elasticsearch.plugins.ScriptPlugin; +import org.elasticsearch.repositories.Repository; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.script.ScriptContext; @@ -393,6 +395,13 @@ public List> getPersistentTasksExecutor(ClusterServic .collect(toList()); } + @Override + public Map getRepositories(Environment env, NamedXContentRegistry namedXContentRegistry) { + HashMap repositories = new HashMap<>(super.getRepositories(env, namedXContentRegistry)); + filterPlugins(RepositoryPlugin.class).forEach(r -> repositories.putAll(r.getRepositories(env, namedXContentRegistry))); + return repositories; + } + @Override public void close() throws IOException { IOUtils.close(plugins); From a2c7576b4038e2f98573d4287c797f9b7268c44c Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 27 Nov 2018 17:57:18 -0700 Subject: [PATCH 02/20] WIP --- .../plugins/RepositoryPlugin.java | 5 + .../repositories/RepositoriesService.java | 35 +++- .../java/org/elasticsearch/xpack/ccr/Ccr.java | 170 +++--------------- .../xpack/ccr/CcrRepositoryManager.java | 50 ++++++ .../xpack/ccr/CcrRepositoryManagerTests.java | 67 +++++++ 5 files changed, 185 insertions(+), 142 deletions(-) create mode 100644 x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java create mode 100644 x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerTests.java diff --git a/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java b/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java index 3d1032206edda..a9de9eb4cce69 100644 --- a/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java @@ -44,6 +44,11 @@ default Map getRepositories(Environment env, NamedXC return Collections.emptyMap(); } + /** + * Passes the {@link RepositoriesService} to the plugin. + * + * @param repositoriesService instance for the plugin to optionally use + */ default void supplyRepositoriesService(RepositoriesService repositoriesService) { } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index 295b64c554f6e..5583d60723e3e 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -37,6 +37,7 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.snapshots.RestoreService; import org.elasticsearch.snapshots.SnapshotsService; import org.elasticsearch.threadpool.ThreadPool; @@ -67,6 +68,7 @@ public class RepositoriesService implements ClusterStateApplier { private final VerifyNodeRepositoryAction verifyAction; private volatile Map repositories = Collections.emptyMap(); + private volatile Map internalRepositories = ConcurrentCollections.newConcurrentMap(); @Inject public RepositoriesService(Settings settings, ClusterService clusterService, TransportService transportService, @@ -351,9 +353,40 @@ public Repository repository(String repositoryName) { if (repository != null) { return repository; } + repository = internalRepositories.get(repositoryName); + if (repository != null) { + return repository; + } throw new RepositoryMissingException(repositoryName); } + public void registerInternalRepository(String name, Repository repository) { + RepositoryMetaData metadata = repository.getMetadata(); + logger.debug(() -> new ParameterizedMessage("Registering internal [type={}] repository [name={}].", metadata.type(), name)); + Repository existingRepository = internalRepositories.putIfAbsent(name, repository); + if (existingRepository != null) { + logger.error(new ParameterizedMessage("Error registering internal [type={}] repository [name={}]. " + + "Internal repository with that name already registered.", metadata.type(), name)); + } + if (repositories.containsKey(name)) { + logger.warn(new ParameterizedMessage("Non-internal repository [name={}] already registered. This repository will block the " + + "usage of internal [type={}] repository [name={}].", name, metadata.type(), name)); + } + } + + public void unregisterInternalRepository(String name) { + Repository repository = internalRepositories.remove(name); + RepositoryMetaData metadata = repository.getMetadata(); + if (repository != null) { + logger.debug(() -> new ParameterizedMessage("Unregistering internal [type={}] repository [name={}].", metadata.type(), name)); + closeRepository(repository); + repository.close(); + } else { + logger.warn(() -> new ParameterizedMessage("Attempted to unregistering internal [type={}] repository [name={}]. " + + "Repository could not found.", metadata.type(), name)); + } + } + /** * Creates a new repository and adds it to the list of registered repositories. *

@@ -384,7 +417,7 @@ private boolean registerRepository(RepositoryMetaData repositoryMetaData) throws } /** Closes the given repository. */ - private void closeRepository(Repository repository) throws IOException { + private void closeRepository(Repository repository) { logger.debug("closing repository [{}][{}]", repository.getMetadata().type(), repository.getMetadata().name()); repository.close(); } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java index e1d49754997c8..50903349f51a8 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java @@ -6,20 +6,11 @@ package org.elasticsearch.xpack.ccr; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.lucene.util.SetOnce; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.client.Client; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.ClusterStateUpdateTask; -import org.elasticsearch.cluster.LocalNodeMasterListener; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; -import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.cluster.metadata.RepositoriesMetaData; -import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.ParseField; @@ -30,7 +21,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsFilter; import org.elasticsearch.common.settings.SettingsModule; -import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.env.Environment; import org.elasticsearch.env.NodeEnvironment; @@ -44,6 +34,7 @@ import org.elasticsearch.plugins.PersistentTaskPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.RepositoryPlugin; +import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.repositories.Repository; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; @@ -52,56 +43,52 @@ import org.elasticsearch.threadpool.ExecutorBuilder; import org.elasticsearch.threadpool.FixedExecutorBuilder; import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.transport.RemoteClusterAware; import org.elasticsearch.watcher.ResourceWatcherService; import org.elasticsearch.xpack.ccr.action.AutoFollowCoordinator; -import org.elasticsearch.xpack.ccr.action.TransportGetAutoFollowPatternAction; -import org.elasticsearch.xpack.ccr.action.TransportUnfollowAction; -import org.elasticsearch.xpack.ccr.repository.CcrRepository; -import org.elasticsearch.xpack.ccr.rest.RestGetAutoFollowPatternAction; -import org.elasticsearch.xpack.ccr.action.TransportCcrStatsAction; -import org.elasticsearch.xpack.ccr.rest.RestCcrStatsAction; -import org.elasticsearch.xpack.ccr.rest.RestUnfollowAction; -import org.elasticsearch.xpack.core.ccr.action.CcrStatsAction; -import org.elasticsearch.xpack.core.ccr.action.DeleteAutoFollowPatternAction; -import org.elasticsearch.xpack.core.ccr.action.GetAutoFollowPatternAction; -import org.elasticsearch.xpack.core.ccr.action.PutAutoFollowPatternAction; import org.elasticsearch.xpack.ccr.action.ShardChangesAction; import org.elasticsearch.xpack.ccr.action.ShardFollowTask; import org.elasticsearch.xpack.ccr.action.ShardFollowTasksExecutor; +import org.elasticsearch.xpack.ccr.action.TransportCcrStatsAction; +import org.elasticsearch.xpack.ccr.action.TransportDeleteAutoFollowPatternAction; import org.elasticsearch.xpack.ccr.action.TransportFollowStatsAction; +import org.elasticsearch.xpack.ccr.action.TransportGetAutoFollowPatternAction; +import org.elasticsearch.xpack.ccr.action.TransportPauseFollowAction; +import org.elasticsearch.xpack.ccr.action.TransportPutAutoFollowPatternAction; import org.elasticsearch.xpack.ccr.action.TransportPutFollowAction; -import org.elasticsearch.xpack.ccr.action.TransportDeleteAutoFollowPatternAction; import org.elasticsearch.xpack.ccr.action.TransportResumeFollowAction; -import org.elasticsearch.xpack.ccr.action.TransportPutAutoFollowPatternAction; -import org.elasticsearch.xpack.ccr.action.TransportPauseFollowAction; +import org.elasticsearch.xpack.ccr.action.TransportUnfollowAction; import org.elasticsearch.xpack.ccr.action.bulk.BulkShardOperationsAction; import org.elasticsearch.xpack.ccr.action.bulk.TransportBulkShardOperationsAction; import org.elasticsearch.xpack.ccr.index.engine.FollowingEngineFactory; +import org.elasticsearch.xpack.ccr.repository.CcrRepository; +import org.elasticsearch.xpack.ccr.rest.RestCcrStatsAction; +import org.elasticsearch.xpack.ccr.rest.RestDeleteAutoFollowPatternAction; import org.elasticsearch.xpack.ccr.rest.RestFollowStatsAction; +import org.elasticsearch.xpack.ccr.rest.RestGetAutoFollowPatternAction; +import org.elasticsearch.xpack.ccr.rest.RestPauseFollowAction; +import org.elasticsearch.xpack.ccr.rest.RestPutAutoFollowPatternAction; import org.elasticsearch.xpack.ccr.rest.RestPutFollowAction; -import org.elasticsearch.xpack.ccr.rest.RestDeleteAutoFollowPatternAction; import org.elasticsearch.xpack.ccr.rest.RestResumeFollowAction; -import org.elasticsearch.xpack.ccr.rest.RestPutAutoFollowPatternAction; -import org.elasticsearch.xpack.ccr.rest.RestPauseFollowAction; +import org.elasticsearch.xpack.ccr.rest.RestUnfollowAction; import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.ccr.ShardFollowNodeTaskStatus; +import org.elasticsearch.xpack.core.ccr.action.CcrStatsAction; +import org.elasticsearch.xpack.core.ccr.action.DeleteAutoFollowPatternAction; import org.elasticsearch.xpack.core.ccr.action.FollowStatsAction; +import org.elasticsearch.xpack.core.ccr.action.GetAutoFollowPatternAction; +import org.elasticsearch.xpack.core.ccr.action.PauseFollowAction; +import org.elasticsearch.xpack.core.ccr.action.PutAutoFollowPatternAction; import org.elasticsearch.xpack.core.ccr.action.PutFollowAction; import org.elasticsearch.xpack.core.ccr.action.ResumeFollowAction; -import org.elasticsearch.xpack.core.ccr.action.PauseFollowAction; import org.elasticsearch.xpack.core.ccr.action.UnfollowAction; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.function.Supplier; import static java.util.Collections.emptyList; @@ -123,7 +110,8 @@ public class Ccr extends Plugin implements ActionPlugin, PersistentTaskPlugin, E private final boolean enabled; private final Settings settings; private final CcrLicenseChecker ccrLicenseChecker; - private SetOnce repositoryManager = new SetOnce<>(); + private SetOnce clusterService = new SetOnce<>(); + private SetOnce repositoryManager = new SetOnce<>(); /** * Construct an instance of the CCR container with the specified settings. @@ -162,7 +150,7 @@ public Collection createComponents( return emptyList(); } - repositoryManager.set(new CCRRepositoryManager(settings, clusterService)); + this.clusterService.set(clusterService); return Arrays.asList( ccrLicenseChecker, @@ -287,113 +275,13 @@ public Map getRepositories(Environment env, NamedXCo return Collections.singletonMap(CcrRepository.TYPE, repositoryFactory); } - protected XPackLicenseState getLicenseState() { return XPackPlugin.getSharedLicenseState(); } - - private static class CCRRepositoryManager extends RemoteClusterAware implements LocalNodeMasterListener { - - private static final Logger LOGGER = LogManager.getLogger(CCRRepositoryManager.class); - private static final String SOURCE = "refreshing " + CcrRepository.TYPE + " repositories"; - - private final ClusterService clusterService; - private final Set clusters = ConcurrentCollections.newConcurrentSet(); - private volatile boolean isMasterNode = false; - - private CCRRepositoryManager(Settings settings, ClusterService clusterService) { - super(settings); - this.clusterService = clusterService; - clusters.addAll(buildRemoteClustersDynamicConfig(settings).keySet()); - clusterService.addLocalNodeMasterListener(this); - listenForUpdates(clusterService.getClusterSettings()); - } - - @Override - protected Set getRemoteClusterNames() { - return clusters; - } - - @Override - protected void updateRemoteCluster(String clusterAlias, List addresses, String proxyAddress) { - if (addresses.isEmpty()) { - if (clusters.remove(clusterAlias) && isMasterNode) { - refreshCCRRepositories(); - } - } else { - if (clusters.add(clusterAlias) && isMasterNode) { - refreshCCRRepositories(); - } - } - } - - @Override - public void onMaster() { - this.isMasterNode = true; - refreshCCRRepositories(); - } - - @Override - public void offMaster() { - this.isMasterNode = false; - } - - @Override - public String executorName() { - return ThreadPool.Names.SAME; - } - - private void refreshCCRRepositories() { - clusterService.submitStateUpdateTask(SOURCE, new ClusterStateUpdateTask() { - - @Override - public ClusterState execute(ClusterState currentState) throws Exception { - MetaData metaData = currentState.metaData(); - RepositoriesMetaData repositories = metaData.custom(RepositoriesMetaData.TYPE); - - if (repositories == null) { - List repositoriesMetaData = new ArrayList<>(clusters.size()); - for (String cluster : clusters) { - LOGGER.info("put [{}] repository [{}]", CcrRepository.TYPE, cluster); - repositoriesMetaData.add(new RepositoryMetaData(cluster, CcrRepository.TYPE, Settings.EMPTY)); - } - repositories = new RepositoriesMetaData(repositoriesMetaData); - } else { - List repositoriesMetaData = new ArrayList<>(repositories.repositories().size()); - - Set needToAdd = new HashSet<>(clusters); - - for (RepositoryMetaData repositoryMetaData : repositories.repositories()) { - String name = repositoryMetaData.name(); - if (CcrRepository.TYPE.equals(repositoryMetaData.type())) { - if (needToAdd.remove(name)) { - repositoriesMetaData.add(new RepositoryMetaData(name, CcrRepository.TYPE, Settings.EMPTY)); - } else { - LOGGER.info("delete [{}] repository [{}]", CcrRepository.TYPE, name); - } - } else { - if (needToAdd.remove(name)) { - throw new IllegalStateException("Repository name conflict. Cannot put [" + - CcrRepository.TYPE + "] repository [" + name + "]. A [" + - repositoryMetaData.type() + "] repository with the same name is already registered."); - } - repositoriesMetaData.add(repositoryMetaData); - } - } - for (String cluster : needToAdd) { - LOGGER.info("put [{}] repository [{}]", CcrRepository.TYPE, cluster); - repositoriesMetaData.add(new RepositoryMetaData(cluster, CcrRepository.TYPE, Settings.EMPTY)); - } - repositories = new RepositoriesMetaData(repositoriesMetaData); - } + @Override + public void supplyRepositoriesService(RepositoriesService repositoriesService) { + ClusterService clusterService = this.clusterService.get(); + assert clusterService != null : "ClusterService not set."; + repositoryManager.set(new CcrRepositoryManager(settings, clusterService, repositoriesService)); + } - MetaData.Builder mdBuilder = MetaData.builder(currentState.metaData()); - mdBuilder.putCustom(RepositoriesMetaData.TYPE, repositories); - return ClusterState.builder(currentState).metaData(mdBuilder).build(); - } + protected XPackLicenseState getLicenseState() { return XPackPlugin.getSharedLicenseState(); } - @Override - public void onFailure(String source, Exception e) { - LOGGER.warn(new ParameterizedMessage("failed to refresh [{}] repositories", CcrRepository.TYPE), e); - } - }); - } - } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java new file mode 100644 index 0000000000000..f3332bb3951f2 --- /dev/null +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java @@ -0,0 +1,50 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.ccr; + +import org.elasticsearch.cluster.metadata.RepositoryMetaData; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ConcurrentCollections; +import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.transport.RemoteClusterAware; +import org.elasticsearch.xpack.ccr.repository.CcrRepository; + +import java.util.List; +import java.util.Set; + +class CcrRepositoryManager extends RemoteClusterAware { + + private final RepositoriesService repositoriesService; + private final Set clusters = ConcurrentCollections.newConcurrentSet(); + + CcrRepositoryManager(Settings settings, ClusterService clusterService, RepositoriesService repositoriesService) { + super(settings); + this.repositoriesService = repositoriesService; + clusters.addAll(buildRemoteClustersDynamicConfig(settings).keySet()); + listenForUpdates(clusterService.getClusterSettings()); + } + + @Override + protected Set getRemoteClusterNames() { + return clusters; + } + + @Override + protected void updateRemoteCluster(String clusterAlias, List addresses, String proxyAddress) { + if (addresses.isEmpty()) { + if (clusters.remove(clusterAlias)) { + repositoriesService.unregisterInternalRepository(clusterAlias); + } + } else { + if (clusters.add(clusterAlias)) { + RepositoryMetaData metaData = new RepositoryMetaData(clusterAlias, CcrRepository.TYPE, settings); + repositoriesService.registerInternalRepository(clusterAlias, new CcrRepository(metaData, settings)); + } + } + } +} diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerTests.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerTests.java new file mode 100644 index 0000000000000..4372fa894e142 --- /dev/null +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerTests.java @@ -0,0 +1,67 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.ccr; + +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.UUIDs; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.repositories.RepositoryMissingException; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; +import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.ccr.repository.CcrRepository; + +import java.util.Collections; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class CcrRepositoryManagerTests extends ESTestCase { + + private RepositoriesService repositoriesService; + private ClusterSettings clusterSettings; + + @Override + public void setUp() throws Exception { + super.setUp(); + + Settings settings = Settings.EMPTY; + ThreadPool threadPool = mock(ThreadPool.class); + TransportService transportService = new TransportService(settings, mock(Transport.class), threadPool, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, + boundAddress -> DiscoveryNode.createLocal(settings, boundAddress.publishAddress(), UUIDs.randomBase64UUID()), null, + Collections.emptySet()); + ClusterService clusterService = mock(ClusterService.class); + clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + when(clusterService.getClusterSettings()).thenReturn(clusterSettings); + repositoriesService = new RepositoriesService(settings, clusterService, + transportService, null, threadPool); + CcrRepositoryManager repositoryManager = new CcrRepositoryManager(settings, clusterService, repositoriesService); + } + + public void testRepositoryIsRegistered() { + String clusterName = "cluster_1"; + clusterSettings.applySettings(Settings.builder().put("cluster.remote." + clusterName + ".seeds", "192.168.0.1:8080").build()); + CcrRepository repository = (CcrRepository) repositoriesService.repository(clusterName); + assertEquals(CcrRepository.TYPE, repository.getMetadata().type()); + assertEquals(clusterName, repository.getMetadata().name()); + } + + public void testRepositoryIsUnregistered() { + String clusterName = "cluster_1"; + clusterSettings.applySettings(Settings.builder().put("cluster.remote." + clusterName + ".seeds", "192.168.0.1:8080").build()); + // Will throw is it does not exist + repositoriesService.repository(clusterName); + + clusterSettings.applySettings(Settings.builder().put("cluster.remote.cluster_1.seeds", "").build()); + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository("cluster_1")); + } +} From e1b260e477ab7c9dc2639ce80715378be2e1408d Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 27 Nov 2018 17:58:46 -0700 Subject: [PATCH 03/20] WIP --- .../elasticsearch/repositories/RepositoriesService.java | 2 ++ .../src/main/java/org/elasticsearch/xpack/ccr/Ccr.java | 9 --------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index 5583d60723e3e..863c0779b5de8 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -375,6 +375,8 @@ public void registerInternalRepository(String name, Repository repository) { } public void unregisterInternalRepository(String name) { + // TODO: We normally have validation to prevent removing a repository that is in use. + // How do we want to handle that for internal repositories? Repository repository = internalRepositories.remove(name); RepositoryMetaData metadata = repository.getMetadata(); if (repository != null) { diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java index 50903349f51a8..af4b9727301cb 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java @@ -35,7 +35,6 @@ import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.RepositoryPlugin; import org.elasticsearch.repositories.RepositoriesService; -import org.elasticsearch.repositories.Repository; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.script.ScriptService; @@ -60,7 +59,6 @@ import org.elasticsearch.xpack.ccr.action.bulk.BulkShardOperationsAction; import org.elasticsearch.xpack.ccr.action.bulk.TransportBulkShardOperationsAction; import org.elasticsearch.xpack.ccr.index.engine.FollowingEngineFactory; -import org.elasticsearch.xpack.ccr.repository.CcrRepository; import org.elasticsearch.xpack.ccr.rest.RestCcrStatsAction; import org.elasticsearch.xpack.ccr.rest.RestDeleteAutoFollowPatternAction; import org.elasticsearch.xpack.ccr.rest.RestFollowStatsAction; @@ -86,7 +84,6 @@ import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.function.Supplier; @@ -269,12 +266,6 @@ public List> getExecutorBuilders(Settings settings) { return Collections.singletonList(new FixedExecutorBuilder(settings, CCR_THREAD_POOL_NAME, 32, 100, "xpack.ccr.ccr_thread_pool")); } - @Override - public Map getRepositories(Environment env, NamedXContentRegistry namedXContentRegistry) { - Repository.Factory repositoryFactory = (metadata) -> new CcrRepository(metadata, settings); - return Collections.singletonMap(CcrRepository.TYPE, repositoryFactory); - } - @Override public void supplyRepositoriesService(RepositoriesService repositoriesService) { ClusterService clusterService = this.clusterService.get(); From e45dfe7e557060c2e5402065ffc0488ef2ffb1a9 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 28 Nov 2018 15:26:46 -0700 Subject: [PATCH 04/20] Work on creating actions --- .../DeleteInternalRepositoryAction.java | 130 +++++++++++++++++ .../put/PutInternalRepositoryAction.java | 136 ++++++++++++++++++ .../repositories/RepositoriesService.java | 35 +++++ 3 files changed, 301 insertions(+) create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java new file mode 100644 index 0000000000000..d7f4d11f1a295 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java @@ -0,0 +1,130 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action.admin.cluster.repositories.delete; + +import org.elasticsearch.action.Action; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.TransportAction; +import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.transport.TransportService; + +import java.io.IOException; +import java.util.Objects; + +import static org.elasticsearch.action.ValidateActions.addValidationError; + +public class DeleteInternalRepositoryAction extends Action { + + public static final String NAME = "cluster:admin/internal_repository/delete"; + + protected DeleteInternalRepositoryAction() { + super(NAME); + } + + @Override + public AcknowledgedResponse newResponse() { + throw new UnsupportedOperationException(); + } + + @Override + public Writeable.Reader getResponseReader() { + return in -> { + AcknowledgedResponse acknowledgedResponse = new AcknowledgedResponse(); + acknowledgedResponse.readFrom(in); + return acknowledgedResponse; + }; + } + + public static class DeleteInternalRepositoryRequest extends ActionRequest { + + private String name; + + public DeleteInternalRepositoryRequest(String name) { + this.name = name; + } + + @Override + public ActionRequestValidationException validate() { + ActionRequestValidationException validationException = null; + if (name == null) { + validationException = addValidationError("name is missing", validationException); + } + return validationException; + } + + @Override + public void readFrom(StreamInput in) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + DeleteInternalRepositoryRequest that = (DeleteInternalRepositoryRequest) o; + return Objects.equals(name, that.name); + } + + @Override + public int hashCode() { + return Objects.hash(name); + } + + @Override + public String toString() { + return "DeleteInternalRepositoryRequest{" + + "name='" + name + '\'' + + '}'; + } + } + + public static class DeleteInternalRepositoryTransportAction + extends TransportAction { + + private final RepositoriesService repositoriesService; + + @Inject + public DeleteInternalRepositoryTransportAction(RepositoriesService repositoriesService, ActionFilters actionFilters, + TransportService transportService) { + super(NAME, actionFilters, transportService.getTaskManager()); + this.repositoriesService = repositoriesService; + } + + @Override + protected void doExecute(Task task, DeleteInternalRepositoryRequest request, ActionListener listener) { + repositoriesService.unregisterInternalRepository(request.name); + listener.onResponse(new AcknowledgedResponse(true)); + } + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java new file mode 100644 index 0000000000000..49896789367aa --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java @@ -0,0 +1,136 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action.admin.cluster.repositories.put; + +import org.elasticsearch.action.Action; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.TransportAction; +import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.transport.TransportService; + +import java.io.IOException; +import java.util.Objects; + +import static org.elasticsearch.action.ValidateActions.addValidationError; + +public class PutInternalRepositoryAction extends Action { + + public static final String NAME = "cluster:admin/internal_repository/put"; + + protected PutInternalRepositoryAction() { + super(NAME); + } + + @Override + public AcknowledgedResponse newResponse() { + throw new UnsupportedOperationException(); + } + + @Override + public Writeable.Reader getResponseReader() { + return in -> { + AcknowledgedResponse acknowledgedResponse = new AcknowledgedResponse(); + acknowledgedResponse.readFrom(in); + return acknowledgedResponse; + }; + } + + public static class PutInternalRepositoryRequest extends ActionRequest { + + private String name; + private String type; + + public PutInternalRepositoryRequest(String name, String type) { + this.name = name; + this.type = type; + } + + @Override + public ActionRequestValidationException validate() { + ActionRequestValidationException validationException = null; + if (name == null) { + validationException = addValidationError("name is missing", validationException); + } + if (type == null) { + validationException = addValidationError("type is missing", validationException); + } + return validationException; + } + + @Override + public void readFrom(StreamInput in) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + PutInternalRepositoryRequest that = (PutInternalRepositoryRequest) o; + return Objects.equals(name, that.name) && + Objects.equals(type, that.type); + } + + @Override + public int hashCode() { + return Objects.hash(name, type); + } + + @Override + public String toString() { + return "PutInternalRepositoryRequest{" + + "name='" + name + '\'' + + ", type='" + type + '\'' + + '}'; + } + } + + public static class PutInternalRepositoryTransportAction extends TransportAction { + + private final RepositoriesService repositoriesService; + + @Inject + public PutInternalRepositoryTransportAction(RepositoriesService repositoriesService, ActionFilters actionFilters, + TransportService transportService) { + super(NAME, actionFilters, transportService.getTaskManager()); + this.repositoriesService = repositoriesService; + } + + @Override + protected void doExecute(Task task, PutInternalRepositoryRequest request, ActionListener listener) { + repositoriesService.registerInternalRepository(request.name, request.type); + listener.onResponse(new AcknowledgedResponse(true)); + } + } +} diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index 863c0779b5de8..8d1cd71b0c043 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -60,6 +60,7 @@ public class RepositoriesService implements ClusterStateApplier { private static final Logger logger = LogManager.getLogger(RepositoriesService.class); private final Map typesRegistry; + private final Map internalTypesRegistry; private final ClusterService clusterService; @@ -75,6 +76,7 @@ public RepositoriesService(Settings settings, ClusterService clusterService, Tra Map typesRegistry, ThreadPool threadPool) { this.typesRegistry = typesRegistry; + this.internalTypesRegistry = Collections.emptyMap(); this.clusterService = clusterService; this.threadPool = threadPool; // Doesn't make sense to maintain repositories on non-master and non-data nodes @@ -360,6 +362,39 @@ public Repository repository(String repositoryName) { throw new RepositoryMissingException(repositoryName); } + public void registerInternalRepository(String name, String type) { + // TODO: Do we want settings to be empty + RepositoryMetaData metaData = new RepositoryMetaData(name, type, Settings.EMPTY); + + logger.debug("creating repository [{}][{}]", metaData.type(), metaData.name()); + Repository.Factory factory = internalTypesRegistry.get(metaData.type()); + if (factory == null) { + throw new RepositoryException(metaData.name(), "repository type [" + metaData.type() + "] does not exist"); + } + + Repository repository; + try { + repository = factory.create(metaData, internalTypesRegistry::get); + } catch (Exception e) { + logger.warn(() -> new ParameterizedMessage("failed to create repository [{}][{}]", metaData.type(), metaData.name()), e); + throw new RepositoryException(metaData.name(), "failed to create repository", e); + } + + Repository existingRepository = internalRepositories.putIfAbsent(name, repository); + + if (existingRepository != null) { + logger.error(new ParameterizedMessage("Error registering internal [type={}] repository [name={}]. " + + "Internal repository with that name already registered.", metaData.type(), name)); + } + if (repositories.containsKey(name)) { + logger.warn(new ParameterizedMessage("Non-internal repository [name={}] already registered. This repository will block the " + + "usage of internal [type={}] repository [name={}].", name, metaData.type(), name)); + } + + repository.start(); + + } + public void registerInternalRepository(String name, Repository repository) { RepositoryMetaData metadata = repository.getMetadata(); logger.debug(() -> new ParameterizedMessage("Registering internal [type={}] repository [name={}].", metadata.type(), name)); From 68823cc641ca45ea22f0507ad5cda03a3da2b015 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 28 Nov 2018 18:39:01 -0700 Subject: [PATCH 05/20] WIP --- .../elasticsearch/action/ActionModule.java | 6 ++ .../DeleteInternalRepositoryAction.java | 5 +- .../put/PutInternalRepositoryAction.java | 22 ++++-- .../put/TransportPutRepositoryAction.java | 1 - .../java/org/elasticsearch/node/Node.java | 2 - .../plugins/RepositoryPlugin.java | 9 --- .../repositories/RepositoriesModule.java | 8 -- .../repositories/RepositoriesService.java | 49 +++---------- .../java/org/elasticsearch/xpack/ccr/Ccr.java | 9 +-- .../xpack/ccr/CcrRepositoryManager.java | 19 +++-- .../xpack/ccr/CcrRepositoryManagerTests.java | 3 +- .../repository/CcrRepositoryManagerIT.java | 73 +++++++++++++++++++ 12 files changed, 124 insertions(+), 82 deletions(-) create mode 100644 x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/repository/CcrRepositoryManagerIT.java diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 71eddc65c18a4..093a2f30048a2 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -44,10 +44,12 @@ import org.elasticsearch.action.admin.cluster.node.usage.TransportNodesUsageAction; import org.elasticsearch.action.admin.cluster.remote.RemoteInfoAction; import org.elasticsearch.action.admin.cluster.remote.TransportRemoteInfoAction; +import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteInternalRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.delete.TransportDeleteRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesAction; import org.elasticsearch.action.admin.cluster.repositories.get.TransportGetRepositoriesAction; +import org.elasticsearch.action.admin.cluster.repositories.put.PutInternalRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.put.TransportPutRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.verify.TransportVerifyRepositoryAction; @@ -435,6 +437,10 @@ public void reg actions.register(GetRepositoriesAction.INSTANCE, TransportGetRepositoriesAction.class); actions.register(DeleteRepositoryAction.INSTANCE, TransportDeleteRepositoryAction.class); actions.register(VerifyRepositoryAction.INSTANCE, TransportVerifyRepositoryAction.class); + actions.register(PutInternalRepositoryAction.INSTANCE, + PutInternalRepositoryAction.TransportPutInternalRepositoryAction.class); + actions.register(DeleteInternalRepositoryAction.INSTANCE, + DeleteInternalRepositoryAction.TransportDeleteInternalRepositoryAction.class); actions.register(GetSnapshotsAction.INSTANCE, TransportGetSnapshotsAction.class); actions.register(DeleteSnapshotAction.INSTANCE, TransportDeleteSnapshotAction.class); actions.register(CreateSnapshotAction.INSTANCE, TransportCreateSnapshotAction.class); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java index d7f4d11f1a295..f4b33a9fb9720 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java @@ -41,6 +41,7 @@ public class DeleteInternalRepositoryAction extends Action { + public static final DeleteInternalRepositoryAction INSTANCE = new DeleteInternalRepositoryAction(); public static final String NAME = "cluster:admin/internal_repository/delete"; protected DeleteInternalRepositoryAction() { @@ -109,13 +110,13 @@ public String toString() { } } - public static class DeleteInternalRepositoryTransportAction + public static class TransportDeleteInternalRepositoryAction extends TransportAction { private final RepositoriesService repositoriesService; @Inject - public DeleteInternalRepositoryTransportAction(RepositoriesService repositoriesService, ActionFilters actionFilters, + public TransportDeleteInternalRepositoryAction(RepositoriesService repositoriesService, ActionFilters actionFilters, TransportService transportService) { super(NAME, actionFilters, transportService.getTaskManager()); this.repositoriesService = repositoriesService; diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java index 49896789367aa..9ee43074118f5 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java @@ -30,6 +30,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; @@ -41,9 +42,10 @@ public class PutInternalRepositoryAction extends Action { + public static final PutInternalRepositoryAction INSTANCE = new PutInternalRepositoryAction(); public static final String NAME = "cluster:admin/internal_repository/put"; - protected PutInternalRepositoryAction() { + private PutInternalRepositoryAction() { super(NAME); } @@ -65,10 +67,16 @@ public static class PutInternalRepositoryRequest extends ActionRequest { private String name; private String type; + private Settings settings; public PutInternalRepositoryRequest(String name, String type) { + this(name, type, Settings.EMPTY); + } + + public PutInternalRepositoryRequest(String name, String type, Settings settings) { this.name = name; this.type = type; + this.settings = settings; } @Override @@ -99,12 +107,13 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; PutInternalRepositoryRequest that = (PutInternalRepositoryRequest) o; return Objects.equals(name, that.name) && - Objects.equals(type, that.type); + Objects.equals(type, that.type) && + Objects.equals(settings, that.settings); } @Override public int hashCode() { - return Objects.hash(name, type); + return Objects.hash(name, type, settings); } @Override @@ -112,16 +121,17 @@ public String toString() { return "PutInternalRepositoryRequest{" + "name='" + name + '\'' + ", type='" + type + '\'' + + ", settings=" + settings + '}'; } } - public static class PutInternalRepositoryTransportAction extends TransportAction { + public static class TransportPutInternalRepositoryAction extends TransportAction { private final RepositoriesService repositoriesService; @Inject - public PutInternalRepositoryTransportAction(RepositoriesService repositoriesService, ActionFilters actionFilters, + public TransportPutInternalRepositoryAction(RepositoriesService repositoriesService, ActionFilters actionFilters, TransportService transportService) { super(NAME, actionFilters, transportService.getTaskManager()); this.repositoriesService = repositoriesService; @@ -129,7 +139,7 @@ public PutInternalRepositoryTransportAction(RepositoriesService repositoriesServ @Override protected void doExecute(Task task, PutInternalRepositoryRequest request, ActionListener listener) { - repositoriesService.registerInternalRepository(request.name, request.type); + repositoriesService.registerInternalRepository(request.name, request.type, request.settings); listener.onResponse(new AcknowledgedResponse(true)); } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutRepositoryAction.java index a495ba72f35b7..c298afa8bdcb7 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutRepositoryAction.java @@ -87,5 +87,4 @@ public void onFailure(Exception e) { } }); } - } diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index c2691abcbfd60..6b181e20949a1 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -573,8 +573,6 @@ protected Node( actionModule.initRestHandlers(() -> clusterService.state().nodes()); logger.info("initialized"); - repositoriesModule.initRepositoryPlugins(injector.getInstance(RepositoriesService.class)); - success = true; } catch (IOException ex) { throw new ElasticsearchException("failed to bind service", ex); diff --git a/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java b/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java index a9de9eb4cce69..fc847752eac1a 100644 --- a/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java @@ -43,13 +43,4 @@ public interface RepositoryPlugin { default Map getRepositories(Environment env, NamedXContentRegistry namedXContentRegistry) { return Collections.emptyMap(); } - - /** - * Passes the {@link RepositoriesService} to the plugin. - * - * @param repositoriesService instance for the plugin to optionally use - */ - default void supplyRepositoriesService(RepositoriesService repositoriesService) { - - } } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java index 7fefde85bf944..3073e1f0b0ef6 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java @@ -41,10 +41,8 @@ public class RepositoriesModule extends AbstractModule { private final Map repositoryTypes; - private final List repoPlugins; public RepositoriesModule(Environment env, List repoPlugins, NamedXContentRegistry namedXContentRegistry) { - this.repoPlugins = repoPlugins; Map factories = new HashMap<>(); factories.put(FsRepository.TYPE, (metadata) -> new FsRepository(metadata, env, namedXContentRegistry)); @@ -69,10 +67,4 @@ protected void configure() { MapBinder typesBinder = MapBinder.newMapBinder(binder(), String.class, Repository.Factory.class); repositoryTypes.forEach((k, v) -> typesBinder.addBinding(k).toInstance(v)); } - - public void initRepositoryPlugins(RepositoriesService repositoriesService) { - for (RepositoryPlugin plugin : repoPlugins) { - plugin.supplyRepositoriesService(repositoriesService); - } - } } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index 8d1cd71b0c043..ef1af62a5ea60 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -60,7 +60,6 @@ public class RepositoriesService implements ClusterStateApplier { private static final Logger logger = LogManager.getLogger(RepositoriesService.class); private final Map typesRegistry; - private final Map internalTypesRegistry; private final ClusterService clusterService; @@ -76,7 +75,6 @@ public RepositoriesService(Settings settings, ClusterService clusterService, Tra Map typesRegistry, ThreadPool threadPool) { this.typesRegistry = typesRegistry; - this.internalTypesRegistry = Collections.emptyMap(); this.clusterService = clusterService; this.threadPool = threadPool; // Doesn't make sense to maintain repositories on non-master and non-data nodes @@ -362,50 +360,21 @@ public Repository repository(String repositoryName) { throw new RepositoryMissingException(repositoryName); } - public void registerInternalRepository(String name, String type) { - // TODO: Do we want settings to be empty - RepositoryMetaData metaData = new RepositoryMetaData(name, type, Settings.EMPTY); + public void registerInternalRepository(String name, String type, Settings settings) { + RepositoryMetaData metaData = new RepositoryMetaData(name, type, settings); - logger.debug("creating repository [{}][{}]", metaData.type(), metaData.name()); - Repository.Factory factory = internalTypesRegistry.get(metaData.type()); - if (factory == null) { - throw new RepositoryException(metaData.name(), "repository type [" + metaData.type() + "] does not exist"); - } - - Repository repository; - try { - repository = factory.create(metaData, internalTypesRegistry::get); - } catch (Exception e) { - logger.warn(() -> new ParameterizedMessage("failed to create repository [{}][{}]", metaData.type(), metaData.name()), e); - throw new RepositoryException(metaData.name(), "failed to create repository", e); - } + Repository repository = createRepository(metaData); Repository existingRepository = internalRepositories.putIfAbsent(name, repository); if (existingRepository != null) { - logger.error(new ParameterizedMessage("Error registering internal [type={}] repository [name={}]. " + + logger.error(new ParameterizedMessage("Error registering internal repository [{}][{}]. " + "Internal repository with that name already registered.", metaData.type(), name)); + repository.close(); } if (repositories.containsKey(name)) { - logger.warn(new ParameterizedMessage("Non-internal repository [name={}] already registered. This repository will block the " + - "usage of internal [type={}] repository [name={}].", name, metaData.type(), name)); - } - - repository.start(); - - } - - public void registerInternalRepository(String name, Repository repository) { - RepositoryMetaData metadata = repository.getMetadata(); - logger.debug(() -> new ParameterizedMessage("Registering internal [type={}] repository [name={}].", metadata.type(), name)); - Repository existingRepository = internalRepositories.putIfAbsent(name, repository); - if (existingRepository != null) { - logger.error(new ParameterizedMessage("Error registering internal [type={}] repository [name={}]. " + - "Internal repository with that name already registered.", metadata.type(), name)); - } - if (repositories.containsKey(name)) { - logger.warn(new ParameterizedMessage("Non-internal repository [name={}] already registered. This repository will block the " + - "usage of internal [type={}] repository [name={}].", name, metadata.type(), name)); + logger.warn(new ParameterizedMessage("Non-internal repository [{}] already registered. This repository will block the " + + "usage of internal repository [{}][{}].", name, metaData.type(), name)); } } @@ -415,11 +384,11 @@ public void unregisterInternalRepository(String name) { Repository repository = internalRepositories.remove(name); RepositoryMetaData metadata = repository.getMetadata(); if (repository != null) { - logger.debug(() -> new ParameterizedMessage("Unregistering internal [type={}] repository [name={}].", metadata.type(), name)); + logger.debug(() -> new ParameterizedMessage("Unregistering internal repository [{}][{}].", metadata.type(), name)); closeRepository(repository); repository.close(); } else { - logger.warn(() -> new ParameterizedMessage("Attempted to unregistering internal [type={}] repository [name={}]. " + + logger.warn(() -> new ParameterizedMessage("Attempted to unregistering internal repository [{}][{}]. " + "Repository could not found.", metadata.type(), name)); } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java index af4b9727301cb..a99a83b39f056 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java @@ -10,6 +10,7 @@ import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.client.Client; +import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.service.ClusterService; @@ -148,6 +149,7 @@ public Collection createComponents( } this.clusterService.set(clusterService); + this.repositoryManager.set(new CcrRepositoryManager(settings, clusterService, (NodeClient) client)); return Arrays.asList( ccrLicenseChecker, @@ -266,13 +268,6 @@ public List> getExecutorBuilders(Settings settings) { return Collections.singletonList(new FixedExecutorBuilder(settings, CCR_THREAD_POOL_NAME, 32, 100, "xpack.ccr.ccr_thread_pool")); } - @Override - public void supplyRepositoriesService(RepositoriesService repositoriesService) { - ClusterService clusterService = this.clusterService.get(); - assert clusterService != null : "ClusterService not set."; - repositoryManager.set(new CcrRepositoryManager(settings, clusterService, repositoriesService)); - } - protected XPackLicenseState getLicenseState() { return XPackPlugin.getSharedLicenseState(); } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java index f3332bb3951f2..20cb558f2dbf6 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java @@ -6,6 +6,11 @@ package org.elasticsearch.xpack.ccr; +import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteInternalRepositoryAction; +import org.elasticsearch.action.admin.cluster.repositories.put.PutInternalRepositoryAction; +import org.elasticsearch.client.Client; +import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; @@ -13,18 +18,19 @@ import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.transport.RemoteClusterAware; import org.elasticsearch.xpack.ccr.repository.CcrRepository; +import org.elasticsearch.xpack.core.ccr.action.DeleteAutoFollowPatternAction; import java.util.List; import java.util.Set; class CcrRepositoryManager extends RemoteClusterAware { - private final RepositoriesService repositoriesService; + private final NodeClient client; private final Set clusters = ConcurrentCollections.newConcurrentSet(); - CcrRepositoryManager(Settings settings, ClusterService clusterService, RepositoriesService repositoriesService) { + CcrRepositoryManager(Settings settings, ClusterService clusterService, NodeClient client) { super(settings); - this.repositoriesService = repositoriesService; + this.client = client; clusters.addAll(buildRemoteClustersDynamicConfig(settings).keySet()); listenForUpdates(clusterService.getClusterSettings()); } @@ -38,12 +44,13 @@ protected Set getRemoteClusterNames() { protected void updateRemoteCluster(String clusterAlias, List addresses, String proxyAddress) { if (addresses.isEmpty()) { if (clusters.remove(clusterAlias)) { - repositoriesService.unregisterInternalRepository(clusterAlias); + DeleteAutoFollowPatternAction.Request request = new DeleteAutoFollowPatternAction.Request(clusterAlias); + client.execute(DeleteInternalRepositoryAction.INSTANCE, request); } } else { if (clusters.add(clusterAlias)) { - RepositoryMetaData metaData = new RepositoryMetaData(clusterAlias, CcrRepository.TYPE, settings); - repositoriesService.registerInternalRepository(clusterAlias, new CcrRepository(metaData, settings)); + ActionRequest request = new PutInternalRepositoryAction.PutInternalRepositoryRequest(clusterAlias, CcrRepository.TYPE); + client.execute(PutInternalRepositoryAction.INSTANCE, request); } } } diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerTests.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerTests.java index 4372fa894e142..0bdb934adc0f3 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerTests.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.ccr; +import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.UUIDs; @@ -44,7 +45,7 @@ public void setUp() throws Exception { when(clusterService.getClusterSettings()).thenReturn(clusterSettings); repositoriesService = new RepositoriesService(settings, clusterService, transportService, null, threadPool); - CcrRepositoryManager repositoryManager = new CcrRepositoryManager(settings, clusterService, repositoriesService); + CcrRepositoryManager repositoryManager = new CcrRepositoryManager(settings, clusterService, new NodeClient(Settings.EMPTY, threadPool)); } public void testRepositoryIsRegistered() { diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/repository/CcrRepositoryManagerIT.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/repository/CcrRepositoryManagerIT.java new file mode 100644 index 0000000000000..4f4bb79e48bd5 --- /dev/null +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/repository/CcrRepositoryManagerIT.java @@ -0,0 +1,73 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.ccr.repository; + +import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesResponse; +import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.CcrIntegTestCase; + +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; + +public class CcrRepositoryManagerIT extends CcrIntegTestCase { + + public void testThatRepositoryIsPutAndRemovedWhenRemoteClusterIsUpdated() throws Exception { + assertBusy(() -> { + GetRepositoriesResponse response = followerClient() + .admin() + .cluster() + .prepareGetRepositories() + .get(); + assertEquals(1, response.repositories().size()); + assertEquals(CcrRepository.TYPE, response.repositories().get(0).type()); + assertEquals("leader_cluster", response.repositories().get(0).name()); + }); + + ClusterUpdateSettingsRequest putFollowerRequest = new ClusterUpdateSettingsRequest(); + String address = getFollowerCluster().getDataNodeInstance(TransportService.class).boundAddress().publishAddress().toString(); + putFollowerRequest.persistentSettings(Settings.builder().put("cluster.remote.follower_cluster_copy.seeds", address)); + assertAcked(followerClient().admin().cluster().updateSettings(putFollowerRequest).actionGet()); + + assertBusy(() -> { + GetRepositoriesResponse response = followerClient() + .admin() + .cluster() + .prepareGetRepositories() + .get(); + assertEquals(2, response.repositories().size()); + }); + + ClusterUpdateSettingsRequest deleteLeaderRequest = new ClusterUpdateSettingsRequest(); + deleteLeaderRequest.persistentSettings(Settings.builder().put("cluster.remote.leader_cluster.seeds", "")); + assertAcked(followerClient().admin().cluster().updateSettings(deleteLeaderRequest).actionGet()); + + assertBusy(() -> { + GetRepositoriesResponse response = followerClient() + .admin() + .cluster() + .prepareGetRepositories() + .get(); + assertEquals(1, response.repositories().size()); + assertEquals(CcrRepository.TYPE, response.repositories().get(0).type()); + assertEquals("follower_cluster_copy", response.repositories().get(0).name()); + }); + + ClusterUpdateSettingsRequest deleteFollowerRequest = new ClusterUpdateSettingsRequest(); + deleteFollowerRequest.persistentSettings(Settings.builder().put("cluster.remote.follower_cluster_copy.seeds", "")); + assertAcked(followerClient().admin().cluster().updateSettings(deleteFollowerRequest).actionGet()); + + assertBusy(() -> { + GetRepositoriesResponse response = followerClient() + .admin() + .cluster() + .prepareGetRepositories() + .get(); + assertEquals(0, response.repositories().size()); + }); + } +} From 719e7e8542cdbb1aab3c6fe9bc60b9c8dd8acff9 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 28 Nov 2018 18:58:22 -0700 Subject: [PATCH 06/20] WIP --- .../repositories/RepositoriesService.java | 2 +- .../java/org/elasticsearch/xpack/ccr/Ccr.java | 9 +++ .../xpack/ccr/CcrRepositoryManager.java | 7 +- .../CcrRepositoryManagerIT.java | 59 ++++++++-------- .../xpack/ccr/CcrRepositoryManagerTests.java | 68 ------------------- 5 files changed, 41 insertions(+), 104 deletions(-) rename x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/{repository => }/CcrRepositoryManagerIT.java (50%) delete mode 100644 x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerTests.java diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index ef1af62a5ea60..266962b1b9a22 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -388,7 +388,7 @@ public void unregisterInternalRepository(String name) { closeRepository(repository); repository.close(); } else { - logger.warn(() -> new ParameterizedMessage("Attempted to unregistering internal repository [{}][{}]. " + + logger.warn(() -> new ParameterizedMessage("Attempted to unregistered internal repository [{}][{}]. " + "Repository could not found.", metadata.type(), name)); } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java index a99a83b39f056..92c09624bdfc1 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java @@ -36,6 +36,7 @@ import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.RepositoryPlugin; import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.repositories.Repository; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.script.ScriptService; @@ -60,6 +61,7 @@ import org.elasticsearch.xpack.ccr.action.bulk.BulkShardOperationsAction; import org.elasticsearch.xpack.ccr.action.bulk.TransportBulkShardOperationsAction; import org.elasticsearch.xpack.ccr.index.engine.FollowingEngineFactory; +import org.elasticsearch.xpack.ccr.repository.CcrRepository; import org.elasticsearch.xpack.ccr.rest.RestCcrStatsAction; import org.elasticsearch.xpack.ccr.rest.RestDeleteAutoFollowPatternAction; import org.elasticsearch.xpack.ccr.rest.RestFollowStatsAction; @@ -85,6 +87,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.function.Supplier; @@ -268,6 +271,12 @@ public List> getExecutorBuilders(Settings settings) { return Collections.singletonList(new FixedExecutorBuilder(settings, CCR_THREAD_POOL_NAME, 32, 100, "xpack.ccr.ccr_thread_pool")); } + @Override + public Map getRepositories(Environment env, NamedXContentRegistry namedXContentRegistry) { + Repository.Factory repositoryFactory = (metadata) -> new CcrRepository(metadata, settings); + return Collections.singletonMap(CcrRepository.TYPE, repositoryFactory); + } + protected XPackLicenseState getLicenseState() { return XPackPlugin.getSharedLicenseState(); } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java index 20cb558f2dbf6..716a4a24974c1 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java @@ -9,16 +9,12 @@ import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteInternalRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.put.PutInternalRepositoryAction; -import org.elasticsearch.client.Client; import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; -import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.transport.RemoteClusterAware; import org.elasticsearch.xpack.ccr.repository.CcrRepository; -import org.elasticsearch.xpack.core.ccr.action.DeleteAutoFollowPatternAction; import java.util.List; import java.util.Set; @@ -44,7 +40,8 @@ protected Set getRemoteClusterNames() { protected void updateRemoteCluster(String clusterAlias, List addresses, String proxyAddress) { if (addresses.isEmpty()) { if (clusters.remove(clusterAlias)) { - DeleteAutoFollowPatternAction.Request request = new DeleteAutoFollowPatternAction.Request(clusterAlias); + DeleteInternalRepositoryAction.DeleteInternalRepositoryRequest request = + new DeleteInternalRepositoryAction.DeleteInternalRepositoryRequest(clusterAlias); client.execute(DeleteInternalRepositoryAction.INSTANCE, request); } } else { diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/repository/CcrRepositoryManagerIT.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java similarity index 50% rename from x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/repository/CcrRepositoryManagerIT.java rename to x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java index 4f4bb79e48bd5..e6ac60892d017 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/repository/CcrRepositoryManagerIT.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java @@ -4,13 +4,16 @@ * you may not use this file except in compliance with the Elastic License. */ -package org.elasticsearch.xpack.ccr.repository; +package org.elasticsearch.xpack.ccr; -import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesResponse; import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.repositories.Repository; +import org.elasticsearch.repositories.RepositoryMissingException; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.CcrIntegTestCase; +import org.elasticsearch.xpack.ccr.repository.CcrRepository; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; @@ -18,14 +21,15 @@ public class CcrRepositoryManagerIT extends CcrIntegTestCase { public void testThatRepositoryIsPutAndRemovedWhenRemoteClusterIsUpdated() throws Exception { assertBusy(() -> { - GetRepositoriesResponse response = followerClient() - .admin() - .cluster() - .prepareGetRepositories() - .get(); - assertEquals(1, response.repositories().size()); - assertEquals(CcrRepository.TYPE, response.repositories().get(0).type()); - assertEquals("leader_cluster", response.repositories().get(0).name()); + final RepositoriesService repositoriesService = + getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); + try { + Repository repository = repositoriesService.repository("leader_cluster"); + assertEquals(CcrRepository.TYPE, repository.getMetadata().type()); + assertEquals("leader_cluster", repository.getMetadata().name()); + } catch (RepositoryMissingException e) { + fail("need repository"); + } }); ClusterUpdateSettingsRequest putFollowerRequest = new ClusterUpdateSettingsRequest(); @@ -34,12 +38,15 @@ public void testThatRepositoryIsPutAndRemovedWhenRemoteClusterIsUpdated() throws assertAcked(followerClient().admin().cluster().updateSettings(putFollowerRequest).actionGet()); assertBusy(() -> { - GetRepositoriesResponse response = followerClient() - .admin() - .cluster() - .prepareGetRepositories() - .get(); - assertEquals(2, response.repositories().size()); + final RepositoriesService repositoriesService = + getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); + try { + Repository repository = repositoriesService.repository("follower_cluster_copy"); + assertEquals(CcrRepository.TYPE, repository.getMetadata().type()); + assertEquals("follower_cluster_copy", repository.getMetadata().name()); + } catch (RepositoryMissingException e) { + fail("need repository"); + } }); ClusterUpdateSettingsRequest deleteLeaderRequest = new ClusterUpdateSettingsRequest(); @@ -47,14 +54,9 @@ public void testThatRepositoryIsPutAndRemovedWhenRemoteClusterIsUpdated() throws assertAcked(followerClient().admin().cluster().updateSettings(deleteLeaderRequest).actionGet()); assertBusy(() -> { - GetRepositoriesResponse response = followerClient() - .admin() - .cluster() - .prepareGetRepositories() - .get(); - assertEquals(1, response.repositories().size()); - assertEquals(CcrRepository.TYPE, response.repositories().get(0).type()); - assertEquals("follower_cluster_copy", response.repositories().get(0).name()); + final RepositoriesService repositoriesService = + getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository("leader_cluster")); }); ClusterUpdateSettingsRequest deleteFollowerRequest = new ClusterUpdateSettingsRequest(); @@ -62,12 +64,9 @@ public void testThatRepositoryIsPutAndRemovedWhenRemoteClusterIsUpdated() throws assertAcked(followerClient().admin().cluster().updateSettings(deleteFollowerRequest).actionGet()); assertBusy(() -> { - GetRepositoriesResponse response = followerClient() - .admin() - .cluster() - .prepareGetRepositories() - .get(); - assertEquals(0, response.repositories().size()); + final RepositoriesService repositoriesService = + getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository("follower_cluster_copy")); }); } } diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerTests.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerTests.java deleted file mode 100644 index 0bdb934adc0f3..0000000000000 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerTests.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -package org.elasticsearch.xpack.ccr; - -import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.UUIDs; -import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.repositories.RepositoriesService; -import org.elasticsearch.repositories.RepositoryMissingException; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.transport.Transport; -import org.elasticsearch.transport.TransportService; -import org.elasticsearch.xpack.ccr.repository.CcrRepository; - -import java.util.Collections; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -public class CcrRepositoryManagerTests extends ESTestCase { - - private RepositoriesService repositoriesService; - private ClusterSettings clusterSettings; - - @Override - public void setUp() throws Exception { - super.setUp(); - - Settings settings = Settings.EMPTY; - ThreadPool threadPool = mock(ThreadPool.class); - TransportService transportService = new TransportService(settings, mock(Transport.class), threadPool, - TransportService.NOOP_TRANSPORT_INTERCEPTOR, - boundAddress -> DiscoveryNode.createLocal(settings, boundAddress.publishAddress(), UUIDs.randomBase64UUID()), null, - Collections.emptySet()); - ClusterService clusterService = mock(ClusterService.class); - clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - when(clusterService.getClusterSettings()).thenReturn(clusterSettings); - repositoriesService = new RepositoriesService(settings, clusterService, - transportService, null, threadPool); - CcrRepositoryManager repositoryManager = new CcrRepositoryManager(settings, clusterService, new NodeClient(Settings.EMPTY, threadPool)); - } - - public void testRepositoryIsRegistered() { - String clusterName = "cluster_1"; - clusterSettings.applySettings(Settings.builder().put("cluster.remote." + clusterName + ".seeds", "192.168.0.1:8080").build()); - CcrRepository repository = (CcrRepository) repositoriesService.repository(clusterName); - assertEquals(CcrRepository.TYPE, repository.getMetadata().type()); - assertEquals(clusterName, repository.getMetadata().name()); - } - - public void testRepositoryIsUnregistered() { - String clusterName = "cluster_1"; - clusterSettings.applySettings(Settings.builder().put("cluster.remote." + clusterName + ".seeds", "192.168.0.1:8080").build()); - // Will throw is it does not exist - repositoriesService.repository(clusterName); - - clusterSettings.applySettings(Settings.builder().put("cluster.remote.cluster_1.seeds", "").build()); - expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository("cluster_1")); - } -} From 194957864d255a9590ebbfcf33997d1e44d94531 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 28 Nov 2018 20:00:13 -0700 Subject: [PATCH 07/20] WIP --- server/src/main/java/org/elasticsearch/node/Node.java | 5 +---- .../java/org/elasticsearch/plugins/RepositoryPlugin.java | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 6b181e20949a1..f3433dfa1ba1a 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -132,7 +132,6 @@ import org.elasticsearch.plugins.ScriptPlugin; import org.elasticsearch.plugins.SearchPlugin; import org.elasticsearch.repositories.RepositoriesModule; -import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.rest.RestController; import org.elasticsearch.script.ScriptModule; import org.elasticsearch.script.ScriptService; @@ -397,9 +396,7 @@ protected Node( .flatMap(p -> p.getNamedXContent().stream()), ClusterModule.getNamedXWriteables().stream()) .flatMap(Function.identity()).collect(toList())); - RepositoriesModule repositoriesModule = new RepositoriesModule(this.environment, pluginsService.filterPlugins(RepositoryPlugin.class), - xContentRegistry); - modules.add(repositoriesModule); + modules.add(new RepositoriesModule(this.environment, pluginsService.filterPlugins(RepositoryPlugin.class), xContentRegistry)); final MetaStateService metaStateService = new MetaStateService(nodeEnvironment, xContentRegistry); // collect engine factory providers from server and from plugins diff --git a/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java b/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java index fc847752eac1a..a3af52a9a4aca 100644 --- a/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java @@ -24,7 +24,6 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.env.Environment; -import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.repositories.Repository; /** From 0f77fceab8978b29b7c91992c9e26eeddcf05dd3 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 28 Nov 2018 20:31:41 -0700 Subject: [PATCH 08/20] WIP --- .../elasticsearch/xpack/ccr/CcrRepositoryManager.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java index 716a4a24974c1..5f0815f246bd9 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java @@ -9,6 +9,8 @@ import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteInternalRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.put.PutInternalRepositoryAction; +import org.elasticsearch.action.support.PlainActionFuture; +import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; @@ -42,12 +44,16 @@ protected void updateRemoteCluster(String clusterAlias, List addresses, if (clusters.remove(clusterAlias)) { DeleteInternalRepositoryAction.DeleteInternalRepositoryRequest request = new DeleteInternalRepositoryAction.DeleteInternalRepositoryRequest(clusterAlias); - client.execute(DeleteInternalRepositoryAction.INSTANCE, request); + PlainActionFuture future = PlainActionFuture.newFuture(); + client.executeLocally(DeleteInternalRepositoryAction.INSTANCE, request, future); + assert future.isDone() : "Should be completed as it is executed synchronously"; } } else { if (clusters.add(clusterAlias)) { ActionRequest request = new PutInternalRepositoryAction.PutInternalRepositoryRequest(clusterAlias, CcrRepository.TYPE); - client.execute(PutInternalRepositoryAction.INSTANCE, request); + PlainActionFuture future = PlainActionFuture.newFuture(); + client.executeLocally(PutInternalRepositoryAction.INSTANCE, request, future); + assert future.isDone() : "Should be completed as it is executed synchronously"; } } } From 3dd4a4499d5f344a078d03b49e063ff81c05ffbd Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Thu, 29 Nov 2018 14:35:24 -0700 Subject: [PATCH 09/20] WIP --- .../elasticsearch/action/ActionModule.java | 8 +- .../DeleteInternalRepositoryAction.java | 66 ----------- .../DeleteInternalRepositoryRequest.java | 82 ++++++++++++++ ...ansportDeleteInternalRepositoryAction.java | 47 ++++++++ .../put/PutInternalRepositoryAction.java | 90 --------------- .../put/PutInternalRepositoryRequest.java | 106 ++++++++++++++++++ .../TransportPutInternalRepositoryAction.java | 47 ++++++++ .../plugins/RepositoryPlugin.java | 13 +++ .../repositories/RepositoriesModule.java | 14 ++- .../repositories/RepositoriesService.java | 41 +++++-- ...ClusterStateServiceRandomUpdatesTests.java | 2 +- .../java/org/elasticsearch/xpack/ccr/Ccr.java | 2 +- .../xpack/ccr/CcrRepositoryManager.java | 7 +- .../core/LocalStateCompositeXPackPlugin.java | 8 ++ 14 files changed, 355 insertions(+), 178 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryRequest.java create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteInternalRepositoryAction.java create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryRequest.java create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutInternalRepositoryAction.java diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 093a2f30048a2..0a86cc368b27b 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -46,11 +46,13 @@ import org.elasticsearch.action.admin.cluster.remote.TransportRemoteInfoAction; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteInternalRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryAction; +import org.elasticsearch.action.admin.cluster.repositories.delete.TransportDeleteInternalRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.delete.TransportDeleteRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesAction; import org.elasticsearch.action.admin.cluster.repositories.get.TransportGetRepositoriesAction; import org.elasticsearch.action.admin.cluster.repositories.put.PutInternalRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryAction; +import org.elasticsearch.action.admin.cluster.repositories.put.TransportPutInternalRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.put.TransportPutRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.verify.TransportVerifyRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.verify.VerifyRepositoryAction; @@ -437,10 +439,8 @@ public void reg actions.register(GetRepositoriesAction.INSTANCE, TransportGetRepositoriesAction.class); actions.register(DeleteRepositoryAction.INSTANCE, TransportDeleteRepositoryAction.class); actions.register(VerifyRepositoryAction.INSTANCE, TransportVerifyRepositoryAction.class); - actions.register(PutInternalRepositoryAction.INSTANCE, - PutInternalRepositoryAction.TransportPutInternalRepositoryAction.class); - actions.register(DeleteInternalRepositoryAction.INSTANCE, - DeleteInternalRepositoryAction.TransportDeleteInternalRepositoryAction.class); + actions.register(PutInternalRepositoryAction.INSTANCE, TransportPutInternalRepositoryAction.class); + actions.register(DeleteInternalRepositoryAction.INSTANCE, TransportDeleteInternalRepositoryAction.class); actions.register(GetSnapshotsAction.INSTANCE, TransportGetSnapshotsAction.class); actions.register(DeleteSnapshotAction.INSTANCE, TransportDeleteSnapshotAction.class); actions.register(CreateSnapshotAction.INSTANCE, TransportCreateSnapshotAction.class); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java index f4b33a9fb9720..d110b393d368d 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java @@ -62,70 +62,4 @@ public Writeable.Reader getResponseReader() { }; } - public static class DeleteInternalRepositoryRequest extends ActionRequest { - - private String name; - - public DeleteInternalRepositoryRequest(String name) { - this.name = name; - } - - @Override - public ActionRequestValidationException validate() { - ActionRequestValidationException validationException = null; - if (name == null) { - validationException = addValidationError("name is missing", validationException); - } - return validationException; - } - - @Override - public void readFrom(StreamInput in) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - DeleteInternalRepositoryRequest that = (DeleteInternalRepositoryRequest) o; - return Objects.equals(name, that.name); - } - - @Override - public int hashCode() { - return Objects.hash(name); - } - - @Override - public String toString() { - return "DeleteInternalRepositoryRequest{" + - "name='" + name + '\'' + - '}'; - } - } - - public static class TransportDeleteInternalRepositoryAction - extends TransportAction { - - private final RepositoriesService repositoriesService; - - @Inject - public TransportDeleteInternalRepositoryAction(RepositoriesService repositoriesService, ActionFilters actionFilters, - TransportService transportService) { - super(NAME, actionFilters, transportService.getTaskManager()); - this.repositoriesService = repositoriesService; - } - - @Override - protected void doExecute(Task task, DeleteInternalRepositoryRequest request, ActionListener listener) { - repositoriesService.unregisterInternalRepository(request.name); - listener.onResponse(new AcknowledgedResponse(true)); - } - } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryRequest.java new file mode 100644 index 0000000000000..2567c0fb637e5 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryRequest.java @@ -0,0 +1,82 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action.admin.cluster.repositories.delete; + +import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; + +import java.io.IOException; +import java.util.Objects; + +import static org.elasticsearch.action.ValidateActions.addValidationError; + +public class DeleteInternalRepositoryRequest extends ActionRequest { + + private String name; + + public DeleteInternalRepositoryRequest(String name) { + this.name = name; + } + + @Override + public ActionRequestValidationException validate() { + ActionRequestValidationException validationException = null; + if (name == null) { + validationException = addValidationError("name is missing", validationException); + } + return validationException; + } + + @Override + public void readFrom(StreamInput in) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + throw new UnsupportedOperationException(); + } + + public String getName() { + return name; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + DeleteInternalRepositoryRequest that = (DeleteInternalRepositoryRequest) o; + return Objects.equals(name, that.name); + } + + @Override + public int hashCode() { + return Objects.hash(name); + } + + @Override + public String toString() { + return "DeleteInternalRepositoryRequest{" + + "name='" + name + '\'' + + '}'; + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteInternalRepositoryAction.java new file mode 100644 index 0000000000000..1190cd94491d6 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteInternalRepositoryAction.java @@ -0,0 +1,47 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action.admin.cluster.repositories.delete; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.TransportAction; +import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.transport.TransportService; + +public class TransportDeleteInternalRepositoryAction extends TransportAction { + + private final RepositoriesService repositoriesService; + + @Inject + public TransportDeleteInternalRepositoryAction(RepositoriesService repositoriesService, ActionFilters actionFilters, + TransportService transportService) { + super(DeleteInternalRepositoryAction.NAME, actionFilters, transportService.getTaskManager()); + this.repositoriesService = repositoriesService; + } + + @Override + protected void doExecute(Task task, DeleteInternalRepositoryRequest request, ActionListener listener) { + repositoriesService.unregisterInternalRepository(request.getName()); + listener.onResponse(new AcknowledgedResponse(true)); + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java index 9ee43074118f5..7e746b16500db 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java @@ -21,25 +21,15 @@ import org.elasticsearch.action.Action; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.ActionRequest; -import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; -import java.io.IOException; -import java.util.Objects; - -import static org.elasticsearch.action.ValidateActions.addValidationError; - public class PutInternalRepositoryAction extends Action { public static final PutInternalRepositoryAction INSTANCE = new PutInternalRepositoryAction(); @@ -63,84 +53,4 @@ public Writeable.Reader getResponseReader() { }; } - public static class PutInternalRepositoryRequest extends ActionRequest { - - private String name; - private String type; - private Settings settings; - - public PutInternalRepositoryRequest(String name, String type) { - this(name, type, Settings.EMPTY); - } - - public PutInternalRepositoryRequest(String name, String type, Settings settings) { - this.name = name; - this.type = type; - this.settings = settings; - } - - @Override - public ActionRequestValidationException validate() { - ActionRequestValidationException validationException = null; - if (name == null) { - validationException = addValidationError("name is missing", validationException); - } - if (type == null) { - validationException = addValidationError("type is missing", validationException); - } - return validationException; - } - - @Override - public void readFrom(StreamInput in) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - PutInternalRepositoryRequest that = (PutInternalRepositoryRequest) o; - return Objects.equals(name, that.name) && - Objects.equals(type, that.type) && - Objects.equals(settings, that.settings); - } - - @Override - public int hashCode() { - return Objects.hash(name, type, settings); - } - - @Override - public String toString() { - return "PutInternalRepositoryRequest{" + - "name='" + name + '\'' + - ", type='" + type + '\'' + - ", settings=" + settings + - '}'; - } - } - - public static class TransportPutInternalRepositoryAction extends TransportAction { - - private final RepositoriesService repositoriesService; - - @Inject - public TransportPutInternalRepositoryAction(RepositoriesService repositoriesService, ActionFilters actionFilters, - TransportService transportService) { - super(NAME, actionFilters, transportService.getTaskManager()); - this.repositoriesService = repositoriesService; - } - - @Override - protected void doExecute(Task task, PutInternalRepositoryRequest request, ActionListener listener) { - repositoriesService.registerInternalRepository(request.name, request.type, request.settings); - listener.onResponse(new AcknowledgedResponse(true)); - } - } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryRequest.java new file mode 100644 index 0000000000000..d7ac1aa96034c --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryRequest.java @@ -0,0 +1,106 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action.admin.cluster.repositories.put; + +import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.settings.Settings; + +import java.io.IOException; +import java.util.Objects; + +import static org.elasticsearch.action.ValidateActions.addValidationError; + +public class PutInternalRepositoryRequest extends ActionRequest { + + private String name; + private String type; + private Settings settings; + + public PutInternalRepositoryRequest(String name, String type) { + this(name, type, Settings.EMPTY); + } + + public PutInternalRepositoryRequest(String name, String type, Settings settings) { + this.name = name; + this.type = type; + this.settings = settings; + } + + @Override + public ActionRequestValidationException validate() { + ActionRequestValidationException validationException = null; + if (name == null) { + validationException = addValidationError("name is missing", validationException); + } + if (type == null) { + validationException = addValidationError("type is missing", validationException); + } + return validationException; + } + + @Override + public void readFrom(StreamInput in) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + throw new UnsupportedOperationException(); + } + + public String getName() { + return name; + } + + public String getType() { + return type; + } + + public Settings getSettings() { + return settings; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + PutInternalRepositoryRequest that = (PutInternalRepositoryRequest) o; + return Objects.equals(name, that.name) && + Objects.equals(type, that.type) && + Objects.equals(settings, that.settings); + } + + @Override + public int hashCode() { + return Objects.hash(name, type, settings); + } + + @Override + public String toString() { + return "PutInternalRepositoryRequest{" + + "name='" + name + '\'' + + ", type='" + type + '\'' + + ", settings=" + settings + + '}'; + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutInternalRepositoryAction.java new file mode 100644 index 0000000000000..2db5db99259f5 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutInternalRepositoryAction.java @@ -0,0 +1,47 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action.admin.cluster.repositories.put; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.TransportAction; +import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.transport.TransportService; + +public class TransportPutInternalRepositoryAction extends TransportAction { + + private final RepositoriesService repositoriesService; + + @Inject + public TransportPutInternalRepositoryAction(RepositoriesService repositoriesService, ActionFilters actionFilters, + TransportService transportService) { + super(PutInternalRepositoryAction.NAME, actionFilters, transportService.getTaskManager()); + this.repositoriesService = repositoriesService; + } + + @Override + protected void doExecute(Task task, PutInternalRepositoryRequest request, ActionListener listener) { + repositoriesService.registerInternalRepository(request.getName(), request.getType(), request.getSettings()); + listener.onResponse(new AcknowledgedResponse(true)); + } +} diff --git a/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java b/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java index a3af52a9a4aca..5c15040609863 100644 --- a/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/RepositoryPlugin.java @@ -42,4 +42,17 @@ public interface RepositoryPlugin { default Map getRepositories(Environment env, NamedXContentRegistry namedXContentRegistry) { return Collections.emptyMap(); } + + /** + * Returns internal repository types added by this plugin. Internal repositories cannot be registered + * through the external API. + * + * @param env The environment for the local node, which may be used for the local settings and path.repo + * + * The key of the returned {@link Map} is the type name of the repository and + * the value is a factory to construct the {@link Repository} interface. + */ + default Map getInternalRepositories(Environment env, NamedXContentRegistry namedXContentRegistry) { + return Collections.emptyMap(); + } } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java index 637ec2d8dfbc1..9b3d20cd78efa 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java @@ -58,8 +58,20 @@ public RepositoriesModule(Environment env, List repoPlugins, T } } + Map internalFactories = new HashMap<>(); + for (RepositoryPlugin repoPlugin : repoPlugins) { + Map newRepoTypes = repoPlugin.getInternalRepositories(env, namedXContentRegistry); + for (Map.Entry entry : newRepoTypes.entrySet()) { + if (internalFactories.put(entry.getKey(), entry.getValue()) != null) { + throw new IllegalArgumentException("Internal repository type [" + entry.getKey() + "] is already registered"); + } + } + } + Map repositoryTypes = Collections.unmodifiableMap(factories); - repositoriesService = new RepositoriesService(env.settings(), clusterService, transportService, repositoryTypes, threadPool); + Map internalRepositoryTypes = Collections.unmodifiableMap(internalFactories); + repositoriesService = new RepositoriesService(env.settings(), clusterService, transportService, repositoryTypes, + internalRepositoryTypes, threadPool); } @Override diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index 77580c33b46aa..fac2e58b7e809 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -59,6 +59,7 @@ public class RepositoriesService implements ClusterStateApplier { private static final Logger logger = LogManager.getLogger(RepositoriesService.class); private final Map typesRegistry; + private final Map internalTypesRegistry; private final ClusterService clusterService; @@ -70,9 +71,10 @@ public class RepositoriesService implements ClusterStateApplier { private volatile Map internalRepositories = ConcurrentCollections.newConcurrentMap(); public RepositoriesService(Settings settings, ClusterService clusterService, TransportService transportService, - Map typesRegistry, + Map typesRegistry, Map internalTypesRegistry, ThreadPool threadPool) { this.typesRegistry = typesRegistry; + this.internalTypesRegistry = internalTypesRegistry; this.clusterService = clusterService; this.threadPool = threadPool; // Doesn't make sense to maintain repositories on non-master and non-data nodes @@ -311,7 +313,7 @@ public void applyClusterState(ClusterChangedEvent event) { closeRepository(repository); repository = null; try { - repository = createRepository(repositoryMetaData); + repository = createRepository(repositoryMetaData, typesRegistry); } catch (RepositoryException ex) { // TODO: this catch is bogus, it means the old repo is already closed, // but we have nothing to replace it @@ -320,7 +322,7 @@ public void applyClusterState(ClusterChangedEvent event) { } } else { try { - repository = createRepository(repositoryMetaData); + repository = createRepository(repositoryMetaData, typesRegistry); } catch (RepositoryException ex) { logger.warn(() -> new ParameterizedMessage("failed to create repository [{}]", repositoryMetaData.name()), ex); } @@ -360,8 +362,7 @@ public Repository repository(String repositoryName) { public void registerInternalRepository(String name, String type, Settings settings) { RepositoryMetaData metaData = new RepositoryMetaData(name, type, settings); - - Repository repository = createRepository(metaData); + Repository repository = createRepository(metaData, internalTypesRegistry); Repository existingRepository = internalRepositories.putIfAbsent(name, repository); @@ -377,8 +378,6 @@ public void registerInternalRepository(String name, String type, Settings settin } public void unregisterInternalRepository(String name) { - // TODO: We normally have validation to prevent removing a repository that is in use. - // How do we want to handle that for internal repositories? Repository repository = internalRepositories.remove(name); RepositoryMetaData metadata = repository.getMetadata(); if (repository != null) { @@ -410,7 +409,7 @@ private boolean registerRepository(RepositoryMetaData repositoryMetaData) throws return false; } } - Repository newRepo = createRepository(repositoryMetaData); + Repository newRepo = createRepository(repositoryMetaData, typesRegistry); if (previous != null) { closeRepository(previous); } @@ -427,11 +426,29 @@ private void closeRepository(Repository repository) { } /** - * Creates repository holder + * Creates internal repository holder. This method does not start the repository. + */ + private Repository createInternalRepository(RepositoryMetaData repositoryMetaData) { + logger.debug("creating internal repository [{}][{}]", repositoryMetaData.type(), repositoryMetaData.name()); + Repository.Factory factory = internalTypesRegistry.get(repositoryMetaData.type()); + if (factory == null) { + throw new RepositoryException(repositoryMetaData.name(), + "internal repository type [" + repositoryMetaData.type() + "] does not exist"); + } + try { + return factory.create(repositoryMetaData, internalTypesRegistry::get); + } catch (Exception e) { + logger.warn(new ParameterizedMessage("failed to internal create repository [{}][{}]", repositoryMetaData.type(), repositoryMetaData.name()), e); + throw new RepositoryException(repositoryMetaData.name(), "failed to internal create repository", e); + } + } + + /** + * Creates repository holder. This method starts the repository */ - private Repository createRepository(RepositoryMetaData repositoryMetaData) { + private Repository createRepository(RepositoryMetaData repositoryMetaData, Map factories) { logger.debug("creating repository [{}][{}]", repositoryMetaData.type(), repositoryMetaData.name()); - Repository.Factory factory = typesRegistry.get(repositoryMetaData.type()); + Repository.Factory factory = factories.get(repositoryMetaData.type()); if (factory == null) { throw new RepositoryException(repositoryMetaData.name(), "repository type [" + repositoryMetaData.type() + "] does not exist"); @@ -441,7 +458,7 @@ private Repository createRepository(RepositoryMetaData repositoryMetaData) { repository.start(); return repository; } catch (Exception e) { - logger.warn(() -> new ParameterizedMessage("failed to create repository [{}][{}]", repositoryMetaData.type(), repositoryMetaData.name()), e); + logger.warn(new ParameterizedMessage("failed to create repository [{}][{}]", repositoryMetaData.type(), repositoryMetaData.name()), e); throw new RepositoryException(repositoryMetaData.name(), "failed to create repository", e); } } diff --git a/server/src/test/java/org/elasticsearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java b/server/src/test/java/org/elasticsearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java index 4625aa04be372..89a6bc8dcf89d 100644 --- a/server/src/test/java/org/elasticsearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java +++ b/server/src/test/java/org/elasticsearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java @@ -461,7 +461,7 @@ private IndicesClusterStateService createIndicesClusterStateService(DiscoveryNod Collections.emptySet()); final ClusterService clusterService = mock(ClusterService.class); final RepositoriesService repositoriesService = new RepositoriesService(settings, clusterService, - transportService, null, threadPool); + transportService, Collections.emptyMap(), Collections.emptyMap(), threadPool); final PeerRecoveryTargetService recoveryTargetService = new PeerRecoveryTargetService(threadPool, transportService, null, clusterService); final ShardStateAction shardStateAction = mock(ShardStateAction.class); diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java index 92c09624bdfc1..0225d15784da7 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java @@ -272,7 +272,7 @@ public List> getExecutorBuilders(Settings settings) { } @Override - public Map getRepositories(Environment env, NamedXContentRegistry namedXContentRegistry) { + public Map getInternalRepositories(Environment env, NamedXContentRegistry namedXContentRegistry) { Repository.Factory repositoryFactory = (metadata) -> new CcrRepository(metadata, settings); return Collections.singletonMap(CcrRepository.TYPE, repositoryFactory); } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java index 5f0815f246bd9..379bc6e2a67f3 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java @@ -8,7 +8,9 @@ import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteInternalRepositoryAction; +import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteInternalRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.put.PutInternalRepositoryAction; +import org.elasticsearch.action.admin.cluster.repositories.put.PutInternalRepositoryRequest; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.node.NodeClient; @@ -42,15 +44,14 @@ protected Set getRemoteClusterNames() { protected void updateRemoteCluster(String clusterAlias, List addresses, String proxyAddress) { if (addresses.isEmpty()) { if (clusters.remove(clusterAlias)) { - DeleteInternalRepositoryAction.DeleteInternalRepositoryRequest request = - new DeleteInternalRepositoryAction.DeleteInternalRepositoryRequest(clusterAlias); + DeleteInternalRepositoryRequest request = new DeleteInternalRepositoryRequest(clusterAlias); PlainActionFuture future = PlainActionFuture.newFuture(); client.executeLocally(DeleteInternalRepositoryAction.INSTANCE, request, future); assert future.isDone() : "Should be completed as it is executed synchronously"; } } else { if (clusters.add(clusterAlias)) { - ActionRequest request = new PutInternalRepositoryAction.PutInternalRepositoryRequest(clusterAlias, CcrRepository.TYPE); + ActionRequest request = new PutInternalRepositoryRequest(clusterAlias, CcrRepository.TYPE); PlainActionFuture future = PlainActionFuture.newFuture(); client.executeLocally(PutInternalRepositoryAction.INSTANCE, request, future); assert future.isDone() : "Should be completed as it is executed synchronously"; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java index 369ee5e2be016..d0686bd03d9e1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java @@ -402,6 +402,14 @@ public Map getRepositories(Environment env, NamedXCo return repositories; } + @Override + public Map getInternalRepositories(Environment env, NamedXContentRegistry namedXContentRegistry) { + HashMap internalRepositories = new HashMap<>(super.getInternalRepositories(env, namedXContentRegistry)); + filterPlugins(RepositoryPlugin.class).forEach(r -> + internalRepositories.putAll(r.getInternalRepositories(env, namedXContentRegistry))); + return internalRepositories; + } + @Override public void close() throws IOException { IOUtils.close(plugins); From 8bf1a1dbfc2ac4860825a065fbd3a7ca9bafe002 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Thu, 29 Nov 2018 14:40:18 -0700 Subject: [PATCH 10/20] WIP --- .../DeleteInternalRepositoryRequest.java | 4 ++-- .../put/PutInternalRepositoryAction.java | 1 - .../put/PutInternalRepositoryRequest.java | 4 ++-- .../repositories/RepositoriesService.java | 20 +------------------ 4 files changed, 5 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryRequest.java index 2567c0fb637e5..b5e0aaec37c40 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryRequest.java @@ -48,12 +48,12 @@ public ActionRequestValidationException validate() { @Override public void readFrom(StreamInput in) throws IOException { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("DeleteInternalRepositoryRequest cannot be serialized for sending across the wire."); } @Override public void writeTo(StreamOutput out) throws IOException { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("DeleteInternalRepositoryRequest cannot be serialized for sending across the wire."); } public String getName() { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java index 7e746b16500db..d2bfe3ed9d443 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java @@ -52,5 +52,4 @@ public Writeable.Reader getResponseReader() { return acknowledgedResponse; }; } - } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryRequest.java index d7ac1aa96034c..bdc1f2b88b2d3 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryRequest.java @@ -60,12 +60,12 @@ public ActionRequestValidationException validate() { @Override public void readFrom(StreamInput in) throws IOException { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("PutInternalRepositoryRequest cannot be serialized for sending across the wire."); } @Override public void writeTo(StreamOutput out) throws IOException { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("PutInternalRepositoryRequest cannot be serialized for sending across the wire."); } public String getName() { diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index fac2e58b7e809..07aa88ccb6ec7 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -425,24 +425,6 @@ private void closeRepository(Repository repository) { repository.close(); } - /** - * Creates internal repository holder. This method does not start the repository. - */ - private Repository createInternalRepository(RepositoryMetaData repositoryMetaData) { - logger.debug("creating internal repository [{}][{}]", repositoryMetaData.type(), repositoryMetaData.name()); - Repository.Factory factory = internalTypesRegistry.get(repositoryMetaData.type()); - if (factory == null) { - throw new RepositoryException(repositoryMetaData.name(), - "internal repository type [" + repositoryMetaData.type() + "] does not exist"); - } - try { - return factory.create(repositoryMetaData, internalTypesRegistry::get); - } catch (Exception e) { - logger.warn(new ParameterizedMessage("failed to internal create repository [{}][{}]", repositoryMetaData.type(), repositoryMetaData.name()), e); - throw new RepositoryException(repositoryMetaData.name(), "failed to internal create repository", e); - } - } - /** * Creates repository holder. This method starts the repository */ @@ -454,7 +436,7 @@ private Repository createRepository(RepositoryMetaData repositoryMetaData, Map Date: Thu, 29 Nov 2018 15:11:27 -0700 Subject: [PATCH 11/20] comment --- .../org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java index e6ac60892d017..1a4ce8a34a4ea 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java @@ -17,6 +17,8 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +// TODO: Fold this integration test into a more expansive integration test as more bootstrap from remote work +// TODO: is completed. public class CcrRepositoryManagerIT extends CcrIntegTestCase { public void testThatRepositoryIsPutAndRemovedWhenRemoteClusterIsUpdated() throws Exception { From 9e4736edfbe189b6961037babba086019a41526a Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Thu, 29 Nov 2018 15:41:22 -0700 Subject: [PATCH 12/20] Fix checkstyle --- .../delete/DeleteInternalRepositoryAction.java | 18 +----------------- .../put/PutInternalRepositoryAction.java | 7 ------- .../java/org/elasticsearch/xpack/ccr/Ccr.java | 1 - 3 files changed, 1 insertion(+), 25 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java index d110b393d368d..b621e44a23b05 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java @@ -20,31 +20,15 @@ package org.elasticsearch.action.admin.cluster.repositories.delete; import org.elasticsearch.action.Action; -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.ActionRequest; -import org.elasticsearch.action.ActionRequestValidationException; -import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.repositories.RepositoriesService; -import org.elasticsearch.tasks.Task; -import org.elasticsearch.transport.TransportService; - -import java.io.IOException; -import java.util.Objects; - -import static org.elasticsearch.action.ValidateActions.addValidationError; public class DeleteInternalRepositoryAction extends Action { public static final DeleteInternalRepositoryAction INSTANCE = new DeleteInternalRepositoryAction(); public static final String NAME = "cluster:admin/internal_repository/delete"; - protected DeleteInternalRepositoryAction() { + private DeleteInternalRepositoryAction() { super(NAME); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java index d2bfe3ed9d443..4298fc9d2f402 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java @@ -20,15 +20,8 @@ package org.elasticsearch.action.admin.cluster.repositories.put; import org.elasticsearch.action.Action; -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.repositories.RepositoriesService; -import org.elasticsearch.tasks.Task; -import org.elasticsearch.transport.TransportService; public class PutInternalRepositoryAction extends Action { diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java index 0225d15784da7..903a97ab30a0b 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java @@ -35,7 +35,6 @@ import org.elasticsearch.plugins.PersistentTaskPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.RepositoryPlugin; -import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.repositories.Repository; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; From 8f11a775cb2b03a2200803b5ad1a45cb93a2a9d1 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 30 Nov 2018 14:36:56 -0700 Subject: [PATCH 13/20] Changes from review --- .../elasticsearch/action/ActionModule.java | 8 +--- ...ansportDeleteInternalRepositoryAction.java | 47 ------------------- .../TransportPutInternalRepositoryAction.java | 47 ------------------- .../put/TransportPutRepositoryAction.java | 1 + .../repositories/RepositoriesService.java | 13 +++-- .../transport/RemoteClusterAware.java | 10 ++-- .../transport/RemoteClusterService.java | 5 +- .../transport/RemoteClusterServiceTests.java | 9 ++-- .../java/org/elasticsearch/xpack/ccr/Ccr.java | 9 ++-- .../xpack/ccr/CcrRepositoryManager.java | 37 +++++---------- .../DeleteInternalRepositoryAction.java | 27 ++++++++++- .../DeleteInternalRepositoryRequest.java | 14 ++---- .../PutInternalRepositoryAction.java | 28 ++++++++++- .../PutInternalRepositoryRequest.java | 21 +++------ .../authz/IndicesAndAliasesResolver.java | 9 +--- 15 files changed, 103 insertions(+), 182 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteInternalRepositoryAction.java delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutInternalRepositoryAction.java rename {server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete => x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories}/DeleteInternalRepositoryAction.java (56%) rename {server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete => x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories}/DeleteInternalRepositoryRequest.java (84%) rename {server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put => x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories}/PutInternalRepositoryAction.java (55%) rename {server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put => x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories}/PutInternalRepositoryRequest.java (83%) diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 0a86cc368b27b..f442748c15ab1 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -19,8 +19,8 @@ package org.elasticsearch.action; -import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.elasticsearch.action.admin.cluster.allocation.ClusterAllocationExplainAction; import org.elasticsearch.action.admin.cluster.allocation.TransportClusterAllocationExplainAction; import org.elasticsearch.action.admin.cluster.health.ClusterHealthAction; @@ -44,15 +44,11 @@ import org.elasticsearch.action.admin.cluster.node.usage.TransportNodesUsageAction; import org.elasticsearch.action.admin.cluster.remote.RemoteInfoAction; import org.elasticsearch.action.admin.cluster.remote.TransportRemoteInfoAction; -import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteInternalRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryAction; -import org.elasticsearch.action.admin.cluster.repositories.delete.TransportDeleteInternalRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.delete.TransportDeleteRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesAction; import org.elasticsearch.action.admin.cluster.repositories.get.TransportGetRepositoriesAction; -import org.elasticsearch.action.admin.cluster.repositories.put.PutInternalRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryAction; -import org.elasticsearch.action.admin.cluster.repositories.put.TransportPutInternalRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.put.TransportPutRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.verify.TransportVerifyRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.verify.VerifyRepositoryAction; @@ -439,8 +435,6 @@ public void reg actions.register(GetRepositoriesAction.INSTANCE, TransportGetRepositoriesAction.class); actions.register(DeleteRepositoryAction.INSTANCE, TransportDeleteRepositoryAction.class); actions.register(VerifyRepositoryAction.INSTANCE, TransportVerifyRepositoryAction.class); - actions.register(PutInternalRepositoryAction.INSTANCE, TransportPutInternalRepositoryAction.class); - actions.register(DeleteInternalRepositoryAction.INSTANCE, TransportDeleteInternalRepositoryAction.class); actions.register(GetSnapshotsAction.INSTANCE, TransportGetSnapshotsAction.class); actions.register(DeleteSnapshotAction.INSTANCE, TransportDeleteSnapshotAction.class); actions.register(CreateSnapshotAction.INSTANCE, TransportCreateSnapshotAction.class); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteInternalRepositoryAction.java deleted file mode 100644 index 1190cd94491d6..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteInternalRepositoryAction.java +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.action.admin.cluster.repositories.delete; - -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.TransportAction; -import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.repositories.RepositoriesService; -import org.elasticsearch.tasks.Task; -import org.elasticsearch.transport.TransportService; - -public class TransportDeleteInternalRepositoryAction extends TransportAction { - - private final RepositoriesService repositoriesService; - - @Inject - public TransportDeleteInternalRepositoryAction(RepositoriesService repositoriesService, ActionFilters actionFilters, - TransportService transportService) { - super(DeleteInternalRepositoryAction.NAME, actionFilters, transportService.getTaskManager()); - this.repositoriesService = repositoriesService; - } - - @Override - protected void doExecute(Task task, DeleteInternalRepositoryRequest request, ActionListener listener) { - repositoriesService.unregisterInternalRepository(request.getName()); - listener.onResponse(new AcknowledgedResponse(true)); - } -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutInternalRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutInternalRepositoryAction.java deleted file mode 100644 index 2db5db99259f5..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutInternalRepositoryAction.java +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.action.admin.cluster.repositories.put; - -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.TransportAction; -import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.repositories.RepositoriesService; -import org.elasticsearch.tasks.Task; -import org.elasticsearch.transport.TransportService; - -public class TransportPutInternalRepositoryAction extends TransportAction { - - private final RepositoriesService repositoriesService; - - @Inject - public TransportPutInternalRepositoryAction(RepositoriesService repositoriesService, ActionFilters actionFilters, - TransportService transportService) { - super(PutInternalRepositoryAction.NAME, actionFilters, transportService.getTaskManager()); - this.repositoriesService = repositoriesService; - } - - @Override - protected void doExecute(Task task, PutInternalRepositoryRequest request, ActionListener listener) { - repositoriesService.registerInternalRepository(request.getName(), request.getType(), request.getSettings()); - listener.onResponse(new AcknowledgedResponse(true)); - } -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutRepositoryAction.java index c298afa8bdcb7..a495ba72f35b7 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutRepositoryAction.java @@ -87,4 +87,5 @@ public void onFailure(Exception e) { } }); } + } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index 07aa88ccb6ec7..6f730fbe8ab92 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -367,12 +367,12 @@ public void registerInternalRepository(String name, String type, Settings settin Repository existingRepository = internalRepositories.putIfAbsent(name, repository); if (existingRepository != null) { - logger.error(new ParameterizedMessage("Error registering internal repository [{}][{}]. " + - "Internal repository with that name already registered.", metaData.type(), name)); + logger.error(new ParameterizedMessage("error registering internal repository [{}][{}]. " + + "internal repository with that name already registered.", metaData.type(), name)); repository.close(); } if (repositories.containsKey(name)) { - logger.warn(new ParameterizedMessage("Non-internal repository [{}] already registered. This repository will block the " + + logger.warn(new ParameterizedMessage("non-internal repository [{}] already registered. this repository will block the " + "usage of internal repository [{}][{}].", name, metaData.type(), name)); } } @@ -381,12 +381,11 @@ public void unregisterInternalRepository(String name) { Repository repository = internalRepositories.remove(name); RepositoryMetaData metadata = repository.getMetadata(); if (repository != null) { - logger.debug(() -> new ParameterizedMessage("Unregistering internal repository [{}][{}].", metadata.type(), name)); + logger.debug(() -> new ParameterizedMessage("unregistering internal repository [{}][{}].", metadata.type(), name)); closeRepository(repository); - repository.close(); } else { - logger.warn(() -> new ParameterizedMessage("Attempted to unregistered internal repository [{}][{}]. " + - "Repository could not found.", metadata.type(), name)); + logger.warn(() -> new ParameterizedMessage("attempted to unregister internal repository [{}][{}]. " + + "repository could not found.", metadata.type(), name)); } } diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java index 8da433fdc6c8e..2c36af8638f63 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java @@ -167,7 +167,7 @@ public String getKey(final String key) { REMOTE_CLUSTERS_SEEDS); protected final Settings settings; - protected final ClusterNameExpressionResolver clusterNameResolver; + private final ClusterNameExpressionResolver clusterNameResolver; /** * Creates a new {@link RemoteClusterAware} instance @@ -242,14 +242,15 @@ static DiscoveryNode buildSeedNode(String clusterName, String address, boolean p * indices per cluster are collected as a list in the returned map keyed by the cluster alias. Local indices are grouped under * {@link #LOCAL_CLUSTER_GROUP_KEY}. The returned map is mutable. * + * @param remoteClusterNames the remote cluster names * @param requestIndices the indices in the search request to filter * @param indexExists a predicate that can test if a certain index or alias exists in the local cluster * * @return a map of grouped remote and local indices */ - public Map> groupClusterIndices(String[] requestIndices, Predicate indexExists) { + protected Map> groupClusterIndices(Set remoteClusterNames, String[] requestIndices, + Predicate indexExists) { Map> perClusterIndices = new HashMap<>(); - Set remoteClusterNames = getRemoteClusterNames(); for (String index : requestIndices) { int i = index.indexOf(RemoteClusterService.REMOTE_CLUSTER_INDEX_SEPARATOR); if (i >= 0) { @@ -281,9 +282,6 @@ public Map> groupClusterIndices(String[] requestIndices, Pr return perClusterIndices; } - protected abstract Set getRemoteClusterNames(); - - /** * Subclasses must implement this to receive information about updated cluster aliases. If the given address list is * empty the cluster alias is unregistered and should be removed. diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java index 0028a2537a266..1997be3dc1be3 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java @@ -278,7 +278,7 @@ boolean isRemoteNodeConnected(final String remoteCluster, final DiscoveryNode no public Map groupIndices(IndicesOptions indicesOptions, String[] indices, Predicate indexExists) { Map originalIndicesMap = new HashMap<>(); if (isCrossClusterSearchEnabled()) { - final Map> groupedIndices = groupClusterIndices(indices, indexExists); + final Map> groupedIndices = groupClusterIndices(remoteClusters.keySet(), indices, indexExists); if (groupedIndices.isEmpty()) { //search on _all in the local cluster if neither local indices nor remote indices were specified originalIndicesMap.put(LOCAL_CLUSTER_GROUP_KEY, new OriginalIndices(Strings.EMPTY_ARRAY, indicesOptions)); @@ -380,8 +380,7 @@ RemoteClusterConnection getRemoteClusterConnection(String cluster) { return connection; } - @Override - protected Set getRemoteClusterNames() { + Set getRemoteClusterNames() { return this.remoteClusters.keySet(); } diff --git a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java index 5bc05507b9d9f..9a185163436af 100644 --- a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java +++ b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java @@ -219,8 +219,9 @@ public void testGroupClusterIndices() throws IOException { assertTrue(service.isRemoteClusterRegistered("cluster_1")); assertTrue(service.isRemoteClusterRegistered("cluster_2")); assertFalse(service.isRemoteClusterRegistered("foo")); - Map> perClusterIndices = service.groupClusterIndices(new String[]{"foo:bar", "cluster_1:bar", - "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo", "cluster*:baz", "*:boo", "no*match:boo"}, + Map> perClusterIndices = service.groupClusterIndices(service.getRemoteClusterNames(), + new String[]{"foo:bar", "cluster_1:bar", "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo", + "cluster*:baz", "*:boo", "no*match:boo"}, i -> false); List localIndices = perClusterIndices.remove(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY); assertNotNull(localIndices); @@ -230,7 +231,7 @@ public void testGroupClusterIndices() throws IOException { assertEquals(Arrays.asList("foo:bar", "foo*", "baz", "boo"), perClusterIndices.get("cluster_2")); IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> - service.groupClusterIndices(new String[]{"foo:bar", "cluster_1:bar", + service.groupClusterIndices(service.getRemoteClusterNames(), new String[]{"foo:bar", "cluster_1:bar", "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo"}, "cluster_1:bar"::equals)); assertEquals("Can not filter indices; index cluster_1:bar exists but there is also a remote cluster named:" + @@ -277,7 +278,7 @@ public void testGroupIndices() throws IOException { } { IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> - service.groupClusterIndices(new String[]{"foo:bar", "cluster_1:bar", + service.groupClusterIndices(service.getRemoteClusterNames(), new String[]{"foo:bar", "cluster_1:bar", "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo"}, "cluster_1:bar"::equals)); assertEquals("Can not filter indices; index cluster_1:bar exists but there is also a remote cluster named:" + " cluster_1", iae.getMessage()); diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java index 903a97ab30a0b..343d12b649b53 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java @@ -59,6 +59,8 @@ import org.elasticsearch.xpack.ccr.action.TransportUnfollowAction; import org.elasticsearch.xpack.ccr.action.bulk.BulkShardOperationsAction; import org.elasticsearch.xpack.ccr.action.bulk.TransportBulkShardOperationsAction; +import org.elasticsearch.xpack.ccr.action.repositories.DeleteInternalRepositoryAction; +import org.elasticsearch.xpack.ccr.action.repositories.PutInternalRepositoryAction; import org.elasticsearch.xpack.ccr.index.engine.FollowingEngineFactory; import org.elasticsearch.xpack.ccr.repository.CcrRepository; import org.elasticsearch.xpack.ccr.rest.RestCcrStatsAction; @@ -110,8 +112,7 @@ public class Ccr extends Plugin implements ActionPlugin, PersistentTaskPlugin, E private final boolean enabled; private final Settings settings; private final CcrLicenseChecker ccrLicenseChecker; - private SetOnce clusterService = new SetOnce<>(); - private SetOnce repositoryManager = new SetOnce<>(); + private final SetOnce repositoryManager = new SetOnce<>(); /** * Construct an instance of the CCR container with the specified settings. @@ -150,7 +151,6 @@ public Collection createComponents( return emptyList(); } - this.clusterService.set(clusterService); this.repositoryManager.set(new CcrRepositoryManager(settings, clusterService, (NodeClient) client)); return Arrays.asList( @@ -177,6 +177,9 @@ public List> getPersistentTasksExecutor(ClusterServic // internal actions new ActionHandler<>(BulkShardOperationsAction.INSTANCE, TransportBulkShardOperationsAction.class), new ActionHandler<>(ShardChangesAction.INSTANCE, ShardChangesAction.TransportAction.class), + new ActionHandler<>(PutInternalRepositoryAction.INSTANCE, + PutInternalRepositoryAction.TransportPutInternalRepositoryAction.class), + new ActionHandler<>(DeleteInternalRepositoryAction.INSTANCE, DeleteInternalRepositoryAction.TransportDeleteInternalRepositoryAction.class), // stats action new ActionHandler<>(FollowStatsAction.INSTANCE, TransportFollowStatsAction.class), new ActionHandler<>(CcrStatsAction.INSTANCE, TransportCcrStatsAction.class), diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java index 379bc6e2a67f3..4426413fb9c8f 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java @@ -7,55 +7,42 @@ package org.elasticsearch.xpack.ccr; import org.elasticsearch.action.ActionRequest; -import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteInternalRepositoryAction; -import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteInternalRepositoryRequest; -import org.elasticsearch.action.admin.cluster.repositories.put.PutInternalRepositoryAction; -import org.elasticsearch.action.admin.cluster.repositories.put.PutInternalRepositoryRequest; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.transport.RemoteClusterAware; +import org.elasticsearch.xpack.ccr.action.repositories.DeleteInternalRepositoryAction; +import org.elasticsearch.xpack.ccr.action.repositories.DeleteInternalRepositoryRequest; +import org.elasticsearch.xpack.ccr.action.repositories.PutInternalRepositoryAction; +import org.elasticsearch.xpack.ccr.action.repositories.PutInternalRepositoryRequest; import org.elasticsearch.xpack.ccr.repository.CcrRepository; import java.util.List; -import java.util.Set; class CcrRepositoryManager extends RemoteClusterAware { private final NodeClient client; - private final Set clusters = ConcurrentCollections.newConcurrentSet(); CcrRepositoryManager(Settings settings, ClusterService clusterService, NodeClient client) { super(settings); this.client = client; - clusters.addAll(buildRemoteClustersDynamicConfig(settings).keySet()); listenForUpdates(clusterService.getClusterSettings()); } - @Override - protected Set getRemoteClusterNames() { - return clusters; - } - @Override protected void updateRemoteCluster(String clusterAlias, List addresses, String proxyAddress) { if (addresses.isEmpty()) { - if (clusters.remove(clusterAlias)) { - DeleteInternalRepositoryRequest request = new DeleteInternalRepositoryRequest(clusterAlias); - PlainActionFuture future = PlainActionFuture.newFuture(); - client.executeLocally(DeleteInternalRepositoryAction.INSTANCE, request, future); - assert future.isDone() : "Should be completed as it is executed synchronously"; - } + DeleteInternalRepositoryRequest request = new DeleteInternalRepositoryRequest(clusterAlias); + PlainActionFuture future = PlainActionFuture.newFuture(); + client.executeLocally(DeleteInternalRepositoryAction.INSTANCE, request, future); + assert future.isDone() : "Should be completed as it is executed synchronously"; } else { - if (clusters.add(clusterAlias)) { - ActionRequest request = new PutInternalRepositoryRequest(clusterAlias, CcrRepository.TYPE); - PlainActionFuture future = PlainActionFuture.newFuture(); - client.executeLocally(PutInternalRepositoryAction.INSTANCE, request, future); - assert future.isDone() : "Should be completed as it is executed synchronously"; - } + ActionRequest request = new PutInternalRepositoryRequest(clusterAlias, CcrRepository.TYPE); + PlainActionFuture future = PlainActionFuture.newFuture(); + client.executeLocally(PutInternalRepositoryAction.INSTANCE, request, future); + assert future.isDone() : "Should be completed as it is executed synchronously"; } } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java similarity index 56% rename from server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java rename to x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java index b621e44a23b05..cb9eff50a8c5f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java @@ -17,11 +17,19 @@ * under the License. */ -package org.elasticsearch.action.admin.cluster.repositories.delete; +package org.elasticsearch.xpack.ccr.action.repositories; import org.elasticsearch.action.Action; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.transport.TransportService; public class DeleteInternalRepositoryAction extends Action { @@ -46,4 +54,21 @@ public Writeable.Reader getResponseReader() { }; } + public static class TransportDeleteInternalRepositoryAction extends TransportAction { + + private final RepositoriesService repositoriesService; + + @Inject + public TransportDeleteInternalRepositoryAction(RepositoriesService repositoriesService, ActionFilters actionFilters, + TransportService transportService) { + super(NAME, actionFilters, transportService.getTaskManager()); + this.repositoriesService = repositoriesService; + } + + @Override + protected void doExecute(Task task, DeleteInternalRepositoryRequest request, ActionListener listener) { + repositoriesService.unregisterInternalRepository(request.getName()); + listener.onResponse(new ActionResponse() {}); + } + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryRequest.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryRequest.java similarity index 84% rename from server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryRequest.java rename to x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryRequest.java index b5e0aaec37c40..628ea542c11f9 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteInternalRepositoryRequest.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryRequest.java @@ -17,7 +17,7 @@ * under the License. */ -package org.elasticsearch.action.admin.cluster.repositories.delete; +package org.elasticsearch.xpack.ccr.action.repositories; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; @@ -27,23 +27,17 @@ import java.io.IOException; import java.util.Objects; -import static org.elasticsearch.action.ValidateActions.addValidationError; - public class DeleteInternalRepositoryRequest extends ActionRequest { - private String name; + private final String name; public DeleteInternalRepositoryRequest(String name) { - this.name = name; + this.name = Objects.requireNonNull(name); } @Override public ActionRequestValidationException validate() { - ActionRequestValidationException validationException = null; - if (name == null) { - validationException = addValidationError("name is missing", validationException); - } - return validationException; + return null; } @Override diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java similarity index 55% rename from server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java rename to x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java index 4298fc9d2f402..aa06a15980d20 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java @@ -17,11 +17,19 @@ * under the License. */ -package org.elasticsearch.action.admin.cluster.repositories.put; +package org.elasticsearch.xpack.ccr.action.repositories; import org.elasticsearch.action.Action; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.transport.TransportService; public class PutInternalRepositoryAction extends Action { @@ -45,4 +53,22 @@ public Writeable.Reader getResponseReader() { return acknowledgedResponse; }; } + + public static class TransportPutInternalRepositoryAction extends TransportAction { + + private final RepositoriesService repositoriesService; + + @Inject + public TransportPutInternalRepositoryAction(RepositoriesService repositoriesService, ActionFilters actionFilters, + TransportService transportService) { + super(NAME, actionFilters, transportService.getTaskManager()); + this.repositoriesService = repositoriesService; + } + + @Override + protected void doExecute(Task task, PutInternalRepositoryRequest request, ActionListener listener) { + repositoriesService.registerInternalRepository(request.getName(), request.getType(), request.getSettings()); + listener.onResponse(new ActionResponse() {}); + } + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryRequest.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java similarity index 83% rename from server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryRequest.java rename to x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java index bdc1f2b88b2d3..0612105abf70a 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutInternalRepositoryRequest.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java @@ -17,7 +17,7 @@ * under the License. */ -package org.elasticsearch.action.admin.cluster.repositories.put; +package org.elasticsearch.xpack.ccr.action.repositories; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; @@ -32,30 +32,23 @@ public class PutInternalRepositoryRequest extends ActionRequest { - private String name; - private String type; - private Settings settings; + private final String name; + private final String type; + private final Settings settings; public PutInternalRepositoryRequest(String name, String type) { this(name, type, Settings.EMPTY); } public PutInternalRepositoryRequest(String name, String type, Settings settings) { - this.name = name; - this.type = type; + this.name = Objects.requireNonNull(name); + this.type = Objects.requireNonNull(type); this.settings = settings; } @Override public ActionRequestValidationException validate() { - ActionRequestValidationException validationException = null; - if (name == null) { - validationException = addValidationError("name is missing", validationException); - } - if (type == null) { - validationException = addValidationError("type is missing", validationException); - } - return validationException; + return null; } @Override diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java index cbd0d7ca184cc..aa1461b189a39 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java @@ -70,7 +70,7 @@ class IndicesAndAliasesResolver { * then the index names will be categorized into those that refer to {@link ResolvedIndices#getLocal() local indices}, and those that * refer to {@link ResolvedIndices#getRemote() remote indices}. This categorization follows the standard * {@link RemoteClusterAware#buildRemoteIndexName(String, String) remote index-name format} and also respects the currently defined - * {@link RemoteClusterAware#getRemoteClusterNames() remote clusters}. + * remote clusters}. *


* Thus an index name N will considered to be remote if-and-only-if all of the following are true *
    @@ -438,11 +438,6 @@ private RemoteClusterResolver(Settings settings, ClusterSettings clusterSettings listenForUpdates(clusterSettings); } - @Override - protected Set getRemoteClusterNames() { - return clusters; - } - @Override protected void updateRemoteCluster(String clusterAlias, List addresses, String proxyAddress) { if (addresses.isEmpty()) { @@ -453,7 +448,7 @@ protected void updateRemoteCluster(String clusterAlias, List addresses, } ResolvedIndices splitLocalAndRemoteIndexNames(String... indices) { - final Map> map = super.groupClusterIndices(indices, exists -> false); + final Map> map = super.groupClusterIndices(clusters, indices, exists -> false); final List local = map.remove(LOCAL_CLUSTER_GROUP_KEY); final List remote = map.entrySet().stream() .flatMap(e -> e.getValue().stream().map(v -> e.getKey() + REMOTE_CLUSTER_INDEX_SEPARATOR + v)) From 4d58c11b65191f508d89f7c693d2f8dde37e42a0 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 30 Nov 2018 16:02:50 -0700 Subject: [PATCH 14/20] Changes from review --- .../repositories/RepositoriesService.java | 62 ++++- .../RepositoriesServiceTests.java | 247 ++++++++++++++++++ .../java/org/elasticsearch/xpack/ccr/Ccr.java | 3 +- .../xpack/ccr/CcrRepositoryManager.java | 6 +- .../DeleteInternalRepositoryAction.java | 13 +- .../PutInternalRepositoryAction.java | 13 +- .../PutInternalRepositoryRequest.java | 2 - 7 files changed, 308 insertions(+), 38 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index 6f730fbe8ab92..c0e51c2fd6cb7 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -67,8 +67,8 @@ public class RepositoriesService implements ClusterStateApplier { private final VerifyNodeRepositoryAction verifyAction; + private final Map internalRepositories = ConcurrentCollections.newConcurrentMap(); private volatile Map repositories = Collections.emptyMap(); - private volatile Map internalRepositories = ConcurrentCollections.newConcurrentMap(); public RepositoriesService(Settings settings, ClusterService clusterService, TransportService transportService, Map typesRegistry, Map internalTypesRegistry, @@ -362,30 +362,64 @@ public Repository repository(String repositoryName) { public void registerInternalRepository(String name, String type, Settings settings) { RepositoryMetaData metaData = new RepositoryMetaData(name, type, settings); - Repository repository = createRepository(metaData, internalTypesRegistry); + Repository existingRepository = internalRepositories.get(name); + if (existingRepository != null && existingRepository.getMetadata().equals(metaData)) { + // Identical repository already exists, no need to update it. + return; + } - Repository existingRepository = internalRepositories.putIfAbsent(name, repository); + // TODO: Normally we would do validation when we update a repository to ensure that it is not in use. + // Are we okay with not including that validation under the assumption that internal operations + // will do the right thing. + + Repository newRepository = createRepository(metaData, internalTypesRegistry); + Repository repositoryToClose = null; + boolean updated = false; + synchronized (internalRepositories) { + existingRepository = internalRepositories.get(name); + if (existingRepository != null) { + if (existingRepository.getMetadata().equals(metaData)) { + // Identical repository already exists, no need to update it. + repositoryToClose = newRepository; + } else { + logger.info("update internal repository [{}][{}]", name, type); + internalRepositories.put(name, newRepository); + updated = true; + repositoryToClose = existingRepository; + } + } else { + logger.info("put internal repository [{}][{}]", name, type); + internalRepositories.put(name, newRepository); + updated = true; + } + } - if (existingRepository != null) { - logger.error(new ParameterizedMessage("error registering internal repository [{}][{}]. " + - "internal repository with that name already registered.", metaData.type(), name)); - repository.close(); + if (repositoryToClose != null) { + repositoryToClose.close(); } - if (repositories.containsKey(name)) { + + if (updated && repositories.containsKey(name)) { logger.warn(new ParameterizedMessage("non-internal repository [{}] already registered. this repository will block the " + "usage of internal repository [{}][{}].", name, metaData.type(), name)); } } public void unregisterInternalRepository(String name) { - Repository repository = internalRepositories.remove(name); - RepositoryMetaData metadata = repository.getMetadata(); + Repository repository = internalRepositories.get(name); + if (repository == null) { + // Repository does not exist to delete + return; + } + + synchronized (internalRepositories) { + // Even though we are using a concurrent hash map, the remove must be synchronized as it can + // conflict with the put in #registerInternalRepository which is a multi-step operation + repository = internalRepositories.remove(name); + } if (repository != null) { - logger.debug(() -> new ParameterizedMessage("unregistering internal repository [{}][{}].", metadata.type(), name)); + RepositoryMetaData metadata = repository.getMetadata(); + logger.debug(() -> new ParameterizedMessage("delete internal repository [{}][{}].", metadata.type(), name)); closeRepository(repository); - } else { - logger.warn(() -> new ParameterizedMessage("attempted to unregister internal repository [{}][{}]. " + - "repository could not found.", metadata.type(), name)); } } diff --git a/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java b/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java new file mode 100644 index 0000000000000..f8ba5127f98a4 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java @@ -0,0 +1,247 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.repositories; + +import org.apache.lucene.index.IndexCommit; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.metadata.RepositoryMetaData; +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.UUIDs; +import org.elasticsearch.common.component.Lifecycle; +import org.elasticsearch.common.component.LifecycleListener; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.shard.IndexShard; +import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.index.snapshots.IndexShardSnapshotStatus; +import org.elasticsearch.index.store.Store; +import org.elasticsearch.indices.recovery.RecoveryState; +import org.elasticsearch.snapshots.SnapshotId; +import org.elasticsearch.snapshots.SnapshotInfo; +import org.elasticsearch.snapshots.SnapshotShardFailure; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; +import org.elasticsearch.transport.TransportService; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +import static org.mockito.Mockito.mock; + +public class RepositoriesServiceTests extends ESTestCase { + + private RepositoriesService repositoriesService; + + @Override + public void setUp() throws Exception { + super.setUp(); + ThreadPool threadPool = mock(ThreadPool.class); + final TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), threadPool, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, + boundAddress -> DiscoveryNode.createLocal(Settings.EMPTY, boundAddress.publishAddress(), UUIDs.randomBase64UUID()), null, + Collections.emptySet()); + repositoriesService = new RepositoriesService(Settings.EMPTY, mock(ClusterService.class), + transportService, Collections.emptyMap(), Collections.singletonMap(TestRepository.TYPE, TestRepository::new), threadPool); + } + + public void testRegisterInternalRepository() { + String repoName = "name"; + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(repoName)); + repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE, Settings.EMPTY); + Repository repository = repositoriesService.repository(repoName); + assertEquals(repoName, repository.getMetadata().name()); + assertEquals(TestRepository.TYPE, repository.getMetadata().type()); + assertEquals(Settings.EMPTY, repository.getMetadata().settings()); + assertTrue(((TestRepository) repository).isStarted); + } + + public void testUnregisterInternalRepository() { + String repoName = "name"; + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(repoName)); + repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE, Settings.EMPTY); + Repository repository = repositoriesService.repository(repoName); + assertFalse(((TestRepository) repository).isClosed); + repositoriesService.unregisterInternalRepository(repoName); + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(repoName)); + assertTrue(((TestRepository) repository).isClosed); + } + + public void testRegisterWillNotUpdateIdenticalInternalRepository() { + String repoName = "name"; + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(repoName)); + repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE, Settings.EMPTY); + Repository repository = repositoriesService.repository(repoName); + assertFalse(((TestRepository) repository).isClosed); + repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE, Settings.EMPTY); + assertFalse(((TestRepository) repository).isClosed); + Repository repository2 = repositoriesService.repository(repoName); + assertSame(repository, repository2); + } + + public void testRegisterWillUpdateInternalRepository() { + String repoName = "name"; + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(repoName)); + repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE, Settings.EMPTY); + Repository repository = repositoriesService.repository(repoName); + assertFalse(((TestRepository) repository).isClosed); + Settings newSettings = Settings.builder().put("foo", "bar").build(); + repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE, newSettings); + assertTrue(((TestRepository) repository).isClosed); + Repository repository2 = repositoriesService.repository(repoName); + assertFalse(((TestRepository) repository2).isClosed); + assertEquals(newSettings, repository2.getMetadata().settings()); + } + + private static class TestRepository implements Repository { + + private static final String TYPE = "internal"; + private boolean isClosed; + private boolean isStarted; + + private final RepositoryMetaData metaData; + + private TestRepository(RepositoryMetaData metaData) { + this.metaData = metaData; + } + + @Override + public RepositoryMetaData getMetadata() { + return metaData; + } + + @Override + public SnapshotInfo getSnapshotInfo(SnapshotId snapshotId) { + return null; + } + + @Override + public MetaData getSnapshotGlobalMetaData(SnapshotId snapshotId) { + return null; + } + + @Override + public IndexMetaData getSnapshotIndexMetaData(SnapshotId snapshotId, IndexId index) throws IOException { + return null; + } + + @Override + public RepositoryData getRepositoryData() { + return null; + } + + @Override + public void initializeSnapshot(SnapshotId snapshotId, List indices, MetaData metaData) { + + } + + @Override + public SnapshotInfo finalizeSnapshot(SnapshotId snapshotId, List indices, long startTime, String failure, + int totalShards, List shardFailures, long repositoryStateId, + boolean includeGlobalState) { + return null; + } + + @Override + public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId) { + + } + + @Override + public long getSnapshotThrottleTimeInNanos() { + return 0; + } + + @Override + public long getRestoreThrottleTimeInNanos() { + return 0; + } + + @Override + public String startVerification() { + return null; + } + + @Override + public void endVerification(String verificationToken) { + + } + + @Override + public void verify(String verificationToken, DiscoveryNode localNode) { + + } + + @Override + public boolean isReadOnly() { + return false; + } + + @Override + public void snapshotShard(IndexShard shard, Store store, SnapshotId snapshotId, IndexId indexId, IndexCommit snapshotIndexCommit, + IndexShardSnapshotStatus snapshotStatus) { + + } + + @Override + public void restoreShard(IndexShard shard, SnapshotId snapshotId, Version version, IndexId indexId, ShardId snapshotShardId, + RecoveryState recoveryState) { + + } + + @Override + public IndexShardSnapshotStatus getShardSnapshotStatus(SnapshotId snapshotId, Version version, IndexId indexId, ShardId shardId) { + return null; + } + + @Override + public Lifecycle.State lifecycleState() { + return null; + } + + @Override + public void addLifecycleListener(LifecycleListener listener) { + + } + + @Override + public void removeLifecycleListener(LifecycleListener listener) { + + } + + @Override + public void start() { + isStarted = true; + } + + @Override + public void stop() { + + } + + @Override + public void close() { + isClosed = true; + } + } +} diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java index 343d12b649b53..254fb82de92ad 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java @@ -179,7 +179,8 @@ public List> getPersistentTasksExecutor(ClusterServic new ActionHandler<>(ShardChangesAction.INSTANCE, ShardChangesAction.TransportAction.class), new ActionHandler<>(PutInternalRepositoryAction.INSTANCE, PutInternalRepositoryAction.TransportPutInternalRepositoryAction.class), - new ActionHandler<>(DeleteInternalRepositoryAction.INSTANCE, DeleteInternalRepositoryAction.TransportDeleteInternalRepositoryAction.class), + new ActionHandler<>(DeleteInternalRepositoryAction.INSTANCE, + DeleteInternalRepositoryAction.TransportDeleteInternalRepositoryAction.class), // stats action new ActionHandler<>(FollowStatsAction.INSTANCE, TransportFollowStatsAction.class), new ActionHandler<>(CcrStatsAction.INSTANCE, TransportCcrStatsAction.class), diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java index 4426413fb9c8f..6bb02ecb1122d 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java @@ -7,8 +7,8 @@ package org.elasticsearch.xpack.ccr; import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.support.PlainActionFuture; -import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; @@ -35,12 +35,12 @@ class CcrRepositoryManager extends RemoteClusterAware { protected void updateRemoteCluster(String clusterAlias, List addresses, String proxyAddress) { if (addresses.isEmpty()) { DeleteInternalRepositoryRequest request = new DeleteInternalRepositoryRequest(clusterAlias); - PlainActionFuture future = PlainActionFuture.newFuture(); + PlainActionFuture future = PlainActionFuture.newFuture(); client.executeLocally(DeleteInternalRepositoryAction.INSTANCE, request, future); assert future.isDone() : "Should be completed as it is executed synchronously"; } else { ActionRequest request = new PutInternalRepositoryRequest(clusterAlias, CcrRepository.TYPE); - PlainActionFuture future = PlainActionFuture.newFuture(); + PlainActionFuture future = PlainActionFuture.newFuture(); client.executeLocally(PutInternalRepositoryAction.INSTANCE, request, future); assert future.isDone() : "Should be completed as it is executed synchronously"; } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java index cb9eff50a8c5f..4bc55c906775d 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java @@ -24,14 +24,13 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.TransportAction; -import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; -public class DeleteInternalRepositoryAction extends Action { +public class DeleteInternalRepositoryAction extends Action { public static final DeleteInternalRepositoryAction INSTANCE = new DeleteInternalRepositoryAction(); public static final String NAME = "cluster:admin/internal_repository/delete"; @@ -41,17 +40,13 @@ private DeleteInternalRepositoryAction() { } @Override - public AcknowledgedResponse newResponse() { + public ActionResponse newResponse() { throw new UnsupportedOperationException(); } @Override - public Writeable.Reader getResponseReader() { - return in -> { - AcknowledgedResponse acknowledgedResponse = new AcknowledgedResponse(); - acknowledgedResponse.readFrom(in); - return acknowledgedResponse; - }; + public Writeable.Reader getResponseReader() { + return in -> new ActionResponse() {}; } public static class TransportDeleteInternalRepositoryAction extends TransportAction { diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java index aa06a15980d20..672a70a97df22 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java @@ -24,14 +24,13 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.TransportAction; -import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; -public class PutInternalRepositoryAction extends Action { +public class PutInternalRepositoryAction extends Action { public static final PutInternalRepositoryAction INSTANCE = new PutInternalRepositoryAction(); public static final String NAME = "cluster:admin/internal_repository/put"; @@ -41,17 +40,13 @@ private PutInternalRepositoryAction() { } @Override - public AcknowledgedResponse newResponse() { + public ActionResponse newResponse() { throw new UnsupportedOperationException(); } @Override - public Writeable.Reader getResponseReader() { - return in -> { - AcknowledgedResponse acknowledgedResponse = new AcknowledgedResponse(); - acknowledgedResponse.readFrom(in); - return acknowledgedResponse; - }; + public Writeable.Reader getResponseReader() { + return in -> new ActionResponse() {}; } public static class TransportPutInternalRepositoryAction extends TransportAction { diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java index 0612105abf70a..46eb7fe9a88b2 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java @@ -28,8 +28,6 @@ import java.io.IOException; import java.util.Objects; -import static org.elasticsearch.action.ValidateActions.addValidationError; - public class PutInternalRepositoryRequest extends ActionRequest { private final String name; From a15bffdae3c953f2837269470eca57a6d36414ac Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 30 Nov 2018 16:18:48 -0700 Subject: [PATCH 15/20] Fix licenses --- .../DeleteInternalRepositoryAction.java | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java index 4bc55c906775d..5b6b5753e0ca1 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java @@ -1,20 +1,7 @@ /* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. */ package org.elasticsearch.xpack.ccr.action.repositories; From 2b79524eb2bd6c2f178ad97f9f8c2dee506cc59b Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 30 Nov 2018 16:33:50 -0700 Subject: [PATCH 16/20] Fix license --- .../DeleteInternalRepositoryRequest.java | 19 +++---------------- .../PutInternalRepositoryAction.java | 19 +++---------------- .../PutInternalRepositoryRequest.java | 19 +++---------------- 3 files changed, 9 insertions(+), 48 deletions(-) diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryRequest.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryRequest.java index 628ea542c11f9..97490a3ed24a3 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryRequest.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryRequest.java @@ -1,20 +1,7 @@ /* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. */ package org.elasticsearch.xpack.ccr.action.repositories; diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java index 672a70a97df22..39887b450ca2d 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java @@ -1,20 +1,7 @@ /* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. */ package org.elasticsearch.xpack.ccr.action.repositories; diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java index 46eb7fe9a88b2..127e577e88c2c 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java @@ -1,20 +1,7 @@ /* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. */ package org.elasticsearch.xpack.ccr.action.repositories; From 6cb2750b1fd6f3de17b907d4ebefcafd90ed75a4 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Mon, 3 Dec 2018 12:34:52 -0700 Subject: [PATCH 17/20] Changes for review --- .../elasticsearch/action/ActionModule.java | 2 +- .../repositories/RepositoriesService.java | 61 ++++--------------- .../transport/RemoteClusterService.java | 2 +- .../RepositoriesServiceTests.java | 12 ++-- .../java/org/elasticsearch/xpack/ccr/Ccr.java | 12 ++-- .../xpack/ccr/CcrRepositoryManager.java | 16 ++--- ...=> DeleteInternalCcrRepositoryAction.java} | 10 +-- ...> DeleteInternalCcrRepositoryRequest.java} | 6 +- ...va => PutInternalCcrRepositoryAction.java} | 12 ++-- ...a => PutInternalCcrRepositoryRequest.java} | 25 ++------ 10 files changed, 53 insertions(+), 105 deletions(-) rename x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/{DeleteInternalRepositoryAction.java => DeleteInternalCcrRepositoryAction.java} (80%) rename x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/{DeleteInternalRepositoryRequest.java => DeleteInternalCcrRepositoryRequest.java} (88%) rename x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/{PutInternalRepositoryAction.java => PutInternalCcrRepositoryAction.java} (79%) rename x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/{PutInternalRepositoryRequest.java => PutInternalCcrRepositoryRequest.java} (69%) diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index f442748c15ab1..71eddc65c18a4 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -19,8 +19,8 @@ package org.elasticsearch.action; -import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.action.admin.cluster.allocation.ClusterAllocationExplainAction; import org.elasticsearch.action.admin.cluster.allocation.TransportClusterAllocationExplainAction; import org.elasticsearch.action.admin.cluster.health.ClusterHealthAction; diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index c0e51c2fd6cb7..7c25e2a803d1b 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -360,62 +360,23 @@ public Repository repository(String repositoryName) { throw new RepositoryMissingException(repositoryName); } - public void registerInternalRepository(String name, String type, Settings settings) { - RepositoryMetaData metaData = new RepositoryMetaData(name, type, settings); - Repository existingRepository = internalRepositories.get(name); - if (existingRepository != null && existingRepository.getMetadata().equals(metaData)) { - // Identical repository already exists, no need to update it. - return; - } - - // TODO: Normally we would do validation when we update a repository to ensure that it is not in use. - // Are we okay with not including that validation under the assumption that internal operations - // will do the right thing. - - Repository newRepository = createRepository(metaData, internalTypesRegistry); - Repository repositoryToClose = null; - boolean updated = false; - synchronized (internalRepositories) { - existingRepository = internalRepositories.get(name); - if (existingRepository != null) { - if (existingRepository.getMetadata().equals(metaData)) { - // Identical repository already exists, no need to update it. - repositoryToClose = newRepository; - } else { - logger.info("update internal repository [{}][{}]", name, type); - internalRepositories.put(name, newRepository); - updated = true; - repositoryToClose = existingRepository; - } - } else { - logger.info("put internal repository [{}][{}]", name, type); - internalRepositories.put(name, newRepository); - updated = true; - } - } - - if (repositoryToClose != null) { - repositoryToClose.close(); - } - - if (updated && repositories.containsKey(name)) { + public void registerInternalRepository(String name, String type) { + RepositoryMetaData metaData = new RepositoryMetaData(name, type, Settings.EMPTY); + Repository repository = internalRepositories.computeIfAbsent(name, (n) -> { + logger.debug("put internal repository [{}][{}]", name, type); + return createRepository(metaData, internalTypesRegistry); + }); + if (type.equals(repository.getMetadata().type()) == false) { + logger.warn(new ParameterizedMessage("internal repository [{}][{}] already registered. this prevented the registration of " + + "internal repository [{}][{}].", name, repository.getMetadata().type(), name, type)); + } else if (repositories.containsKey(name)) { logger.warn(new ParameterizedMessage("non-internal repository [{}] already registered. this repository will block the " + "usage of internal repository [{}][{}].", name, metaData.type(), name)); } } public void unregisterInternalRepository(String name) { - Repository repository = internalRepositories.get(name); - if (repository == null) { - // Repository does not exist to delete - return; - } - - synchronized (internalRepositories) { - // Even though we are using a concurrent hash map, the remove must be synchronized as it can - // conflict with the put in #registerInternalRepository which is a multi-step operation - repository = internalRepositories.remove(name); - } + Repository repository = internalRepositories.remove(name); if (repository != null) { RepositoryMetaData metadata = repository.getMetadata(); logger.debug(() -> new ParameterizedMessage("delete internal repository [{}][{}].", metadata.type(), name)); diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java index 1997be3dc1be3..52da474f2dd4a 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java @@ -278,7 +278,7 @@ boolean isRemoteNodeConnected(final String remoteCluster, final DiscoveryNode no public Map groupIndices(IndicesOptions indicesOptions, String[] indices, Predicate indexExists) { Map originalIndicesMap = new HashMap<>(); if (isCrossClusterSearchEnabled()) { - final Map> groupedIndices = groupClusterIndices(remoteClusters.keySet(), indices, indexExists); + final Map> groupedIndices = groupClusterIndices(getRemoteClusterNames(), indices, indexExists); if (groupedIndices.isEmpty()) { //search on _all in the local cluster if neither local indices nor remote indices were specified originalIndicesMap.put(LOCAL_CLUSTER_GROUP_KEY, new OriginalIndices(Strings.EMPTY_ARRAY, indicesOptions)); diff --git a/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java b/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java index f8ba5127f98a4..22af0dbc7c6f7 100644 --- a/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java @@ -68,7 +68,7 @@ public void setUp() throws Exception { public void testRegisterInternalRepository() { String repoName = "name"; expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(repoName)); - repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE, Settings.EMPTY); + repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE); Repository repository = repositoriesService.repository(repoName); assertEquals(repoName, repository.getMetadata().name()); assertEquals(TestRepository.TYPE, repository.getMetadata().type()); @@ -79,7 +79,7 @@ public void testRegisterInternalRepository() { public void testUnregisterInternalRepository() { String repoName = "name"; expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(repoName)); - repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE, Settings.EMPTY); + repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE); Repository repository = repositoriesService.repository(repoName); assertFalse(((TestRepository) repository).isClosed); repositoriesService.unregisterInternalRepository(repoName); @@ -90,10 +90,10 @@ public void testUnregisterInternalRepository() { public void testRegisterWillNotUpdateIdenticalInternalRepository() { String repoName = "name"; expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(repoName)); - repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE, Settings.EMPTY); + repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE); Repository repository = repositoriesService.repository(repoName); assertFalse(((TestRepository) repository).isClosed); - repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE, Settings.EMPTY); + repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE); assertFalse(((TestRepository) repository).isClosed); Repository repository2 = repositoriesService.repository(repoName); assertSame(repository, repository2); @@ -102,11 +102,11 @@ public void testRegisterWillNotUpdateIdenticalInternalRepository() { public void testRegisterWillUpdateInternalRepository() { String repoName = "name"; expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(repoName)); - repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE, Settings.EMPTY); + repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE); Repository repository = repositoriesService.repository(repoName); assertFalse(((TestRepository) repository).isClosed); Settings newSettings = Settings.builder().put("foo", "bar").build(); - repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE, newSettings); + repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE); assertTrue(((TestRepository) repository).isClosed); Repository repository2 = repositoriesService.repository(repoName); assertFalse(((TestRepository) repository2).isClosed); diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java index 254fb82de92ad..7cceecbd399a5 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java @@ -59,8 +59,8 @@ import org.elasticsearch.xpack.ccr.action.TransportUnfollowAction; import org.elasticsearch.xpack.ccr.action.bulk.BulkShardOperationsAction; import org.elasticsearch.xpack.ccr.action.bulk.TransportBulkShardOperationsAction; -import org.elasticsearch.xpack.ccr.action.repositories.DeleteInternalRepositoryAction; -import org.elasticsearch.xpack.ccr.action.repositories.PutInternalRepositoryAction; +import org.elasticsearch.xpack.ccr.action.repositories.DeleteInternalCcrRepositoryAction; +import org.elasticsearch.xpack.ccr.action.repositories.PutInternalCcrRepositoryAction; import org.elasticsearch.xpack.ccr.index.engine.FollowingEngineFactory; import org.elasticsearch.xpack.ccr.repository.CcrRepository; import org.elasticsearch.xpack.ccr.rest.RestCcrStatsAction; @@ -177,10 +177,10 @@ public List> getPersistentTasksExecutor(ClusterServic // internal actions new ActionHandler<>(BulkShardOperationsAction.INSTANCE, TransportBulkShardOperationsAction.class), new ActionHandler<>(ShardChangesAction.INSTANCE, ShardChangesAction.TransportAction.class), - new ActionHandler<>(PutInternalRepositoryAction.INSTANCE, - PutInternalRepositoryAction.TransportPutInternalRepositoryAction.class), - new ActionHandler<>(DeleteInternalRepositoryAction.INSTANCE, - DeleteInternalRepositoryAction.TransportDeleteInternalRepositoryAction.class), + new ActionHandler<>(PutInternalCcrRepositoryAction.INSTANCE, + PutInternalCcrRepositoryAction.TransportPutInternalRepositoryAction.class), + new ActionHandler<>(DeleteInternalCcrRepositoryAction.INSTANCE, + DeleteInternalCcrRepositoryAction.TransportDeleteInternalRepositoryAction.class), // stats action new ActionHandler<>(FollowStatsAction.INSTANCE, TransportFollowStatsAction.class), new ActionHandler<>(CcrStatsAction.INSTANCE, TransportCcrStatsAction.class), diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java index 6bb02ecb1122d..556873c704e05 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java @@ -13,10 +13,10 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.transport.RemoteClusterAware; -import org.elasticsearch.xpack.ccr.action.repositories.DeleteInternalRepositoryAction; -import org.elasticsearch.xpack.ccr.action.repositories.DeleteInternalRepositoryRequest; -import org.elasticsearch.xpack.ccr.action.repositories.PutInternalRepositoryAction; -import org.elasticsearch.xpack.ccr.action.repositories.PutInternalRepositoryRequest; +import org.elasticsearch.xpack.ccr.action.repositories.DeleteInternalCcrRepositoryAction; +import org.elasticsearch.xpack.ccr.action.repositories.DeleteInternalCcrRepositoryRequest; +import org.elasticsearch.xpack.ccr.action.repositories.PutInternalCcrRepositoryAction; +import org.elasticsearch.xpack.ccr.action.repositories.PutInternalCcrRepositoryRequest; import org.elasticsearch.xpack.ccr.repository.CcrRepository; import java.util.List; @@ -34,14 +34,14 @@ class CcrRepositoryManager extends RemoteClusterAware { @Override protected void updateRemoteCluster(String clusterAlias, List addresses, String proxyAddress) { if (addresses.isEmpty()) { - DeleteInternalRepositoryRequest request = new DeleteInternalRepositoryRequest(clusterAlias); + DeleteInternalCcrRepositoryRequest request = new DeleteInternalCcrRepositoryRequest(clusterAlias); PlainActionFuture future = PlainActionFuture.newFuture(); - client.executeLocally(DeleteInternalRepositoryAction.INSTANCE, request, future); + client.executeLocally(DeleteInternalCcrRepositoryAction.INSTANCE, request, future); assert future.isDone() : "Should be completed as it is executed synchronously"; } else { - ActionRequest request = new PutInternalRepositoryRequest(clusterAlias, CcrRepository.TYPE); + ActionRequest request = new PutInternalCcrRepositoryRequest(clusterAlias, CcrRepository.TYPE); PlainActionFuture future = PlainActionFuture.newFuture(); - client.executeLocally(PutInternalRepositoryAction.INSTANCE, request, future); + client.executeLocally(PutInternalCcrRepositoryAction.INSTANCE, request, future); assert future.isDone() : "Should be completed as it is executed synchronously"; } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryAction.java similarity index 80% rename from x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java rename to x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryAction.java index 5b6b5753e0ca1..f23acd5c41b74 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryAction.java @@ -17,12 +17,12 @@ import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; -public class DeleteInternalRepositoryAction extends Action { +public class DeleteInternalCcrRepositoryAction extends Action { - public static final DeleteInternalRepositoryAction INSTANCE = new DeleteInternalRepositoryAction(); + public static final DeleteInternalCcrRepositoryAction INSTANCE = new DeleteInternalCcrRepositoryAction(); public static final String NAME = "cluster:admin/internal_repository/delete"; - private DeleteInternalRepositoryAction() { + private DeleteInternalCcrRepositoryAction() { super(NAME); } @@ -36,7 +36,7 @@ public Writeable.Reader getResponseReader() { return in -> new ActionResponse() {}; } - public static class TransportDeleteInternalRepositoryAction extends TransportAction { + public static class TransportDeleteInternalRepositoryAction extends TransportAction { private final RepositoriesService repositoriesService; @@ -48,7 +48,7 @@ public TransportDeleteInternalRepositoryAction(RepositoriesService repositoriesS } @Override - protected void doExecute(Task task, DeleteInternalRepositoryRequest request, ActionListener listener) { + protected void doExecute(Task task, DeleteInternalCcrRepositoryRequest request, ActionListener listener) { repositoriesService.unregisterInternalRepository(request.getName()); listener.onResponse(new ActionResponse() {}); } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryRequest.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryRequest.java similarity index 88% rename from x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryRequest.java rename to x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryRequest.java index 97490a3ed24a3..12264c1d57c85 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalRepositoryRequest.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryRequest.java @@ -14,11 +14,11 @@ import java.io.IOException; import java.util.Objects; -public class DeleteInternalRepositoryRequest extends ActionRequest { +public class DeleteInternalCcrRepositoryRequest extends ActionRequest { private final String name; - public DeleteInternalRepositoryRequest(String name) { + public DeleteInternalCcrRepositoryRequest(String name) { this.name = Objects.requireNonNull(name); } @@ -45,7 +45,7 @@ public String getName() { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - DeleteInternalRepositoryRequest that = (DeleteInternalRepositoryRequest) o; + DeleteInternalCcrRepositoryRequest that = (DeleteInternalCcrRepositoryRequest) o; return Objects.equals(name, that.name); } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryAction.java similarity index 79% rename from x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java rename to x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryAction.java index 39887b450ca2d..927f282212893 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryAction.java @@ -17,12 +17,12 @@ import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; -public class PutInternalRepositoryAction extends Action { +public class PutInternalCcrRepositoryAction extends Action { - public static final PutInternalRepositoryAction INSTANCE = new PutInternalRepositoryAction(); + public static final PutInternalCcrRepositoryAction INSTANCE = new PutInternalCcrRepositoryAction(); public static final String NAME = "cluster:admin/internal_repository/put"; - private PutInternalRepositoryAction() { + private PutInternalCcrRepositoryAction() { super(NAME); } @@ -36,7 +36,7 @@ public Writeable.Reader getResponseReader() { return in -> new ActionResponse() {}; } - public static class TransportPutInternalRepositoryAction extends TransportAction { + public static class TransportPutInternalRepositoryAction extends TransportAction { private final RepositoriesService repositoriesService; @@ -48,8 +48,8 @@ public TransportPutInternalRepositoryAction(RepositoriesService repositoriesServ } @Override - protected void doExecute(Task task, PutInternalRepositoryRequest request, ActionListener listener) { - repositoriesService.registerInternalRepository(request.getName(), request.getType(), request.getSettings()); + protected void doExecute(Task task, PutInternalCcrRepositoryRequest request, ActionListener listener) { + repositoriesService.registerInternalRepository(request.getName(), request.getType()); listener.onResponse(new ActionResponse() {}); } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryRequest.java similarity index 69% rename from x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java rename to x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryRequest.java index 127e577e88c2c..71efcdf319da9 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalRepositoryRequest.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryRequest.java @@ -10,25 +10,18 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.settings.Settings; import java.io.IOException; import java.util.Objects; -public class PutInternalRepositoryRequest extends ActionRequest { +public class PutInternalCcrRepositoryRequest extends ActionRequest { private final String name; private final String type; - private final Settings settings; - public PutInternalRepositoryRequest(String name, String type) { - this(name, type, Settings.EMPTY); - } - - public PutInternalRepositoryRequest(String name, String type, Settings settings) { + public PutInternalCcrRepositoryRequest(String name, String type) { this.name = Objects.requireNonNull(name); this.type = Objects.requireNonNull(type); - this.settings = settings; } @Override @@ -54,31 +47,25 @@ public String getType() { return type; } - public Settings getSettings() { - return settings; - } - @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - PutInternalRepositoryRequest that = (PutInternalRepositoryRequest) o; + PutInternalCcrRepositoryRequest that = (PutInternalCcrRepositoryRequest) o; return Objects.equals(name, that.name) && - Objects.equals(type, that.type) && - Objects.equals(settings, that.settings); + Objects.equals(type, that.type); } @Override public int hashCode() { - return Objects.hash(name, type, settings); + return Objects.hash(name, type); } @Override public String toString() { - return "PutInternalRepositoryRequest{" + + return "PutInternalCcrRepositoryRequest{" + "name='" + name + '\'' + ", type='" + type + '\'' + - ", settings=" + settings + '}'; } } From 0259d7d5539a0622f7d5327a178f126bd3f2c2ae Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Mon, 3 Dec 2018 16:28:31 -0700 Subject: [PATCH 18/20] Changes --- .../RepositoriesServiceTests.java | 16 +--------- .../xpack/ccr/CcrRepositoryManager.java | 18 +++++------ .../DeleteInternalCcrRepositoryAction.java | 32 ++++++++++++++----- .../PutInternalCcrRepositoryAction.java | 32 ++++++++++++++----- .../xpack/ccr/repository/CcrRepository.java | 1 + .../xpack/ccr/CcrRepositoryManagerIT.java | 14 ++++---- 6 files changed, 67 insertions(+), 46 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java b/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java index 22af0dbc7c6f7..c02ab0d185610 100644 --- a/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/RepositoriesServiceTests.java @@ -87,7 +87,7 @@ public void testUnregisterInternalRepository() { assertTrue(((TestRepository) repository).isClosed); } - public void testRegisterWillNotUpdateIdenticalInternalRepository() { + public void testRegisterWillNotUpdateIfInternalRepositoryWithNameExists() { String repoName = "name"; expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(repoName)); repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE); @@ -99,20 +99,6 @@ public void testRegisterWillNotUpdateIdenticalInternalRepository() { assertSame(repository, repository2); } - public void testRegisterWillUpdateInternalRepository() { - String repoName = "name"; - expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(repoName)); - repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE); - Repository repository = repositoriesService.repository(repoName); - assertFalse(((TestRepository) repository).isClosed); - Settings newSettings = Settings.builder().put("foo", "bar").build(); - repositoriesService.registerInternalRepository(repoName, TestRepository.TYPE); - assertTrue(((TestRepository) repository).isClosed); - Repository repository2 = repositoriesService.repository(repoName); - assertFalse(((TestRepository) repository2).isClosed); - assertEquals(newSettings, repository2.getMetadata().settings()); - } - private static class TestRepository implements Repository { private static final String TYPE = "internal"; diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java index 556873c704e05..f86789a880e24 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrRepositoryManager.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.ccr; import org.elasticsearch.action.ActionRequest; -import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.service.ClusterService; @@ -33,16 +32,17 @@ class CcrRepositoryManager extends RemoteClusterAware { @Override protected void updateRemoteCluster(String clusterAlias, List addresses, String proxyAddress) { + String repositoryName = CcrRepository.NAME_PREFIX + clusterAlias; if (addresses.isEmpty()) { - DeleteInternalCcrRepositoryRequest request = new DeleteInternalCcrRepositoryRequest(clusterAlias); - PlainActionFuture future = PlainActionFuture.newFuture(); - client.executeLocally(DeleteInternalCcrRepositoryAction.INSTANCE, request, future); - assert future.isDone() : "Should be completed as it is executed synchronously"; + DeleteInternalCcrRepositoryRequest request = new DeleteInternalCcrRepositoryRequest(repositoryName); + PlainActionFuture f = PlainActionFuture.newFuture(); + client.executeLocally(DeleteInternalCcrRepositoryAction.INSTANCE, request, f); + assert f.isDone() : "Should be completed as it is executed synchronously"; } else { - ActionRequest request = new PutInternalCcrRepositoryRequest(clusterAlias, CcrRepository.TYPE); - PlainActionFuture future = PlainActionFuture.newFuture(); - client.executeLocally(PutInternalCcrRepositoryAction.INSTANCE, request, future); - assert future.isDone() : "Should be completed as it is executed synchronously"; + ActionRequest request = new PutInternalCcrRepositoryRequest(repositoryName, CcrRepository.TYPE); + PlainActionFuture f = PlainActionFuture.newFuture(); + client.executeLocally(PutInternalCcrRepositoryAction.INSTANCE, request, f); + assert f.isDone() : "Should be completed as it is executed synchronously"; } } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryAction.java index f23acd5c41b74..e85ce65858e0f 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/DeleteInternalCcrRepositoryAction.java @@ -12,31 +12,35 @@ import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; -public class DeleteInternalCcrRepositoryAction extends Action { +import java.io.IOException; + +public class DeleteInternalCcrRepositoryAction extends Action { public static final DeleteInternalCcrRepositoryAction INSTANCE = new DeleteInternalCcrRepositoryAction(); - public static final String NAME = "cluster:admin/internal_repository/delete"; + public static final String NAME = "cluster:admin/ccr/internal_repository/delete"; private DeleteInternalCcrRepositoryAction() { super(NAME); } @Override - public ActionResponse newResponse() { + public DeleteInternalCcrRepositoryResponse newResponse() { throw new UnsupportedOperationException(); } @Override - public Writeable.Reader getResponseReader() { - return in -> new ActionResponse() {}; + public Writeable.Reader getResponseReader() { + return DeleteInternalCcrRepositoryResponse::new; } - public static class TransportDeleteInternalRepositoryAction extends TransportAction { + public static class TransportDeleteInternalRepositoryAction + extends TransportAction { private final RepositoriesService repositoriesService; @@ -48,9 +52,21 @@ public TransportDeleteInternalRepositoryAction(RepositoriesService repositoriesS } @Override - protected void doExecute(Task task, DeleteInternalCcrRepositoryRequest request, ActionListener listener) { + protected void doExecute(Task task, DeleteInternalCcrRepositoryRequest request, + ActionListener listener) { repositoriesService.unregisterInternalRepository(request.getName()); - listener.onResponse(new ActionResponse() {}); + listener.onResponse(new DeleteInternalCcrRepositoryResponse()); + } + } + + public static class DeleteInternalCcrRepositoryResponse extends ActionResponse { + + DeleteInternalCcrRepositoryResponse() { + super(); + } + + DeleteInternalCcrRepositoryResponse(StreamInput streamInput) throws IOException { + super(streamInput); } } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryAction.java index 927f282212893..2d12cc4d77ad1 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryAction.java @@ -12,31 +12,35 @@ import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; -public class PutInternalCcrRepositoryAction extends Action { +import java.io.IOException; + +public class PutInternalCcrRepositoryAction extends Action { public static final PutInternalCcrRepositoryAction INSTANCE = new PutInternalCcrRepositoryAction(); - public static final String NAME = "cluster:admin/internal_repository/put"; + public static final String NAME = "cluster:admin/ccr/internal_repository/put"; private PutInternalCcrRepositoryAction() { super(NAME); } @Override - public ActionResponse newResponse() { + public PutInternalCcrRepositoryResponse newResponse() { throw new UnsupportedOperationException(); } @Override - public Writeable.Reader getResponseReader() { - return in -> new ActionResponse() {}; + public Writeable.Reader getResponseReader() { + return PutInternalCcrRepositoryResponse::new; } - public static class TransportPutInternalRepositoryAction extends TransportAction { + public static class TransportPutInternalRepositoryAction + extends TransportAction { private final RepositoriesService repositoriesService; @@ -48,9 +52,21 @@ public TransportPutInternalRepositoryAction(RepositoriesService repositoriesServ } @Override - protected void doExecute(Task task, PutInternalCcrRepositoryRequest request, ActionListener listener) { + protected void doExecute(Task task, PutInternalCcrRepositoryRequest request, + ActionListener listener) { repositoriesService.registerInternalRepository(request.getName(), request.getType()); - listener.onResponse(new ActionResponse() {}); + listener.onResponse(new PutInternalCcrRepositoryResponse()); + } + } + + public static class PutInternalCcrRepositoryResponse extends ActionResponse { + + PutInternalCcrRepositoryResponse() { + super(); + } + + PutInternalCcrRepositoryResponse(StreamInput streamInput) throws IOException { + super(streamInput); } } } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java index 00646a8d06e1b..81f9bd3f2f932 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java @@ -36,6 +36,7 @@ public class CcrRepository extends AbstractLifecycleComponent implements Repository { public static final String TYPE = "_ccr_"; + public static final String NAME_PREFIX = "_ccr_"; private final RepositoryMetaData metadata; diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java index 1a4ce8a34a4ea..4148894cc3ca5 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java @@ -22,13 +22,14 @@ public class CcrRepositoryManagerIT extends CcrIntegTestCase { public void testThatRepositoryIsPutAndRemovedWhenRemoteClusterIsUpdated() throws Exception { + String leaderClusterRepoName = CcrRepository.NAME_PREFIX + "leader_cluster"; assertBusy(() -> { final RepositoriesService repositoriesService = getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); try { - Repository repository = repositoriesService.repository("leader_cluster"); + Repository repository = repositoriesService.repository(leaderClusterRepoName); assertEquals(CcrRepository.TYPE, repository.getMetadata().type()); - assertEquals("leader_cluster", repository.getMetadata().name()); + assertEquals(leaderClusterRepoName, repository.getMetadata().name()); } catch (RepositoryMissingException e) { fail("need repository"); } @@ -39,13 +40,14 @@ public void testThatRepositoryIsPutAndRemovedWhenRemoteClusterIsUpdated() throws putFollowerRequest.persistentSettings(Settings.builder().put("cluster.remote.follower_cluster_copy.seeds", address)); assertAcked(followerClient().admin().cluster().updateSettings(putFollowerRequest).actionGet()); + String followerCopyRepoName = CcrRepository.NAME_PREFIX + "follower_cluster_copy"; assertBusy(() -> { final RepositoriesService repositoriesService = getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); try { - Repository repository = repositoriesService.repository("follower_cluster_copy"); + Repository repository = repositoriesService.repository(followerCopyRepoName); assertEquals(CcrRepository.TYPE, repository.getMetadata().type()); - assertEquals("follower_cluster_copy", repository.getMetadata().name()); + assertEquals(followerCopyRepoName, repository.getMetadata().name()); } catch (RepositoryMissingException e) { fail("need repository"); } @@ -58,7 +60,7 @@ public void testThatRepositoryIsPutAndRemovedWhenRemoteClusterIsUpdated() throws assertBusy(() -> { final RepositoriesService repositoriesService = getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); - expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository("leader_cluster")); + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(leaderClusterRepoName)); }); ClusterUpdateSettingsRequest deleteFollowerRequest = new ClusterUpdateSettingsRequest(); @@ -68,7 +70,7 @@ public void testThatRepositoryIsPutAndRemovedWhenRemoteClusterIsUpdated() throws assertBusy(() -> { final RepositoriesService repositoriesService = getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); - expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository("follower_cluster_copy")); + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(followerCopyRepoName)); }); } } From a7400ccfa0a002081576f12c05650a3918e7344b Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Mon, 3 Dec 2018 16:37:26 -0700 Subject: [PATCH 19/20] Add validation --- .../org/elasticsearch/repositories/RepositoriesModule.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java index 9b3d20cd78efa..90e3c94dfb3c5 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java @@ -65,6 +65,10 @@ public RepositoriesModule(Environment env, List repoPlugins, T if (internalFactories.put(entry.getKey(), entry.getValue()) != null) { throw new IllegalArgumentException("Internal repository type [" + entry.getKey() + "] is already registered"); } + if (factories.put(entry.getKey(), entry.getValue()) != null) { + throw new IllegalArgumentException("Internal repository type [" + entry.getKey() + "] is already registered as a " + + "non-internal repository"); + } } } From 2dd498189aa7062a0b7c9db8a6d922cb66c5cf85 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 4 Dec 2018 10:25:54 -0700 Subject: [PATCH 20/20] Changes for review --- .../repositories/RepositoriesModuleTests.java | 101 ++++++++++++++++++ .../xpack/ccr/CcrRepositoryManagerIT.java | 50 ++++----- 2 files changed, 119 insertions(+), 32 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/repositories/RepositoriesModuleTests.java diff --git a/server/src/test/java/org/elasticsearch/repositories/RepositoriesModuleTests.java b/server/src/test/java/org/elasticsearch/repositories/RepositoriesModuleTests.java new file mode 100644 index 0000000000000..96a9670d16202 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/repositories/RepositoriesModuleTests.java @@ -0,0 +1,101 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.repositories; + +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.env.Environment; +import org.elasticsearch.plugins.RepositoryPlugin; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.TransportService; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class RepositoriesModuleTests extends ESTestCase { + + private Environment environment; + private NamedXContentRegistry contentRegistry; + private List repoPlugins = new ArrayList<>(); + private RepositoryPlugin plugin1; + private RepositoryPlugin plugin2; + private Repository.Factory factory; + + @Override + public void setUp() throws Exception { + super.setUp(); + environment = mock(Environment.class); + contentRegistry = mock(NamedXContentRegistry.class); + plugin1 = mock(RepositoryPlugin.class); + plugin2 = mock(RepositoryPlugin.class); + factory = mock(Repository.Factory.class); + repoPlugins.add(plugin1); + repoPlugins.add(plugin2); + when(environment.settings()).thenReturn(Settings.EMPTY); + } + + public void testCanRegisterTwoRepositoriesWithDifferentTypes() { + when(plugin1.getRepositories(environment, contentRegistry)).thenReturn(Collections.singletonMap("type1", factory)); + when(plugin2.getRepositories(environment, contentRegistry)).thenReturn(Collections.singletonMap("type2", factory)); + + // Would throw + new RepositoriesModule(environment, repoPlugins, mock(TransportService.class), mock(ClusterService.class), + mock(ThreadPool.class), contentRegistry); + } + + public void testCannotRegisterTwoRepositoriesWithSameTypes() { + when(plugin1.getRepositories(environment, contentRegistry)).thenReturn(Collections.singletonMap("type1", factory)); + when(plugin2.getRepositories(environment, contentRegistry)).thenReturn(Collections.singletonMap("type1", factory)); + + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, + () -> new RepositoriesModule(environment, repoPlugins, mock(TransportService.class), mock(ClusterService.class), + mock(ThreadPool.class), contentRegistry)); + + assertEquals("Repository type [type1] is already registered", ex.getMessage()); + } + + public void testCannotRegisterTwoInternalRepositoriesWithSameTypes() { + when(plugin1.getInternalRepositories(environment, contentRegistry)).thenReturn(Collections.singletonMap("type1", factory)); + when(plugin2.getInternalRepositories(environment, contentRegistry)).thenReturn(Collections.singletonMap("type1", factory)); + + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, + () -> new RepositoriesModule(environment, repoPlugins, mock(TransportService.class), mock(ClusterService.class), + mock(ThreadPool.class), contentRegistry)); + + assertEquals("Internal repository type [type1] is already registered", ex.getMessage()); + } + + public void testCannotRegisterNormalAndInternalRepositoriesWithSameTypes() { + when(plugin1.getRepositories(environment, contentRegistry)).thenReturn(Collections.singletonMap("type1", factory)); + when(plugin2.getInternalRepositories(environment, contentRegistry)).thenReturn(Collections.singletonMap("type1", factory)); + + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, + () -> new RepositoriesModule(environment, repoPlugins, mock(TransportService.class), mock(ClusterService.class), + mock(ThreadPool.class), contentRegistry)); + + assertEquals("Internal repository type [type1] is already registered as a non-internal repository", ex.getMessage()); + } +} diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java index 4148894cc3ca5..133e1ee06064f 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrRepositoryManagerIT.java @@ -23,17 +23,15 @@ public class CcrRepositoryManagerIT extends CcrIntegTestCase { public void testThatRepositoryIsPutAndRemovedWhenRemoteClusterIsUpdated() throws Exception { String leaderClusterRepoName = CcrRepository.NAME_PREFIX + "leader_cluster"; - assertBusy(() -> { - final RepositoriesService repositoriesService = - getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); - try { - Repository repository = repositoriesService.repository(leaderClusterRepoName); - assertEquals(CcrRepository.TYPE, repository.getMetadata().type()); - assertEquals(leaderClusterRepoName, repository.getMetadata().name()); - } catch (RepositoryMissingException e) { - fail("need repository"); - } - }); + final RepositoriesService repositoriesService = + getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); + try { + Repository repository = repositoriesService.repository(leaderClusterRepoName); + assertEquals(CcrRepository.TYPE, repository.getMetadata().type()); + assertEquals(leaderClusterRepoName, repository.getMetadata().name()); + } catch (RepositoryMissingException e) { + fail("need repository"); + } ClusterUpdateSettingsRequest putFollowerRequest = new ClusterUpdateSettingsRequest(); String address = getFollowerCluster().getDataNodeInstance(TransportService.class).boundAddress().publishAddress().toString(); @@ -41,36 +39,24 @@ public void testThatRepositoryIsPutAndRemovedWhenRemoteClusterIsUpdated() throws assertAcked(followerClient().admin().cluster().updateSettings(putFollowerRequest).actionGet()); String followerCopyRepoName = CcrRepository.NAME_PREFIX + "follower_cluster_copy"; - assertBusy(() -> { - final RepositoriesService repositoriesService = - getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); - try { - Repository repository = repositoriesService.repository(followerCopyRepoName); - assertEquals(CcrRepository.TYPE, repository.getMetadata().type()); - assertEquals(followerCopyRepoName, repository.getMetadata().name()); - } catch (RepositoryMissingException e) { - fail("need repository"); - } - }); + try { + Repository repository = repositoriesService.repository(followerCopyRepoName); + assertEquals(CcrRepository.TYPE, repository.getMetadata().type()); + assertEquals(followerCopyRepoName, repository.getMetadata().name()); + } catch (RepositoryMissingException e) { + fail("need repository"); + } ClusterUpdateSettingsRequest deleteLeaderRequest = new ClusterUpdateSettingsRequest(); deleteLeaderRequest.persistentSettings(Settings.builder().put("cluster.remote.leader_cluster.seeds", "")); assertAcked(followerClient().admin().cluster().updateSettings(deleteLeaderRequest).actionGet()); - assertBusy(() -> { - final RepositoriesService repositoriesService = - getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); - expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(leaderClusterRepoName)); - }); + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(leaderClusterRepoName)); ClusterUpdateSettingsRequest deleteFollowerRequest = new ClusterUpdateSettingsRequest(); deleteFollowerRequest.persistentSettings(Settings.builder().put("cluster.remote.follower_cluster_copy.seeds", "")); assertAcked(followerClient().admin().cluster().updateSettings(deleteFollowerRequest).actionGet()); - assertBusy(() -> { - final RepositoriesService repositoriesService = - getFollowerCluster().getDataOrMasterNodeInstances(RepositoriesService.class).iterator().next(); - expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(followerCopyRepoName)); - }); + expectThrows(RepositoryMissingException.class, () -> repositoriesService.repository(followerCopyRepoName)); } }