Re: [PR] Skip creation of ZkCoreNodeProps when we have a Replica instance [solr]
psalagnac merged PR #3315: URL: https://github.com/apache/solr/pull/3315 -- 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. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Skip creation of ZkCoreNodeProps when we have a Replica instance [solr]
epugh commented on code in PR #3315:
URL: https://github.com/apache/solr/pull/3315#discussion_r2032915473
##
solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java:
##
@@ -150,12 +149,11 @@ protected void runCloudTool(CloudSolrClient
cloudSolrClient, CommandLine cli) th
String replicaStatus;
long numDocs = -1L;
-ZkCoreNodeProps replicaCoreProps = new ZkCoreNodeProps(r);
-String coreUrl = replicaCoreProps.getCoreUrl();
+String coreUrl = r.getCoreUrl();
Review Comment:
This is much nicer! I had no idea we had more direct ways...
##
solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java:
##
@@ -1827,12 +1819,9 @@ protected void assertDocCounts(boolean verbose) throws
Exception {
}
ZkStateReader zkStateReader = ZkStateReader.from(cloudClient);
long count = 0;
- final Replica.State currentState =
- Replica.State.getState(cjetty.info.getStr(ZkStateReader.STATE_PROP));
+ final Replica.State currentState = cjetty.info.getState();
Review Comment:
love this!
##
solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java:
##
Review Comment:
so much nicer and less verbose! It does sometimes feel in the cloud stuff
that you have these crazy lookups everywhere, good to see api's have evolved,
and we just need to use them!
##
solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java:
##
@@ -1536,9 +1532,9 @@ protected String checkShardConsistency(String shard,
boolean expectFailure, bool
CloudJettyRunner lastJetty = null;
for (CloudJettyRunner cjetty : solrJetties) {
- ZkNodeProps props = cjetty.info;
+ Replica replica = cjetty.info;
log.debug("client{}", cnt);
- log.debug("PROPS:{}", props);
+ log.debug("PROPS:{}", replica);
Review Comment:
Should this say REPLICA instead?
##
solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java:
##
@@ -183,7 +180,7 @@ protected void runCloudTool(CloudSolrClient
cloudSolrClient, CommandLine cli) th
}
// if we get here, we can trust the state
-replicaStatus = replicaCoreProps.getState();
+replicaStatus = String.valueOf(r.getState());
Review Comment:
Could this be a `.toString()`? I'm not familiar with `valueOf` as a
pattern...
--
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.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Skip creation of ZkCoreNodeProps when we have a Replica instance [solr]
psalagnac commented on code in PR #3315: URL: https://github.com/apache/solr/pull/3315#discussion_r2033055514 ## solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java: ## @@ -183,7 +180,7 @@ protected void runCloudTool(CloudSolrClient cloudSolrClient, CommandLine cli) th } // if we get here, we can trust the state -replicaStatus = replicaCoreProps.getState(); +replicaStatus = String.valueOf(r.getState()); Review Comment: The different is `.toString()` would raise a NPE in case `r.getState()` returns `null`. This is supposed to never happen, but since old code was accepting null values; I made sure new code too. -- 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. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Skip creation of ZkCoreNodeProps when we have a Replica instance [solr]
psalagnac commented on code in PR #3315:
URL: https://github.com/apache/solr/pull/3315#discussion_r2033056568
##
solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java:
##
@@ -1536,9 +1532,9 @@ protected String checkShardConsistency(String shard,
boolean expectFailure, bool
CloudJettyRunner lastJetty = null;
for (CloudJettyRunner cjetty : solrJetties) {
- ZkNodeProps props = cjetty.info;
+ Replica replica = cjetty.info;
log.debug("client{}", cnt);
- log.debug("PROPS:{}", props);
+ log.debug("PROPS:{}", replica);
Review Comment:
Will update. Thanks for the suggestion.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Skip creation of ZkCoreNodeProps when we have a Replica instance [solr]
psalagnac commented on code in PR #3315: URL: https://github.com/apache/solr/pull/3315#discussion_r2033055514 ## solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java: ## @@ -183,7 +180,7 @@ protected void runCloudTool(CloudSolrClient cloudSolrClient, CommandLine cli) th } // if we get here, we can trust the state -replicaStatus = replicaCoreProps.getState(); +replicaStatus = String.valueOf(r.getState()); Review Comment: The difference is `.toString()` would raise a NPE in case `r.getState()` returns `null`. This is supposed to never happen, but since old code was accepting null values; I made sure new code too. -- 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. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
