Skip to content

Commit ca0ea7a

Browse files
ywangdywangd
committed
revert changes to ClusterChangedEvent
1 parent c317b60 commit ca0ea7a

File tree

3 files changed

+36
-89
lines changed

3 files changed

+36
-89
lines changed

‎server/src/main/java/org/elasticsearch/cluster/ClusterChangedEvent.java‎

Lines changed: 16 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import org.elasticsearch.cluster.node.DiscoveryNodes;
1919
import org.elasticsearch.cluster.routing.IndexRoutingTable;
2020
import org.elasticsearch.common.util.set.Sets;
21-
import org.elasticsearch.core.Nullable;
2221
import org.elasticsearch.gateway.GatewayService;
2322
import org.elasticsearch.index.Index;
2423

@@ -243,16 +242,11 @@ public boolean nodesChanged() {
243242
return nodesRemoved() || nodesAdded();
244243
}
245244

246-
public Set<ProjectId> addedProjects() {
247-
return projectsDelta.added();
248-
}
249-
250-
public Set<ProjectId> removedProjects() {
251-
return projectsDelta.removed();
252-
}
253-
254-
public Set<ProjectId> commonProjects() {
255-
return projectsDelta.common(state);
245+
/**
246+
* Returns the {@link ProjectsDelta} between the previous cluster state and the new cluster state.
247+
*/
248+
public ProjectsDelta projectDelta() {
249+
return projectsDelta;
256250
}
257251

258252
/**
@@ -360,62 +354,26 @@ private static ProjectsDelta calculateProjectDelta(Metadata previousMetadata, Me
360354
&& previousMetadata.hasProject(ProjectId.DEFAULT)
361355
&& currentMetadata.projects().size() == 1
362356
&& currentMetadata.hasProject(ProjectId.DEFAULT))) {
363-
return ProjectsDelta.NO_CHANGE;
364-
}
365-
366-
final Set<ProjectId> currentProjectIds = currentMetadata.projects().keySet();
367-
final Set<ProjectId> previousProjectIds = previousMetadata.projects().keySet();
368-
369-
final var added = new HashSet<ProjectId>();
370-
final var common = new HashSet<ProjectId>();
371-
for (var projectId : currentProjectIds) {
372-
if (previousProjectIds.contains(projectId)) {
373-
common.add(projectId);
374-
} else {
375-
added.add(projectId);
376-
}
357+
return ProjectsDelta.EMPTY;
377358
}
378359

379-
final Set<ProjectId> removed = Sets.difference(previousProjectIds, currentProjectIds);
360+
final Set<ProjectId> added = Collections.unmodifiableSet(
361+
Sets.difference(currentMetadata.projects().keySet(), previousMetadata.projects().keySet())
362+
);
363+
final Set<ProjectId> removed = Collections.unmodifiableSet(
364+
Sets.difference(previousMetadata.projects().keySet(), currentMetadata.projects().keySet())
365+
);
380366
// TODO: Enable the following assertions once tests no longer add or remove default projects
381367
// assert added.contains(ProjectId.DEFAULT) == false;
382368
// assert removed.contains(ProjectId.DEFAULT) == false;
383-
384-
if (added.isEmpty() && removed.isEmpty()) {
385-
return ProjectsDelta.NO_CHANGE;
386-
} else {
387-
return new ProjectsDelta(
388-
Collections.unmodifiableSet(added),
389-
Collections.unmodifiableSet(removed),
390-
Collections.unmodifiableSet(common)
391-
);
392-
}
369+
return new ProjectsDelta(added, removed);
393370
}
394371

395-
private record ProjectsDelta(
396-
Set<ProjectId> added,
397-
Set<ProjectId> removed,
398-
@Nullable Set<ProjectId> common // null if all projects are common
399-
) {
400-
401-
private static final ProjectsDelta NO_CHANGE = new ProjectsDelta(Set.of(), Set.of(), null);
372+
public record ProjectsDelta(Set<ProjectId> added, Set<ProjectId> removed) {
373+
private static final ProjectsDelta EMPTY = new ProjectsDelta(Set.of(), Set.of());
402374

403-
private boolean hasNoChange() {
375+
public boolean isEmpty() {
404376
return added.isEmpty() && removed.isEmpty();
405377
}
406-
407-
@Override
408-
public Set<ProjectId> common() {
409-
throw new UnsupportedOperationException("Use common(ClusterState state) instead");
410-
}
411-
412-
Set<ProjectId> common(ClusterState state) {
413-
if (common == null) {
414-
assert hasNoChange() : this;
415-
return state.metadata().projects().keySet();
416-
} else {
417-
return common;
418-
}
419-
}
420378
}
421379
}

‎server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import org.elasticsearch.common.util.concurrent.EsExecutors;
4747
import org.elasticsearch.common.util.concurrent.ListenableFuture;
4848
import org.elasticsearch.common.util.concurrent.ThreadContext;
49+
import org.elasticsearch.common.util.set.Sets;
4950
import org.elasticsearch.core.FixForMultiProject;
5051
import org.elasticsearch.core.IOUtils;
5152
import org.elasticsearch.core.Nullable;
@@ -659,15 +660,19 @@ public void applyClusterState(ClusterChangedEvent event) {
659660
final ClusterState state = event.state();
660661
final ClusterState previousState = event.previousState();
661662

662-
for (var projectId : event.removedProjects()) { // removed projects
663+
for (var projectId : event.projectDelta().removed()) { // removed projects
663664
applyProjectState(state.version(), null, previousState.projectState(projectId));
664665
}
665666

666-
for (var projectId : event.addedProjects()) { // added projects
667+
for (var projectId : event.projectDelta().added()) { // added projects
667668
applyProjectState(state.version(), state.projectState(projectId), null);
668669
}
669670

670-
for (var projectId : event.commonProjects()) { // existing projects
671+
// existing projects
672+
final var common = event.projectDelta().added().isEmpty()
673+
? state.metadata().projects().keySet()
674+
: Sets.difference(state.metadata().projects().keySet(), event.projectDelta().added());
675+
for (var projectId : common) {
671676
applyProjectState(state.version(), state.projectState(projectId), previousState.projectState(projectId));
672677
}
673678
} catch (Exception ex) {

‎server/src/test/java/org/elasticsearch/cluster/ClusterChangedEventTests.java‎

Lines changed: 12 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.elasticsearch.cluster.routing.RoutingTable;
2727
import org.elasticsearch.common.UUIDs;
2828
import org.elasticsearch.common.settings.Settings;
29-
import org.elasticsearch.common.util.set.Sets;
3029
import org.elasticsearch.gateway.GatewayService;
3130
import org.elasticsearch.index.Index;
3231
import org.elasticsearch.index.IndexVersion;
@@ -531,9 +530,7 @@ public void testProjectsDelta() {
531530
.metadata(Metadata.builder(state0.metadata()).put(ReservedStateMetadata.builder("test").build()))
532531
.build();
533532
ClusterChangedEvent event = new ClusterChangedEvent("test", state1, state0);
534-
assertThat(event.addedProjects(), empty());
535-
assertThat(event.removedProjects(), empty());
536-
assertThat(event.commonProjects(), equalTo(Set.of(ProjectId.DEFAULT)));
533+
assertTrue(event.projectDelta().isEmpty());
537534

538535
// Add projects
539536
final List<ProjectId> projectIds = randomList(1, 5, ESTestCase::randomUniqueProjectId);
@@ -543,9 +540,8 @@ public void testProjectsDelta() {
543540
}
544541
final var state2 = ClusterState.builder(state1).metadata(metadataBuilder.build()).build();
545542
event = new ClusterChangedEvent("test", state2, state1);
546-
assertThat(event.addedProjects(), containsInAnyOrder(projectIds.toArray()));
547-
assertThat(event.removedProjects(), empty());
548-
assertThat(event.commonProjects(), equalTo(Set.of(ProjectId.DEFAULT)));
543+
assertThat(event.projectDelta().added(), containsInAnyOrder(projectIds.toArray()));
544+
assertThat(event.projectDelta().removed(), empty());
549545

550546
// Add more projects and delete one
551547
final var removedProjectIds = randomNonEmptySubsetOf(projectIds);
@@ -563,38 +559,26 @@ public void testProjectsDelta() {
563559
final var state3 = ClusterState.builder(state2).metadata(metadataBuilder.build()).routingTable(routingTableBuilder.build()).build();
564560

565561
event = new ClusterChangedEvent("test", state3, state2);
566-
assertThat(event.addedProjects(), containsInAnyOrder(moreProjectIds.toArray()));
567-
assertThat(event.removedProjects(), containsInAnyOrder(removedProjectIds.toArray()));
568-
assertThat(
569-
event.commonProjects(),
570-
equalTo(Sets.union(Sets.difference(Set.copyOf(projectIds), Set.copyOf(removedProjectIds)), Set.of(ProjectId.DEFAULT)))
571-
);
572-
573-
// An update without project membership changes
574-
final var state4 = ClusterState.builder(state3).version(state3.version() + 1).build();
575-
event = new ClusterChangedEvent("test", state4, state3);
576-
assertThat(event.addedProjects(), empty());
577-
assertThat(event.removedProjects(), empty());
578-
assertThat(event.commonProjects(), equalTo(state4.metadata().projects().keySet()));
562+
assertThat(event.projectDelta().added(), containsInAnyOrder(moreProjectIds.toArray()));
563+
assertThat(event.projectDelta().removed(), containsInAnyOrder(removedProjectIds.toArray()));
579564

580565
// Remove all projects
581-
final List<ProjectId> remainingProjects = state4.metadata()
566+
final List<ProjectId> remainingProjects = state3.metadata()
582567
.projects()
583568
.keySet()
584569
.stream()
585570
.filter(projectId -> ProjectId.DEFAULT.equals(projectId) == false)
586571
.toList();
587-
metadataBuilder = Metadata.builder(state4.metadata());
588-
routingTableBuilder = GlobalRoutingTable.builder(state4.globalRoutingTable());
572+
metadataBuilder = Metadata.builder(state3.metadata());
573+
routingTableBuilder = GlobalRoutingTable.builder(state3.globalRoutingTable());
589574
for (ProjectId projectId : remainingProjects) {
590575
metadataBuilder.removeProject(projectId);
591576
routingTableBuilder.removeProject(projectId);
592577
}
593-
final var state5 = ClusterState.builder(state4).metadata(metadataBuilder.build()).routingTable(routingTableBuilder.build()).build();
594-
event = new ClusterChangedEvent("test", state5, state4);
595-
assertThat(event.addedProjects(), empty());
596-
assertThat(event.removedProjects(), containsInAnyOrder(remainingProjects.toArray()));
597-
assertThat(event.commonProjects(), equalTo(Set.of(ProjectId.DEFAULT)));
578+
final var state4 = ClusterState.builder(state3).metadata(metadataBuilder.build()).routingTable(routingTableBuilder.build()).build();
579+
event = new ClusterChangedEvent("test", state4, state3);
580+
assertThat(event.projectDelta().added(), empty());
581+
assertThat(event.projectDelta().removed(), containsInAnyOrder(remainingProjects.toArray()));
598582
}
599583

600584
private static class CustomClusterMetadata2 extends TestClusterCustomMetadata {

0 commit comments

Comments
 (0)