[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15397339#comment-15397339 ] ASF subversion and git services commented on SOLR-9106: --- Commit c2db9fae2cc312a13a66e6dab9989ed65738fe02 in lucene-solr's branch refs/heads/master from [~romseygeek] [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c2db9fa ] SOLR-9106: Add javadocs to ZkStateReader cluster properties methods > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: 6.1 > > Attachments: SOLR-9106.patch, SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15397338#comment-15397338 ] ASF subversion and git services commented on SOLR-9106: --- Commit 8247f9f0cd8fccf7ecfb6d76cb8f180079ece3bb in lucene-solr's branch refs/heads/branch_6x from [~romseygeek] [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8247f9f ] SOLR-9106: Add javadocs to ZkStateReader cluster properties methods > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: 6.1 > > Attachments: SOLR-9106.patch, SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15386009#comment-15386009 ] Alan Woodward commented on SOLR-9106: - bq. If we read the data just in time we always get the latest data If you need this behaviour, you can use an explicit ClusterProperties object. ZkCLI and some tests already do this. I'll add some javadocs to ZkStateReader.getClusterProperty() to make this more explicit though. > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: 6.1 > > Attachments: SOLR-9106.patch, SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15381858#comment-15381858 ] Noble Paul commented on SOLR-9106: -- bqThis seems exactly backwards to me? Watches are pretty lightweight. Do you have some numbers here The problem with watches is that we really don't know when the changes are propagated to each node. If we read the data just in time we always get the latest data. Watching is light weight, I haven't done any benchmarking yet. > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: 6.1 > > Attachments: SOLR-9106.patch, SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15381851#comment-15381851 ] Alan Woodward commented on SOLR-9106: - bq. Not watching clusterprops was done deliberately. it is read rarely and watching is expensive. This seems exactly backwards to me? Watches are pretty lightweight. Do you have some numbers here? If it's true, then it needs to be explicitly documented on the .getClusterProperties() method that it will always read data from ZK - maybe it should be called .readClusterProperties() instead. And it ends up being used in some expected places, for example during admin requests from a CloudSolrClient. > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: 6.1 > > Attachments: SOLR-9106.patch, SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15376668#comment-15376668 ] Noble Paul commented on SOLR-9106: -- [~varunthacker] the problem is, this ticket introduced a new problem where there was none. As [~hgadre] said reading that value once in a while was not a performance issue. Now we are paying the price for constantly watching it and we still don't have up to date data. > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: 6.1 > > Attachments: SOLR-9106.patch, SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15376659#comment-15376659 ] Varun Thacker commented on SOLR-9106: - I think its okay to cache it . We should maybe add some documentation that {{getClusterProperty}} might not always have a latest value if an update has just taken place and that in scenarios where we want the latest value we should forceRefresh it before reading. So things like backup/restore can forceRefresh it since its a user facing API where the user can set some values in the cluster property file and expect it to be there. Things like the OverseerAutoReplicaFailoverThread where the feature might kick in after a couple of seconds because of this caching is fine. > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: 6.1 > > Attachments: SOLR-9106.patch, SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15376118#comment-15376118 ] Hrishikesh Gadre commented on SOLR-9106: We found a scenario where the ZK watch was not fired in a timely fashion resulting in the unit test failure (SOLR-9242). The changes made to fix that failure are exactly opposite to the intent of this JIRA (i.e. it brings back the old behavior). https://github.com/apache/lucene-solr/commit/eefdc62c997f7936b6db203111d8149dc934b243 I agree with Nobel here. [~romseygeek] is reading cluster properties every-time a serious performance problem? CC [~varunthacker] > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: 6.1 > > Attachments: SOLR-9106.patch, SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15370348#comment-15370348 ] Noble Paul commented on SOLR-9106: -- Hi [~romseygeek] I missed this. Not watching clusterprops was done deliberately. it is read rarely and watching is expensive. > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Fix For: 6.1 > > Attachments: SOLR-9106.patch, SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15283502#comment-15283502 ] ASF subversion and git services commented on SOLR-9106: --- Commit dd23fa40153214095527cdeb77c4255cafbc21f9 in lucene-solr's branch refs/heads/branch_6x from [~romseygeek] [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=dd23fa4 ] SOLR-9106: Cache cluster properties on ZkStateReader > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Attachments: SOLR-9106.patch, SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15283500#comment-15283500 ] ASF subversion and git services commented on SOLR-9106: --- Commit 77962f4af4e7f376255954f00e8b97f3dff94396 in lucene-solr's branch refs/heads/master from [~romseygeek] [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=77962f4 ] SOLR-9106: Cache cluster properties on ZkStateReader > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Attachments: SOLR-9106.patch, SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15283167#comment-15283167 ] Scott Blum commented on SOLR-9106: -- LGTM > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Attachments: SOLR-9106.patch, SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15282497#comment-15282497 ] Varun Thacker commented on SOLR-9106: - bq. that's dealt with by the call to SolrZkClient.checkInterrupted() True! I obviously didn't read the LOG.error line . > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Attachments: SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15282221#comment-15282221 ] Alan Woodward commented on SOLR-9106: - [~dragonsinth] yeah, you're right, that's pretty ugly. And it's also wrong, as legacyCloud defaults to 'true' at the moment. Will fix! [~varunthacker] that's dealt with by the call to SolrZkClient.checkInterrupted() > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Attachments: SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15282140#comment-15282140 ] Varun Thacker commented on SOLR-9106: - Hi Alan, There is one common catch block now {{catch (KeeperException | InterruptedException e) {}} Should we handle them separately as we are currently doing so that we can set the interrupt flag? > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Attachments: SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9106) Cache cluster properties in ZkStateReader
[ https://issues.apache.org/jira/browse/SOLR-9106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15281863#comment-15281863 ] Scott Blum commented on SOLR-9106: -- LGTM if tests pass. One comment: {code} - public static boolean isLegacy(Map clusterProps) { -return !"false".equals(clusterProps.get(ZkStateReader.LEGACY_CLOUD)); + public static boolean isLegacy(ZkStateReader stateReader) { +return stateReader.getClusterProperty(ZkStateReader.LEGACY_CLOUD, "false").equals("false") == false; } {code} I think !"false".equals(stateReader.getClusterProperty(ZkStateReader.LEGACY_CLOUD, "false")) would be slightly more readable more like the original. That whole "false").equals("false") == false bit makes my head spin LOL. > Cache cluster properties in ZkStateReader > - > > Key: SOLR-9106 > URL: https://issues.apache.org/jira/browse/SOLR-9106 > Project: Solr > Issue Type: Improvement >Affects Versions: master (7.0) >Reporter: Alan Woodward >Assignee: Alan Woodward > Attachments: SOLR-9106.patch > > > ZkStateReader currently makes calls into ZK every time getClusterProps() is > called. Instead we should keep the data locally and use a Watcher to update > it. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org