dsmiley commented on code in PR #4892:
URL: https://github.com/apache/solr/pull/4892#discussion_r4007019970
##########
solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java:
##########
@@ -47,7 +48,8 @@ public final class DefaultSolrCoreState extends SolrCoreState
implements RecoveryStrategy.RecoveryListener {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
- private final boolean SKIP_AUTO_RECOVERY =
Boolean.getBoolean("solrcloud.skip.autorecovery");
+ private final boolean SKIP_AUTO_RECOVERY =
+ EnvUtils.getPropertyAsBool("solr.cloud.skip.autorecovery.enabled",
false);
Review Comment:
is this a published/known property? It looks like a test-only switch.
AbstractZkTestCase and several ZK/ACL tests set it so they don't have to wait
on recovery.
##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -38,11 +38,11 @@
public class SimplePropagator implements TextMapPropagator {
private static final String TRACE_HOST_NAME =
- EnvUtils.getProperty("solr.traceHostName",
EnvUtils.getProperty("solr.host.advertise"));
+ EnvUtils.getProperty("solr.tracing.host.name",
EnvUtils.getProperty("host"));
Review Comment:
why did you revert the value? A contributor recently improved this.
##########
solr/solr-ref-guide/modules/configuration-guide/pages/solr-properties.adoc:
##########
@@ -118,6 +122,10 @@ NOTE: Properties marked with "!" indicate inverted meaning
between pre Solr 10 a
|solr.tracing.always.on.enabled|solr.alwaysOnTraceId|true|Controls whether
trace IDs are always generated, even when no request contains a trace ID.
+|solr.tracing.host.name|solr.traceHostName|Value of
`solr.host.advertise`|Overrides the host name added to the trace ID generated
for a request, when no trace ID is present on an incoming request.
+
+|solr.tracing.id.header|solr.traceIdHeader|X-Trace-Id|Specifies the HTTP
header name used to propagate the trace ID between Solr nodes.
Review Comment:
I think it's important to point out that that this configuration is ONLY
applicable to Solr's built-in non-OTEL tracing. OTEL uses a standard header...
arguably we should have used that one and not invented one.
Any way, the configuration name, I think, should somehow reflect that.
Using "solr.tracing.always.on.header" correctly associates this setting with
our "always on" tracing, which is itself disable-able and BTW is completely
ignored if someone configures OTEL.
##########
solr/solr-ref-guide/modules/configuration-guide/pages/solr-properties.adoc:
##########
@@ -42,6 +42,10 @@ NOTE: Properties marked with "!" indicate inverted meaning
between pre Solr 10 a
|solr.cloud.prep.recovery.read.timeout.additional.ms|prepRecoveryReadTimeoutExtraWait|8000|Specifies
additional milliseconds to wait during recovery read operations in SolrCloud
mode.
+|solr.cloud.prs.enabled|solr.prs.default|false|Controls the default value for
Per-Replica State (PRS) when creating a collection that doesn't explicitly
specify the `PRS` parameter.
+
+|solr.cloud.skip.autorecovery.enabled|solrcloud.skip.autorecovery|false|Controls
whether automatic recovery of out-of-sync replicas is skipped in SolrCloud
mode. Intended for testing and diagnostics, not production use.
Review Comment:
Then why are you documenting it?
##########
solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java:
##########
@@ -49,7 +50,8 @@
public class SyncStrategy {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
- private final boolean SKIP_AUTO_RECOVERY =
Boolean.getBoolean("solrcloud.skip.autorecovery");
+ private final boolean SKIP_AUTO_RECOVERY =
+ EnvUtils.getPropertyAsBool("solr.cloud.skip.autorecovery.enabled",
false);
Review Comment:
I'd prefer `solr.cloud.replica.autorecovery.skip`
--
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]
