Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SOLR-17535: Deprecate ClusterState.forEachCollection #2854

Merged
merged 1 commit into from
Nov 12, 2024
Merged

SOLR-17535: Deprecate ClusterState.forEachCollection #2854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion solr/CHANGES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ led to the suppression of exceptions. (Andrey Bozhko)

* SOLR-17534: Introduce ClusterState.getCollectionNames, a convenience method (David Smiley)

* SOLR-17535: Introduce ClusterState.collectionStream to replace getCollectionStates and getCollectionsMap (David Smiley)
* SOLR-17535: Introduce ClusterState.collectionStream to replace getCollectionStates, getCollectionsMap,
and forEachCollection, which are now deprecated. (David Smiley)

* SOLR-17545: Upgrade to Gradle 8.10 (Houston Putman)

Expand Down
71 changes: 38 additions & 33 deletions solr/core/src/java/org/apache/solr/cluster/events/impl/CollectionsRepairEventListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.apache.solr.cluster.events.ClusterEvent;
import org.apache.solr.cluster.events.ClusterEventListener;
import org.apache.solr.cluster.events.NodesDownEvent;
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.ReplicaCount;
import org.apache.solr.common.cloud.ReplicaPosition;
import org.apache.solr.common.util.SolrNamedThreadFactory;
Expand Down Expand Up @@ -168,38 +167,44 @@ private void runRepair() {
// collection / positions
Map<String, List<ReplicaPosition>> newPositions = new HashMap<>();
try {
ClusterState clusterState = solrCloudManager.getClusterState();
clusterState.forEachCollection(
coll -> {
// shard / number of replicas per type
Map<String, ReplicaCount> lostReplicas = new HashMap<>();
coll.forEachReplica(
(shard, replica) -> {
if (reallyLostNodes.contains(replica.getNodeName())) {
lostReplicas
.computeIfAbsent(shard, s -> ReplicaCount.empty())
.increment(replica.type);
}
});
Assign.AssignStrategy assignStrategy = Assign.createAssignStrategy(cc);
lostReplicas.forEach(
(shard, types) -> {
Assign.AssignRequest assignRequest =
new Assign.AssignRequestBuilder()
.forCollection(coll.getName())
.forShard(Collections.singletonList(shard))
.assignReplicas(types)
.build();
try {
List<ReplicaPosition> positions =
assignStrategy.assign(solrCloudManager, assignRequest);
newPositions.put(coll.getName(), positions);
} catch (Exception e) {
log.warn(
"Exception computing positions for {}/{}: {}", coll.getName(), shard, e);
}
});
});
// shard / number of replicas per type
solrCloudManager
.getClusterState()
.collectionStream()
.forEach(
coll -> {
// shard / number of replicas per type
Map<String, ReplicaCount> lostReplicas = new HashMap<>();
coll.forEachReplica(
(shard, replica) -> {
if (reallyLostNodes.contains(replica.getNodeName())) {
lostReplicas
.computeIfAbsent(shard, s -> ReplicaCount.empty())
.increment(replica.type);
}
});
Assign.AssignStrategy assignStrategy = Assign.createAssignStrategy(cc);
lostReplicas.forEach(
(shard, types) -> {
Assign.AssignRequest assignRequest =
new Assign.AssignRequestBuilder()
.forCollection(coll.getName())
.forShard(Collections.singletonList(shard))
.assignReplicas(types)
.build();
try {
List<ReplicaPosition> positions =
assignStrategy.assign(solrCloudManager, assignRequest);
newPositions.put(coll.getName(), positions);
} catch (Exception e) {
log.warn(
"Exception computing positions for {}/{}: {}",
coll.getName(),
shard,
e);
}
});
});
} catch (IOException e) {
log.warn("Exception getting cluster state", e);
return;
Expand Down
3 changes: 2 additions & 1 deletion solr/core/src/test/org/apache/solr/cloud/ReindexCollectionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,8 @@ public void testFailure() throws Exception {
// verify that the target and checkpoint collections don't exist
cloudManager
.getClusterState()
.forEachCollection(
.collectionStream()
.forEach(
coll -> {
assertFalse(
coll.getName() + " still exists",
Expand Down
27 changes: 15 additions & 12 deletions ...lrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,21 @@ protected void readReplicaDetails() throws IOException {
if (clusterState == null) { // zkStateReader still initializing
return;
}
clusterState.forEachCollection(
coll ->
coll.forEachReplica(
(shard, replica) -> {
Map<String, Map<String, List<Replica>>> nodeData =
nodeVsCollectionVsShardVsReplicaInfo.computeIfAbsent(
replica.getNodeName(), k -> new HashMap<>());
Map<String, List<Replica>> collData =
nodeData.computeIfAbsent(coll.getName(), k -> new HashMap<>());
List<Replica> replicas = collData.computeIfAbsent(shard, k -> new ArrayList<>());
replicas.add((Replica) replica.clone());
}));
clusterState
.collectionStream()
.forEach(
coll ->
coll.forEachReplica(
(shard, replica) -> {
Map<String, Map<String, List<Replica>>> nodeData =
nodeVsCollectionVsShardVsReplicaInfo.computeIfAbsent(
replica.getNodeName(), k -> new HashMap<>());
Map<String, List<Replica>> collData =
nodeData.computeIfAbsent(coll.getName(), k -> new HashMap<>());
List<Replica> replicas =
collData.computeIfAbsent(shard, k -> new ArrayList<>());
replicas.add((Replica) replica.clone());
}));
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions solr/solrj-zookeeper/src/java/org/apache/solr/common/LazySolrCluster.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ public SolrCollection get(String key) {
public void forEachEntry(BiConsumer<String, ? super SolrCollection> fun) {
zkStateReader
.getClusterState()
.forEachCollection(
coll -> fun.accept(coll.getName(), _collection(coll.getName(), coll)));
.collectionStream()
.forEach(coll -> fun.accept(coll.getName(), _collection(coll.getName(), coll)));
}

@Override
Expand Down
3 changes: 3 additions & 0 deletions solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,10 @@ public Stream<DocCollection> collectionStream() {
/**
* Calls {@code consumer} with a resolved {@link DocCollection}s for all collections. Use this
* sparingly in case there are many collections.
*
* @deprecated see {@link #collectionStream()}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this method was introduced a few days ago (can't remember), I think it can be removed rather than deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has existed for some releases, thus I deprecate it.

*/
@Deprecated
public void forEachCollection(Consumer<DocCollection> consumer) {
collectionStream().forEach(consumer);
}
Expand Down
Loading