Skip to content

Commit 5251826

Browse files
JeremyDahlgrenJeremyDahlgren
committed
Consolidate settings consumers for RemoteClusterService
Moves the REMOTE_CLUSTER_SKIP_UNAVAILABLE setting into the main AffixSettings list in ClusterSettingsLinkedProjectConfigService, eliminating the need for an extra skip_unavailable specific update method on the LinkedProjectConfigListener interface, simplifying the design. Note that it is not possible to just set REMOTE_CLUSTER_SKIP_UNAVAILABLE in a settings update, the RemoteConnectionEnabled settings validator checks that the required settings for an enabled connection are also present. Resolves: ES-12860
1 parent 0cee213 commit 5251826

File tree

6 files changed

+60
-104
lines changed

6 files changed

+60
-104
lines changed

‎server/src/main/java/org/elasticsearch/transport/AbstractLinkedProjectConfigService.java‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99

1010
package org.elasticsearch.transport;
1111

12-
import org.elasticsearch.cluster.metadata.ProjectId;
13-
1412
import java.util.List;
1513
import java.util.concurrent.CopyOnWriteArrayList;
1614

@@ -29,15 +27,4 @@ public void register(LinkedProjectConfigListener listener) {
2927
protected void handleUpdate(LinkedProjectConfig config) {
3028
listeners.forEach(listener -> listener.updateLinkedProject(config));
3129
}
32-
33-
protected void handleSkipUnavailableChanged(
34-
ProjectId originProjectId,
35-
ProjectId linkedProjectId,
36-
String linkedProjectAlias,
37-
boolean skipUnavailable
38-
) {
39-
listeners.forEach(
40-
listener -> listener.skipUnavailableChanged(originProjectId, linkedProjectId, linkedProjectAlias, skipUnavailable)
41-
);
42-
}
4330
}

‎server/src/main/java/org/elasticsearch/transport/ClusterSettingsLinkedProjectConfigService.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ public ClusterSettingsLinkedProjectConfigService(
4848
RemoteClusterSettings.REMOTE_CLUSTER_COMPRESS,
4949
RemoteClusterSettings.REMOTE_CLUSTER_PING_SCHEDULE,
5050
RemoteClusterSettings.REMOTE_CONNECTION_MODE,
51+
RemoteClusterSettings.REMOTE_CLUSTER_SKIP_UNAVAILABLE,
5152
RemoteClusterSettings.SniffConnectionStrategySettings.REMOTE_CLUSTERS_PROXY,
5253
RemoteClusterSettings.SniffConnectionStrategySettings.REMOTE_CLUSTER_SEEDS,
5354
RemoteClusterSettings.SniffConnectionStrategySettings.REMOTE_NODE_CONNECTIONS,
@@ -56,11 +57,6 @@ public ClusterSettingsLinkedProjectConfigService(
5657
RemoteClusterSettings.ProxyConnectionStrategySettings.SERVER_NAME
5758
);
5859
clusterSettings.addAffixGroupUpdateConsumer(remoteClusterSettings, this::settingsChangedCallback);
59-
clusterSettings.addAffixUpdateConsumer(
60-
RemoteClusterSettings.REMOTE_CLUSTER_SKIP_UNAVAILABLE,
61-
this::skipUnavailableChangedCallback,
62-
(alias, value) -> {}
63-
);
6460
}
6561
}
6662

@@ -79,9 +75,4 @@ private void settingsChangedCallback(String clusterAlias, Settings newSettings)
7975
final var config = RemoteClusterSettings.toConfig(projectResolver.getProjectId(), ProjectId.DEFAULT, clusterAlias, mergedSettings);
8076
handleUpdate(config);
8177
}
82-
83-
@FixForMultiProject(description = "Refactor to add the linked project ID associated with the alias.")
84-
private void skipUnavailableChangedCallback(String clusterAlias, Boolean skipUnavailable) {
85-
handleSkipUnavailableChanged(projectResolver.getProjectId(), ProjectId.DEFAULT, clusterAlias, skipUnavailable);
86-
}
8778
}

‎server/src/main/java/org/elasticsearch/transport/LinkedProjectConfigService.java‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99

1010
package org.elasticsearch.transport;
1111

