dsmiley commented on code in PR #2436:
URL: https://github.com/apache/solr/pull/2436#discussion_r1586652988
##########
solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java:
##########
@@ -83,7 +83,7 @@ public class SolrCloudTestCase extends SolrTestCaseJ4 {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
- public static final String PRS_DEFAULT_PROP =
System.getProperty("use.per-replica", null);
+ public static final String PRS_DEFAULT_PROP = "solr.prs.default";
Review Comment:
Out of scope for now but but I'd like such a setting to exist Solr side
officially
##########
solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java:
##########
@@ -97,11 +97,9 @@ protected static SolrZkClient zkClient() {
return cluster.getZkStateReader().getZkClient();
}
- /** if the system property is not specified, use a random value */
+ /** if the system property is not specified, default to false. The
SystemProperty will be set in a beforeClass method. */
public static boolean isPRS() {
- return PRS_DEFAULT_PROP == null
- ? random().nextBoolean()
- : Boolean.parseBoolean(PRS_DEFAULT_PROP);
+ return Boolean.parseBoolean(System.getProperty(PRS_DEFAULT_PROP, "false"));
Review Comment:
Can EnvUtils be used here?
##########
solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java:
##########
@@ -97,11 +97,9 @@ protected static SolrZkClient zkClient() {
return cluster.getZkStateReader().getZkClient();
}
- /** if the system property is not specified, use a random value */
+ /** if the system property is not specified, default to false. The
SystemProperty will be set in a beforeClass method. */
public static boolean isPRS() {
- return PRS_DEFAULT_PROP == null
- ? random().nextBoolean()
Review Comment:
oh wow I see how this was the problem, leading to too-little control in
consistency over wether it's enabled or not in practice
##########
solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java:
##########
@@ -140,29 +138,18 @@ public static void shutdownCluster() throws Exception {
@BeforeClass
public static void setPrsDefault() {
Class<?> target = RandomizedTest.getContext().getTargetClass();
- if (target != null && target.isAnnotationPresent(NoPrs.class)) return;
- if (isPRS()) {
- System.setProperty("solr.prs.default", "true");
+ boolean prsDefault;
+ if (target != null && target.isAnnotationPresent(NoPrs.class)) {
+ prsDefault = false;
+ } else {
+ prsDefault = random().nextBoolean();
Review Comment:
shouldn't this **read** a sys prop so we can choose it for the build?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
