You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by is...@apache.org on 2023/01/17 12:42:32 UTC

[solr] branch branch_9_1 updated: SOLR-16622, SOLR-16414: simplified code / bugfix for a node restart not working correctly

This is an automated email from the ASF dual-hosted git repository.

ishan pushed a commit to branch branch_9_1
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/branch_9_1 by this push:
     new deb9e243995 SOLR-16622, SOLR-16414: simplified code / bugfix for a node restart not working correctly
deb9e243995 is described below

commit deb9e2439957dd42c349fc7fe2529d9fb63bd54d
Author: Ishan Chattopadhyaya <is...@searchscale.com>
AuthorDate: Tue Jan 17 18:10:01 2023 +0530

    SOLR-16622, SOLR-16414: simplified code / bugfix for a node restart not working correctly
---
 .../java/org/apache/solr/cloud/ZkController.java   | 50 ++++++++--------------
 1 file changed, 19 insertions(+), 31 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
index 6d20f57a42a..696ea7c900e 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -2937,40 +2937,28 @@ public class ZkController implements Closeable {
     } else {
       try {
         // Create a concurrently accessible set to avoid repeating collections
-        Set<String> collectionsInThisNode = new HashSet<>();
+        Set<String> processedCollections = new HashSet<>();
         for (CoreDescriptor cd : cc.getCoreDescriptors()) {
-          if (cd.getCloudDescriptor() != null
-              && cd.getCloudDescriptor().getCollectionName() != null) {
-            collectionsInThisNode.add(cd.getCloudDescriptor().getCollectionName());
+          String collName = cd.getCollectionName();
+          DocCollection coll;
+          if (collName != null
+              && processedCollections.add(collName)
+              && (coll = zkStateReader.getCollection(collName)) != null
+              && coll.isPerReplicaState()) {
+            final List<String> replicasToDown = new ArrayList<>();
+            coll.forEachReplica(
+                (s, replica) -> {
+                  if (replica.getNodeName().equals(nodeName)) {
+                    replicasToDown.add(replica.getName());
+                  }
+                });
+            PerReplicaStatesOps.downReplicas(
+                    replicasToDown,
+                    PerReplicaStatesFetcher.fetch(
+                        coll.getZNode(), zkClient, coll.getPerReplicaStates()))
+                .persist(coll.getZNode(), zkClient);
           }
         }
-        collectionsInThisNode.forEach(
-            c -> {
-              final List<String> replicasToDown = new ArrayList<>();
-              DocCollection coll = zkStateReader.getCollection(c);
-              if (coll == null) {
-                // may be the collection no more exists
-                return;
-              }
-              coll.forEachReplica(
-                  (s, r) -> {
-                    if (r.getNodeName().equals(nodeName)) {
-                      replicasToDown.add(r.getName());
-                    }
-                  });
-
-              if (!replicasToDown.isEmpty()) {
-                try {
-                  PerReplicaStatesOps.downReplicas(
-                          replicasToDown,
-                          PerReplicaStatesFetcher.fetch(
-                              coll.getZNode(), zkClient, coll.getPerReplicaStates()))
-                      .persist(coll.getZNode(), zkClient);
-                } catch (KeeperException | InterruptedException e) {
-                  throw new RuntimeException(e);
-                }
-              }
-            });
 
         // We always send a down node event to overseer to be safe, but overseer will not need to do
         // anything for PRS collections


Re: [solr] branch branch_9_1 updated: SOLR-16622, SOLR-16414: simplified code / bugfix for a node restart not working correctly

Posted by Ishan Chattopadhyaya <ic...@gmail.com>.
Answered your question here: https://issues.apache.org/jira/browse/SOLR-16622

On Tue, Jan 17, 2023 at 7:57 PM Kevin Risden <kr...@apache.org> wrote:
>
> Why was this pushed directly to the release branch with no additional tests?
>
> Why is this ONLY pushed to branch_9_1 and not main or branch_9x first?
>
> https://issues.apache.org/jira/browse/SOLR-16414 was pushed to main and
> branch_9x.
>
> Kevin Risden
>
>
> On Tue, Jan 17, 2023 at 7:42 AM <is...@apache.org> wrote:
>
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > ishan pushed a commit to branch branch_9_1
> > in repository https://gitbox.apache.org/repos/asf/solr.git
> >
> >
> > The following commit(s) were added to refs/heads/branch_9_1 by this push:
> >      new deb9e243995 SOLR-16622, SOLR-16414: simplified code / bugfix for
> > a node restart not working correctly
> > deb9e243995 is described below
> >
> > commit deb9e2439957dd42c349fc7fe2529d9fb63bd54d
> > Author: Ishan Chattopadhyaya <is...@searchscale.com>
> > AuthorDate: Tue Jan 17 18:10:01 2023 +0530
> >
> >     SOLR-16622, SOLR-16414: simplified code / bugfix for a node restart
> > not working correctly
> > ---
> >  .../java/org/apache/solr/cloud/ZkController.java   | 50
> > ++++++++--------------
> >  1 file changed, 19 insertions(+), 31 deletions(-)
> >
> > diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
> > b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
> > index 6d20f57a42a..696ea7c900e 100644
> > --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
> > +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
> > @@ -2937,40 +2937,28 @@ public class ZkController implements Closeable {
> >      } else {
> >        try {
> >          // Create a concurrently accessible set to avoid repeating
> > collections
> > -        Set<String> collectionsInThisNode = new HashSet<>();
> > +        Set<String> processedCollections = new HashSet<>();
> >          for (CoreDescriptor cd : cc.getCoreDescriptors()) {
> > -          if (cd.getCloudDescriptor() != null
> > -              && cd.getCloudDescriptor().getCollectionName() != null) {
> > -
> > collectionsInThisNode.add(cd.getCloudDescriptor().getCollectionName());
> > +          String collName = cd.getCollectionName();
> > +          DocCollection coll;
> > +          if (collName != null
> > +              && processedCollections.add(collName)
> > +              && (coll = zkStateReader.getCollection(collName)) != null
> > +              && coll.isPerReplicaState()) {
> > +            final List<String> replicasToDown = new ArrayList<>();
> > +            coll.forEachReplica(
> > +                (s, replica) -> {
> > +                  if (replica.getNodeName().equals(nodeName)) {
> > +                    replicasToDown.add(replica.getName());
> > +                  }
> > +                });
> > +            PerReplicaStatesOps.downReplicas(
> > +                    replicasToDown,
> > +                    PerReplicaStatesFetcher.fetch(
> > +                        coll.getZNode(), zkClient,
> > coll.getPerReplicaStates()))
> > +                .persist(coll.getZNode(), zkClient);
> >            }
> >          }
> > -        collectionsInThisNode.forEach(
> > -            c -> {
> > -              final List<String> replicasToDown = new ArrayList<>();
> > -              DocCollection coll = zkStateReader.getCollection(c);
> > -              if (coll == null) {
> > -                // may be the collection no more exists
> > -                return;
> > -              }
> > -              coll.forEachReplica(
> > -                  (s, r) -> {
> > -                    if (r.getNodeName().equals(nodeName)) {
> > -                      replicasToDown.add(r.getName());
> > -                    }
> > -                  });
> > -
> > -              if (!replicasToDown.isEmpty()) {
> > -                try {
> > -                  PerReplicaStatesOps.downReplicas(
> > -                          replicasToDown,
> > -                          PerReplicaStatesFetcher.fetch(
> > -                              coll.getZNode(), zkClient,
> > coll.getPerReplicaStates()))
> > -                      .persist(coll.getZNode(), zkClient);
> > -                } catch (KeeperException | InterruptedException e) {
> > -                  throw new RuntimeException(e);
> > -                }
> > -              }
> > -            });
> >
> >          // We always send a down node event to overseer to be safe, but
> > overseer will not need to do
> >          // anything for PRS collections
> >
> >

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@solr.apache.org
For additional commands, e-mail: dev-help@solr.apache.org


Re: [solr] branch branch_9_1 updated: SOLR-16622, SOLR-16414: simplified code / bugfix for a node restart not working correctly

Posted by Kevin Risden <kr...@apache.org>.
Why was this pushed directly to the release branch with no additional tests?

Why is this ONLY pushed to branch_9_1 and not main or branch_9x first?

https://issues.apache.org/jira/browse/SOLR-16414 was pushed to main and
branch_9x.

Kevin Risden


On Tue, Jan 17, 2023 at 7:42 AM <is...@apache.org> wrote:

> This is an automated email from the ASF dual-hosted git repository.
>
> ishan pushed a commit to branch branch_9_1
> in repository https://gitbox.apache.org/repos/asf/solr.git
>
>
> The following commit(s) were added to refs/heads/branch_9_1 by this push:
>      new deb9e243995 SOLR-16622, SOLR-16414: simplified code / bugfix for
> a node restart not working correctly
> deb9e243995 is described below
>
> commit deb9e2439957dd42c349fc7fe2529d9fb63bd54d
> Author: Ishan Chattopadhyaya <is...@searchscale.com>
> AuthorDate: Tue Jan 17 18:10:01 2023 +0530
>
>     SOLR-16622, SOLR-16414: simplified code / bugfix for a node restart
> not working correctly
> ---
>  .../java/org/apache/solr/cloud/ZkController.java   | 50
> ++++++++--------------
>  1 file changed, 19 insertions(+), 31 deletions(-)
>
> diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
> b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
> index 6d20f57a42a..696ea7c900e 100644
> --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
> +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
> @@ -2937,40 +2937,28 @@ public class ZkController implements Closeable {
>      } else {
>        try {
>          // Create a concurrently accessible set to avoid repeating
> collections
> -        Set<String> collectionsInThisNode = new HashSet<>();
> +        Set<String> processedCollections = new HashSet<>();
>          for (CoreDescriptor cd : cc.getCoreDescriptors()) {
> -          if (cd.getCloudDescriptor() != null
> -              && cd.getCloudDescriptor().getCollectionName() != null) {
> -
> collectionsInThisNode.add(cd.getCloudDescriptor().getCollectionName());
> +          String collName = cd.getCollectionName();
> +          DocCollection coll;
> +          if (collName != null
> +              && processedCollections.add(collName)
> +              && (coll = zkStateReader.getCollection(collName)) != null
> +              && coll.isPerReplicaState()) {
> +            final List<String> replicasToDown = new ArrayList<>();
> +            coll.forEachReplica(
> +                (s, replica) -> {
> +                  if (replica.getNodeName().equals(nodeName)) {
> +                    replicasToDown.add(replica.getName());
> +                  }
> +                });
> +            PerReplicaStatesOps.downReplicas(
> +                    replicasToDown,
> +                    PerReplicaStatesFetcher.fetch(
> +                        coll.getZNode(), zkClient,
> coll.getPerReplicaStates()))
> +                .persist(coll.getZNode(), zkClient);
>            }
>          }
> -        collectionsInThisNode.forEach(
> -            c -> {
> -              final List<String> replicasToDown = new ArrayList<>();
> -              DocCollection coll = zkStateReader.getCollection(c);
> -              if (coll == null) {
> -                // may be the collection no more exists
> -                return;
> -              }
> -              coll.forEachReplica(
> -                  (s, r) -> {
> -                    if (r.getNodeName().equals(nodeName)) {
> -                      replicasToDown.add(r.getName());
> -                    }
> -                  });
> -
> -              if (!replicasToDown.isEmpty()) {
> -                try {
> -                  PerReplicaStatesOps.downReplicas(
> -                          replicasToDown,
> -                          PerReplicaStatesFetcher.fetch(
> -                              coll.getZNode(), zkClient,
> coll.getPerReplicaStates()))
> -                      .persist(coll.getZNode(), zkClient);
> -                } catch (KeeperException | InterruptedException e) {
> -                  throw new RuntimeException(e);
> -                }
> -              }
> -            });
>
>          // We always send a down node event to overseer to be safe, but
> overseer will not need to do
>          // anything for PRS collections
>
>

Re: [solr] branch branch_9_1 updated: SOLR-16622, SOLR-16414: simplified code / bugfix for a node restart not working correctly

Posted by Kevin Risden <kr...@apache.org>.
Why was this pushed directly to the release branch with no additional tests?

Why is this ONLY pushed to branch_9_1 and not main or branch_9x first?

https://issues.apache.org/jira/browse/SOLR-16414 was pushed to main and
branch_9x.

Kevin Risden


On Tue, Jan 17, 2023 at 7:42 AM <is...@apache.org> wrote:

> This is an automated email from the ASF dual-hosted git repository.
>
> ishan pushed a commit to branch branch_9_1
> in repository https://gitbox.apache.org/repos/asf/solr.git
>
>
> The following commit(s) were added to refs/heads/branch_9_1 by this push:
>      new deb9e243995 SOLR-16622, SOLR-16414: simplified code / bugfix for
> a node restart not working correctly
> deb9e243995 is described below
>
> commit deb9e2439957dd42c349fc7fe2529d9fb63bd54d
> Author: Ishan Chattopadhyaya <is...@searchscale.com>
> AuthorDate: Tue Jan 17 18:10:01 2023 +0530
>
>     SOLR-16622, SOLR-16414: simplified code / bugfix for a node restart
> not working correctly
> ---
>  .../java/org/apache/solr/cloud/ZkController.java   | 50
> ++++++++--------------
>  1 file changed, 19 insertions(+), 31 deletions(-)
>
> diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
> b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
> index 6d20f57a42a..696ea7c900e 100644
> --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
> +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
> @@ -2937,40 +2937,28 @@ public class ZkController implements Closeable {
>      } else {
>        try {
>          // Create a concurrently accessible set to avoid repeating
> collections
> -        Set<String> collectionsInThisNode = new HashSet<>();
> +        Set<String> processedCollections = new HashSet<>();
>          for (CoreDescriptor cd : cc.getCoreDescriptors()) {
> -          if (cd.getCloudDescriptor() != null
> -              && cd.getCloudDescriptor().getCollectionName() != null) {
> -
> collectionsInThisNode.add(cd.getCloudDescriptor().getCollectionName());
> +          String collName = cd.getCollectionName();
> +          DocCollection coll;
> +          if (collName != null
> +              && processedCollections.add(collName)
> +              && (coll = zkStateReader.getCollection(collName)) != null
> +              && coll.isPerReplicaState()) {
> +            final List<String> replicasToDown = new ArrayList<>();
> +            coll.forEachReplica(
> +                (s, replica) -> {
> +                  if (replica.getNodeName().equals(nodeName)) {
> +                    replicasToDown.add(replica.getName());
> +                  }
> +                });
> +            PerReplicaStatesOps.downReplicas(
> +                    replicasToDown,
> +                    PerReplicaStatesFetcher.fetch(
> +                        coll.getZNode(), zkClient,
> coll.getPerReplicaStates()))
> +                .persist(coll.getZNode(), zkClient);
>            }
>          }
> -        collectionsInThisNode.forEach(
> -            c -> {
> -              final List<String> replicasToDown = new ArrayList<>();
> -              DocCollection coll = zkStateReader.getCollection(c);
> -              if (coll == null) {
> -                // may be the collection no more exists
> -                return;
> -              }
> -              coll.forEachReplica(
> -                  (s, r) -> {
> -                    if (r.getNodeName().equals(nodeName)) {
> -                      replicasToDown.add(r.getName());
> -                    }
> -                  });
> -
> -              if (!replicasToDown.isEmpty()) {
> -                try {
> -                  PerReplicaStatesOps.downReplicas(
> -                          replicasToDown,
> -                          PerReplicaStatesFetcher.fetch(
> -                              coll.getZNode(), zkClient,
> coll.getPerReplicaStates()))
> -                      .persist(coll.getZNode(), zkClient);
> -                } catch (KeeperException | InterruptedException e) {
> -                  throw new RuntimeException(e);
> -                }
> -              }
> -            });
>
>          // We always send a down node event to overseer to be safe, but
> overseer will not need to do
>          // anything for PRS collections
>
>