You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/07/29 07:22:58 UTC

[GitHub] [lucene-solr] janhoy opened a new pull request #1701: SOLR-14671: Parsing dynamic ZK config sometimes cause NuberFormatException

janhoy opened a new pull request #1701:
URL: https://github.com/apache/lucene-solr/pull/1701


   See https://issues.apache.org/jira/browse/SOLR-14671


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] henrik242 commented on pull request #1701: SOLR-14671: Parsing dynamic ZK config sometimes cause NuberFormatException

Posted by GitBox <gi...@apache.org>.
henrik242 commented on pull request #1701:
URL: https://github.com/apache/lucene-solr/pull/1701#issuecomment-664775724


   By the way, should the rest of the integer parsers (leaderPort etc) have similar null safety checks?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] HoustonPutman commented on a change in pull request #1701: SOLR-14671: Parsing dynamic ZK config sometimes cause NuberFormatException

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #1701:
URL: https://github.com/apache/lucene-solr/pull/1701#discussion_r461777172



##########
File path: solr/core/src/java/org/apache/solr/handler/admin/ZookeeperStatusHandler.java
##########
@@ -112,15 +112,21 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
           .map(h -> h.resolveClientPortAddress() + ":" + h.clientPort)
           .sorted().collect(Collectors.toList());
       List<String> dynamicHosts = zkDynamicConfig.getServers().stream()
-          .map(h -> h.resolveClientPortAddress() + ":" + h.clientPort)
+          .map(h -> h.resolveClientPortAddress() + ":" + (h.clientPort != null ? h.clientPort : "2181"))

Review comment:
       Maybe instead of defaulting to `2181`, we default to `hostsFromConnectionString.getServers().get(0).clientPort`? I imagine that would match a vast majority of scenarios.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] janhoy merged pull request #1701: SOLR-14671: Parsing dynamic ZK config sometimes cause NuberFormatException

Posted by GitBox <gi...@apache.org>.
janhoy merged pull request #1701:
URL: https://github.com/apache/lucene-solr/pull/1701


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] janhoy commented on pull request #1701: SOLR-14671: Parsing dynamic ZK config sometimes cause NuberFormatException

Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #1701:
URL: https://github.com/apache/lucene-solr/pull/1701#issuecomment-664940160






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] janhoy commented on a change in pull request #1701: SOLR-14671: Parsing dynamic ZK config sometimes cause NuberFormatException

Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #1701:
URL: https://github.com/apache/lucene-solr/pull/1701#discussion_r462128930



##########
File path: solr/core/src/java/org/apache/solr/handler/admin/ZookeeperStatusHandler.java
##########
@@ -112,15 +112,21 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
           .map(h -> h.resolveClientPortAddress() + ":" + h.clientPort)
           .sorted().collect(Collectors.toList());
       List<String> dynamicHosts = zkDynamicConfig.getServers().stream()
-          .map(h -> h.resolveClientPortAddress() + ":" + h.clientPort)
+          .map(h -> h.resolveClientPortAddress() + ":" + (h.clientPort != null ? h.clientPort : "2181"))

Review comment:
       Agree, I'll do that change and merge




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] janhoy commented on a change in pull request #1701: SOLR-14671: Parsing dynamic ZK config sometimes cause NuberFormatException

Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #1701:
URL: https://github.com/apache/lucene-solr/pull/1701#discussion_r461461015



##########
File path: solr/core/src/java/org/apache/solr/handler/admin/ZookeeperStatusHandler.java
##########
@@ -112,15 +112,21 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
           .map(h -> h.resolveClientPortAddress() + ":" + h.clientPort)
           .sorted().collect(Collectors.toList());
       List<String> dynamicHosts = zkDynamicConfig.getServers().stream()
-          .map(h -> h.resolveClientPortAddress() + ":" + h.clientPort)
+          .map(h -> h.resolveClientPortAddress() + ":" + (h.clientPort != null ? h.clientPort : "2181"))

Review comment:
       This is not bullet proof if e.g. user has `clientPort=1234` in `zoo.cfg` and in zkHost connection string. Then we'll add a warning that dynamic config differs from zkHost, which is not entirely true since we just lack the port part. We have no way from client to read the `clientPort` from server except from connecting to the server with 4LW *ont the clientPort* which is a chicken and egg. This hack will make the comparison work for default port, which is a compromise.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] henrik242 commented on a change in pull request #1701: SOLR-14671: Parsing dynamic ZK config sometimes cause NuberFormatException

Posted by GitBox <gi...@apache.org>.
henrik242 commented on a change in pull request #1701:
URL: https://github.com/apache/lucene-solr/pull/1701#discussion_r462134434



##########
File path: solr/CHANGES.txt
##########
@@ -164,6 +164,13 @@ Other Changes
 
 * SOLR-11868: Deprecate CloudSolrClient.setIdField, use information from Zookeeper (Erick Erickson)
 
+==================  8.6.1 ==================
+
+Bug Fixes
+---------------------
+
+* SOLR-14671: Parsing dynamic ZK config sometimes cause NuberFormatException (janhoy)

Review comment:
       @janhoy Typo in Nu[m]berFormatException




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org