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/08/21 09:23:26 UTC
[GitHub] [lucene-solr] der-eismann opened a new pull request #1771: SOLR-14752: Fix zk status with prometheus enabled zookeeper
der-eismann opened a new pull request #1771:
URL: https://github.com/apache/lucene-solr/pull/1771
# Description
Solr checks the zookeeper status in the admin UI and explicitly expects integers to be delivered. However if you enable the prometheus plugin in zookeeper, all values are posted as floats instead which breaks the existing check.
# Solution
This is a small fix to parse the values as floats and cast them into an integer. This should be fine since the number of followers can't actually be a float and the part behind the dot is always 0.
# Tests
None. This is my first Java contribution ever, so I'm happy for any clue how to do this.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `master` branch.
- [ ] I have run `ant precommit` and the appropriate test suite.
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only).
----------------------------------------------------------------
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 #1771: SOLR-14752: Fix zk status with prometheus enabled zookeeper
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #1771:
URL: https://github.com/apache/lucene-solr/pull/1771#issuecomment-678276659
Thanks. Please ad a test as well that proves the fix works, e.g. something like this
```
Index: solr/core/src/test/org/apache/solr/handler/admin/ZookeeperStatusHandlerTest.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- solr/core/src/test/org/apache/solr/handler/admin/ZookeeperStatusHandlerTest.java (revision 2b2b9e71d3defe6aa4578f9a571078de8b88c7c6)
+++ solr/core/src/test/org/apache/solr/handler/admin/ZookeeperStatusHandlerTest.java (date 1598014320557)
@@ -184,9 +184,9 @@
when(zkStatusHandler.getZkRawResponse("zoo1:2181", "ruok")).thenReturn(Arrays.asList("imok"));
when(zkStatusHandler.getZkRawResponse("zoo1:2181", "mntr")).thenReturn(
Arrays.asList("zk_version\t3.5.5-390fe37ea45dee01bf87dc1c042b5e3dcce88653, built on 05/03/2019 12:07 GMT",
- "zk_avg_latency\t1",
+ "zk_avg_latency\t1.0",
"zk_server_state\tleader",
- "zk_synced_followers\t2"));
+ "zk_synced_followers\t2.0"));
when(zkStatusHandler.getZkRawResponse("zoo1:2181", "conf")).thenReturn(
Arrays.asList("clientPort=2181"));
when(zkStatusHandler.getZkStatus(anyString(), any())).thenCallRealMethod();
```
Can you also add a line to solr/CHANGES.txt for Solr 8.7?
----------------------------------------------------------------
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] der-eismann commented on pull request #1771: SOLR-14752: Fix zk status with prometheus enabled zookeeper
Posted by GitBox <gi...@apache.org>.
der-eismann commented on pull request #1771:
URL: https://github.com/apache/lucene-solr/pull/1771#issuecomment-679993953
Hey, sorry for the delay. I added a separate test case and a line for the changes in Solr 8.7.0.
----------------------------------------------------------------
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 #1771: SOLR-14752: Fix zk status with prometheus enabled zookeeper
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #1771:
URL: https://github.com/apache/lucene-solr/pull/1771#discussion_r477091492
##########
File path: solr/core/src/test/org/apache/solr/handler/admin/ZookeeperStatusHandlerTest.java
##########
@@ -211,4 +211,39 @@ public void testMntrBugZk36Solr14463() {
" \"status\":\"red\"}";
assertEquals(expected, JSONUtil.toJSON(mockStatus));
}
+
+ @Test
+ public void testZkWithPrometheusSolr14752() {
+ assumeWorkingMockito();
+ ZookeeperStatusHandler zkStatusHandler = mock(ZookeeperStatusHandler.class);
+ when(zkStatusHandler.getZkRawResponse("zoo1:2181", "ruok")).thenReturn(Arrays.asList("imok"));
+ when(zkStatusHandler.getZkRawResponse("zoo1:2181", "mntr")).thenReturn(
+ Arrays.asList("zk_version\t3.6.1--104dcb3e3fb464b30c5186d229e00af9f332524b, built on 04/21/2020 15:01 GMT",
+ "zk_avg_latency\t0.24",
+ "zk_server_state\tleader",
+ "zk_synced_followers\t2.0"));
+ when(zkStatusHandler.getZkRawResponse("zoo1:2181", "conf")).thenReturn(
+ Arrays.asList("clientPort=2181"));
+ when(zkStatusHandler.getZkStatus(anyString(), any())).thenCallRealMethod();
+ when(zkStatusHandler.monitorZookeeper(anyString())).thenCallRealMethod();
+ when(zkStatusHandler.validateZkRawResponse(ArgumentMatchers.any(), any(), any())).thenAnswer(Answers.CALLS_REAL_METHODS);
+
+ Map<String, Object> mockStatus = zkStatusHandler.getZkStatus("zoo1:2181", ZkDynamicConfig.fromZkConnectString("zoo1:2181"));
+ String expected = "{\n" +
Review comment:
This check seems wrong for what we want to test here. Sure, the check succeeds since there is no NumberFormatException anywhere, but it would be better with a smaller test that does not trigger any other errors.
----------------------------------------------------------------
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 closed pull request #1771: SOLR-14752: Fix zk status with prometheus enabled zookeeper
Posted by GitBox <gi...@apache.org>.
janhoy closed pull request #1771:
URL: https://github.com/apache/lucene-solr/pull/1771
----------------------------------------------------------------
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