Extract a common base class to allow services to listen to remote cluster config updates #24367
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ster config updates RemoteClusterService is an internal service that should not necessarily be exposed to plugins or other parts of the system. Yet, for cluster name parsing for instance it is crucial to reuse some code that is used for the RemoteClusterService. This change extracts a base class that allows to share the settings related code as well as cluster settings updates to `search.remote.*` to be observed by other services.
tvernum
requested changes
May 2, 2017
| "seeds", (key) -> Setting.listSetting(key, Collections.emptyList(), RemoteClusterAware::parseSeedAddress, | ||
| Setting.Property.NodeScope, Setting.Property.Dynamic)); | ||
| protected static final char REMOTE_CLUSTER_INDEX_SEPARATOR = ':'; | ||
| static final String LOCAL_CLUSTER_GROUP_KEY = ""; |
There was a problem hiding this comment.
can this be protected so that derived classes can safely interpret the result from groupClusterIndices?
| * {@link #LOCAL_CLUSTER_GROUP_KEY}. The returned map is mutable. | ||
| * | ||
| * @param requestIndices the indices in the search request to filter | ||
| * @param indexExists a predicate that can test if a certain index or alias exists |
There was a problem hiding this comment.
We we append "locally" or "in the local cluster" to this param description?
| protected abstract void updateRemoteCluster(String clusterAlias, List<InetSocketAddress> addresses); | ||
|
|
||
| /** | ||
| * Registers this instance to listen to updates ont the cluster settings. |
| boolean executeImmediate = false; | ||
| synchronized (this) { | ||
| boolean executeImmediate = false; | ||
| synchronized (this) { |
There was a problem hiding this comment.
I don't think this indenting is intended.
| for (Map.Entry<String, List<DiscoveryNode>> entry : seeds.entrySet()) { | ||
| updateRemoteCluster(entry.getKey(), entry.getValue().stream().map(n -> n.getAddress().address()) | ||
| .collect(Collectors.toList())); | ||
| } |
There was a problem hiding this comment.
Can we pull this initialisation code into a method.
I imagine most derived classes will implement updateRemoteCluster by storing the cluster names/addresses into some form of collection.
But if that collection is a field, then it won't have been initialised at this point (in the parent constructor).
The typical constructor is going to need to look like:
{
super(settings, false);
clusterNames = new ArrayList<>();
initialize();
}
|
@tvernum can you look again... |
s1monw
added a commit
that referenced
this pull request
May 2, 2017
…ster config updates (#24367) RemoteClusterService is an internal service that should not necessarily be exposed to plugins or other parts of the system. Yet, for cluster name parsing for instance it is crucial to reuse some code that is used for the RemoteClusterService. This change extracts a base class that allows to share the settings related code as well as cluster settings updates to `search.remote.*` to be observed by other services.
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
May 2, 2017
* master: (27 commits) Check index sorting with no replica since we cannot ensure that the replica index is ready when forceMerge is called. Closes elastic#24416 Docs: correct indentation on callout Build that java api docs from a test (elastic#24354) Move RemoteClusterService into TransportService (elastic#24424) Fix license header in WildflyIT.java Try not to lose stacktraces (elastic#24426) [DOCS] Update XPack Reference URL for 5.4 (elastic#24425) Painless: Add tests to check for existence and correct detection of the special Java 9 optimizations: Indified String concat and MethodHandles#ArrayLengthHelper() (elastic#24405) Extract a common base class to allow services to listen to remote cluster config updates (elastic#24367) Adds check to snapshot repository incompatible-snapshots blob to delete a pre-existing one before attempting to overwrite it. Added docs for batched_reduce_size Fixes checkstyle errors Allow scripted metric agg to access `_score` (elastic#24295) [Test] Add unit tests for HDR/TDigest PercentilesAggregators (elastic#24245) Fix FieldCaps documentation Upgrade to JUnit 4.12 (elastic#23877) Set available processors for Netty Painless: Fix method references to ctor with the new LambdaBootstrap and cleanup code (elastic#24406) Doc test: use propery regex for file size [DOCS] Tweak doc test to sync_flush ...
clintongormley
added
:Distributed/Distributed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
RemoteClusterService is an internal service that should not necessarily be exposed
to plugins or other parts of the system. Yet, for cluster name parsing for instance
it is crucial to reuse some code that is used for the RemoteClusterService. This
change extracts a base class that allows to share the settings related code as well
as cluster settings updates to
search.remote.*to be observed by other services.