12-
import org.elasticsearch.cluster.metadata.ProjectId;
13-
1412
import java.util.Collection;
1513

1614
/**
@@ -29,22 +27,6 @@ interface LinkedProjectConfigListener {
2927
* @param config The updated {@link LinkedProjectConfig}.
3028
*/
3129
void updateLinkedProject(LinkedProjectConfig config);
32-
33-
/**
34-
* Called when the boolean skip_unavailable setting has changed for a linked project configuration.
35-
* Note that skip_unavailable may not be supported in all contexts where linked projects are used.
36-
*
37-
* @param originProjectId The {@link ProjectId} of the owning project that has the linked project configuration.
38-
* @param linkedProjectId The {@link ProjectId} of the linked project.
39-
* @param linkedProjectAlias The alias used for the linked project.
40-
* @param skipUnavailable The new value of the skip_unavailable setting.
41-
*/
42-
default void skipUnavailableChanged(
43-
ProjectId originProjectId,
44-
ProjectId linkedProjectId,
45-
String linkedProjectAlias,
46-
boolean skipUnavailable
47-
) {}
4830
}
4931

5032
/**

‎server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -285,19 +285,6 @@ public RemoteClusterConnection getRemoteClusterConnection(String cluster) {
285285
return connection;
286286
}
287287

288-
@Override
289-
public void skipUnavailableChanged(
290-
ProjectId originProjectId,
291-
ProjectId linkedProjectId,
292-
String linkedProjectAlias,
293-
boolean skipUnavailable
294-
) {
295-
final var remote = getConnectionsMapForProject(originProjectId).get(linkedProjectAlias);
296-
if (remote != null) {
297-
remote.setSkipUnavailable(skipUnavailable);
298-
}
299-
}
300-
301288
@FixForMultiProject(description = "Refactor as needed to support project specific changes to linked remotes.")
302289
public synchronized void updateRemoteClusterCredentials(Supplier<Settings> settingsSupplier, ActionListener<Void> listener) {
303290
final var projectId = projectResolver.getProjectId();
@@ -450,6 +437,9 @@ synchronized void updateRemoteCluster(
450437
connectionMap.put(clusterAlias, remote);
451438
remote.ensureConnected(listener.map(ignored -> RemoteClusterConnectionStatus.RECONNECTED));
452439
} else {
440+
if (crossProjectEnabled == false) {
441+
remote.setSkipUnavailable(config.skipUnavailable());
442+
}
453443
// No changes to connection configuration.
454444
listener.onResponse(RemoteClusterConnectionStatus.UNCHANGED);
455445
}

‎server/src/test/java/org/elasticsearch/transport/ClusterSettingsLinkedProjectConfigServiceTests.java‎

Lines changed: 9 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
package org.elasticsearch.transport;
1111

12-
import org.elasticsearch.cluster.metadata.ProjectId;
1312
import org.elasticsearch.cluster.project.DefaultProjectResolver;
1413
import org.elasticsearch.common.settings.ClusterSettings;
1514
import org.elasticsearch.common.settings.Settings;
@@ -25,35 +24,16 @@
2524

2625
public class ClusterSettingsLinkedProjectConfigServiceTests extends ESTestCase {
2726

28-
private record SkipUnavailableUpdate(
29-
ProjectId originProjectId,
30-
ProjectId linkedProjectId,
31-
String linkedProjectAlias,
32-
boolean skipUnavailable
33-
) {}
34-
3527
private static class StubLinkedProjectConfigListener implements LinkedProjectConfigListener {
3628
LinkedProjectConfig updatedConfig;
37-
SkipUnavailableUpdate skipUnavailableUpdate;
3829

3930
@Override
4031
public void updateLinkedProject(LinkedProjectConfig config) {
4132
updatedConfig = config;
4233
}
4334

44-
@Override
45-
public void skipUnavailableChanged(
46-
ProjectId originProjectId,
47-
ProjectId linkedProjectId,
48-
String linkedProjectAlias,
49-
boolean skipUnavailable
50-
) {
51-
skipUnavailableUpdate = new SkipUnavailableUpdate(originProjectId, linkedProjectId, linkedProjectAlias, skipUnavailable);
52-
}
53-
5435
void reset() {
5536
updatedConfig = null;
56-
skipUnavailableUpdate = null;
5737
}
5838
}
5939

@@ -66,17 +46,21 @@ public void testListenersReceiveUpdates() {
6646
final var alias = randomAlphaOfLength(10);
6747

6848
final var initialProxyAddress = "localhost:9400";
49+
final var initialSkipUnavailable = randomBoolean();
6950
final var initialSettings = Settings.builder()
7051
.put("cluster.remote." + alias + ".mode", "proxy")
7152
.put("cluster.remote." + alias + ".proxy_address", initialProxyAddress)
53+
.put("cluster.remote." + alias + ".skip_unavailable", initialSkipUnavailable)
7254
.build();
7355
final var clusterSettings = ClusterSettings.createBuiltInClusterSettings(initialSettings);
7456
final var service = new ClusterSettingsLinkedProjectConfigService(
7557
initialSettings,
7658
clusterSettings,
7759
DefaultProjectResolver.INSTANCE
7860
);
79-
final var config = new ProxyLinkedProjectConfigBuilder(alias).proxyAddress(initialProxyAddress).build();
61+
final var config = new ProxyLinkedProjectConfigBuilder(alias).proxyAddress(initialProxyAddress)
62+
.skipUnavailable(initialSkipUnavailable)
63+
.build();
8064

8165
// Verify we can get the linked projects on startup.
8266
assertThat(service.getInitialLinkedProjectConfigs(), equalTo(List.of(config)));
@@ -92,48 +76,23 @@ public void testListenersReceiveUpdates() {
9276
clusterSettings.applySettings(initialSettings);
9377
for (int i = 0; i < numListeners; ++i) {
9478
assertThat(listeners.get(i).updatedConfig, sameInstance(null));
95-
assertThat(listeners.get(i).skipUnavailableUpdate, sameInstance(null));
9679
listeners.get(i).reset();
9780
}
9881

99-
// Change the skip_unavailable, leave the other settings alone, we should get the skip_unavailable update only.
100-
var expectedSkipUnavailableUpdate = new SkipUnavailableUpdate(
101-
config.originProjectId(),
102-
config.linkedProjectId(),
103-
config.linkedProjectAlias(),
104-
config.skipUnavailable() == false
105-
);
106-
clusterSettings.applySettings(
107-
Settings.builder()
108-
.put(initialSettings)
109-
.put("cluster.remote." + alias + ".skip_unavailable", expectedSkipUnavailableUpdate.skipUnavailable)
110-
.build()
111-
);
112-
for (int i = 0; i < numListeners; ++i) {
113-
assertThat(listeners.get(i).updatedConfig, sameInstance(null));
114-
assertThat(listeners.get(i).skipUnavailableUpdate, equalTo(expectedSkipUnavailableUpdate));
115-
listeners.get(i).reset();
116-
}
117-
118-
// Change the proxy address, and set skip_unavailable back to original value, we should get both updates.
119-
expectedSkipUnavailableUpdate = new SkipUnavailableUpdate(
120-
config.originProjectId(),
121-
config.linkedProjectId(),
122-
config.linkedProjectAlias(),
123-
config.skipUnavailable()
124-
);
82+
// Change the proxy address and skip_unavailable values.
12583
final var newProxyAddress = "localhost:9401";
84+
final var newSkipUnavailable = config.skipUnavailable() == false;
12685
clusterSettings.applySettings(
12786
Settings.builder()
12887
.put(initialSettings)
12988
.put("cluster.remote." + alias + ".proxy_address", newProxyAddress)
130-
.put("cluster.remote." + alias + ".skip_unavailable", expectedSkipUnavailableUpdate.skipUnavailable)
89+
.put("cluster.remote." + alias + ".skip_unavailable", newSkipUnavailable)
13190
.build()
13291
);
13392
for (int i = 0; i < numListeners; ++i) {
13493
assertNotNull("expected non-null updatedConfig for listener " + i, listeners.get(i).updatedConfig);
13594
assertThat(listeners.get(i).updatedConfig.proxyAddress(), equalTo(newProxyAddress));
136-
assertThat(listeners.get(i).skipUnavailableUpdate, equalTo(expectedSkipUnavailableUpdate));
95+
assertThat(listeners.get(i).updatedConfig.skipUnavailable(), equalTo(newSkipUnavailable));
13796
}
13897
}
13998
}

‎server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import java.util.concurrent.CountDownLatch;
5252
import java.util.concurrent.TimeUnit;
5353
import java.util.concurrent.atomic.AtomicReference;
54+
import java.util.function.BiConsumer;
5455
import java.util.function.BiFunction;
5556

5657
import static org.elasticsearch.test.MockLog.assertThatLogger;
@@ -111,6 +112,10 @@ private MockTransportService startTransport(
111112
return RemoteClusterConnectionTests.startTransport(id, knownNodes, version, transportVersion, threadPool, settings);
112113
}
113114

115+
private MockTransportService startTransport(final String id) {
116+
return startTransport(id, List.of(), VersionInformation.CURRENT, TransportVersion.current(), Settings.EMPTY);
117+
}
118+
114119
public void testSettingsAreRegistered() {
115120
assertTrue(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS.contains(RemoteClusterSettings.REMOTE_CLUSTER_SKIP_UNAVAILABLE));
116121
assertTrue(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS.contains(RemoteClusterSettings.REMOTE_INITIAL_CONNECTION_TIMEOUT_SETTING));
@@ -1828,6 +1833,48 @@ public void testLogsConnectionResult() throws IOException {
18281833
}
18291834
}
18301835

1836+
public void testSetSkipUnavailable() throws IOException {
1837+
final var skipUnavailableProperty = RemoteClusterSettings.REMOTE_CLUSTER_SKIP_UNAVAILABLE.getConcreteSettingForNamespace("remote")
1838+
.getKey();
1839+
final var seedNodeProperty = SniffConnectionStrategySettings.REMOTE_CLUSTER_SEEDS.getConcreteSettingForNamespace("remote").getKey();
1840+
final var clusterSettings = ClusterSettings.createBuiltInClusterSettings();
1841+
1842+
try (
1843+
var remote1Transport = startTransport("remote1");
1844+
var remote2Transport = startTransport("remote2");
1845+
var local = startTransport("local");
1846+
var remoteClusterService = createRemoteClusterService(Settings.EMPTY, clusterSettings, local)
1847+
) {
1848+
linkedProjectConfigService.register(remoteClusterService);
1849+
1850+
final BiConsumer<Boolean, MockTransportService> applySettingsAndVerify = (skipUnavailableValue, mockTransportService) -> {
1851+
clusterSettings.applySettings(
1852+
Settings.builder()
1853+
.put(skipUnavailableProperty, skipUnavailableValue)
1854+
.put(seedNodeProperty, mockTransportService.getLocalNode().getAddress().toString())
1855+
.build()
1856+
);
1857+
assertTrue(isRemoteClusterRegistered(remoteClusterService, "remote"));
1858+
assertEquals(skipUnavailableValue, remoteClusterService.isSkipUnavailable("remote").orElseThrow());
1859+
};
1860+
1861+
// Apply the initial settings and verify the new connection is built.
1862+
var skipUnavailable = randomBoolean();
1863+
applySettingsAndVerify.accept(skipUnavailable, remote1Transport);
1864+
1865+
// Change skip_unavailable value, but not seed node, connection should not be rebuilt, but skip_unavailable should be modified.
1866+
skipUnavailable = skipUnavailable == false;
1867+
applySettingsAndVerify.accept(skipUnavailable, remote1Transport);
1868+
1869+
// Change the seed node but not skip_unavailable, connection should be rebuilt and skip_unavailable should stay the same.
1870+
applySettingsAndVerify.accept(skipUnavailable, remote2Transport);
1871+
1872+
// Change skip_unavailable value and the seed node, connection should be rebuilt and skip_unavailable should also be modified.
1873+
skipUnavailable = skipUnavailable == false;
1874+
applySettingsAndVerify.accept(skipUnavailable, remote1Transport);
1875+
}
1876+
}
1877+
18311878
private void updateRemoteCluster(
18321879
RemoteClusterService service,
18331880
String alias,

0 commit comments

Comments
 (0)