You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "epugh (via GitHub)" <gi...@apache.org> on 2026/09/09 13:18:38 UTC
[PR] SOLR-17864: Provide backwards support for "${solr.hideStackTrace:false}" [solr]
epugh opened a new pull request, #4893:
URL: https://github.com/apache/solr/pull/4893
https://issues.apache.org/jira/browse/SOLR-17864
# Description
I thought migrating solr.hideStackTrace to solr.responses.stacktrace.enabled would be easy... it had been partly done. Then discovered that in solrconfig.xml and solr.xml, the property substitution didn't leverage the deprecation mechnaism.
So: `<str name="hideStackTrace">${solr.hideStackTrace:false}</str>` wouldn't actually read in the `solr.responses.stacktrace.enabled` value, and also didn't take into account that we flipped the value. So you HAVE to do `<str name="hideStackTrace">${solr.responses.stacktrace.enabled:true}</str>`
This is potentially a big problem for anyone migrating from older version to newer version who uses this feature.
# Solution
Rethink a bit how EnvUtils work... Add a Record for tracking properties and their inverted status. Now wires in so older solr.xml etc can participate in reading the deprecated values.
# Tests
add tests, and then did this script:
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17864: Provide backwards support for "${solr.hideStackTrace:false}" [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #4893:
URL: https://github.com/apache/solr/pull/4893#issuecomment-5629156516
> host, zkClientTimeout — same category as hideStackTrace, which we deliberately decided not to rename in config
Why did we not thoroughly change then; are we not sure we wanted to actually change?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17864: Provide backwards support for "${solr.hideStackTrace:false}" [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #4893:
URL: https://github.com/apache/solr/pull/4893#discussion_r3971977039
##########
solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java:
##########
@@ -108,24 +120,52 @@ public static String getProperty(String key) {
* @param defaultValue fallback value if property is not found
*/
public static String getProperty(String key, String defaultValue) {
- String value = getPropertyWithCamelCaseFallback(key);
+ String value = resolvePropertyValue(key);
return value != null ? value : defaultValue;
}
/**
- * Get a property from given key or an alias key converted from CamelCase to dot separated.
+ * Resolves {@code key} by trying, in order: the key as given; the key converted from CamelCase to
+ * dot-separated; and -- if {@code key} is itself a legacy/deprecated property name -- the current
+ * property it was replaced by. See {@link #resolveViaDeprecatedMapping} for why that last
+ * fallback is needed.
*
- * @return property value or value of dot-separated alias key or null if not found
+ * @return the resolved value, or null if none of the above are found
*/
- private static String getPropertyWithCamelCaseFallback(String key) {
+ private static String resolvePropertyValue(String key) {
String value = System.getProperty(key);
if (value != null) {
return value;
- } else {
- // Figure out if string is CamelCase and convert to dot separated
- String altKey = camelCaseToDotSeparated(key);
- return System.getProperty(altKey);
}
+ // Figure out if string is CamelCase and convert to dot separated
+ String altKey = camelCaseToDotSeparated(key);
+ value = System.getProperty(altKey);
+ if (value != null) {
+ return value;
+ }
+ return resolveViaDeprecatedMapping(altKey);
+ }
+
+ /**
+ * If {@code dotKey} is a known legacy/deprecated property name, returns the value of the current
+ * property it was replaced by (inverted, if the mapping is boolean-inverted), or null if that
+ * current property hasn't been set either.
+ *
+ * <p>This matters for config files (e.g. {@code solr.xml}) that still contain a {@code
+ * ${legacyName:default}} substitution token from before a property was renamed: without this,
+ * setting only the new property name would silently have no effect on that token, since nothing
+ * else ever rewrites the token's text. See SOLR-17864.
+ */
+ private static String resolveViaDeprecatedMapping(String dotKey) {
+ DeprecatedMapping mapping = DEPRECATED_MAPPINGS.get(dotKey);
+ if (mapping == null) {
+ return null;
+ }
+ String newValue = System.getProperty(mapping.currentName());
Review Comment:
humm, I didn't want to wait till this landed, and it wasn't that much, so I added it here.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17864: Provide backwards support for "${solr.hideStackTrace:false}" [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4893:
URL: https://github.com/apache/solr/pull/4893#discussion_r3969285070
##########
solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java:
##########
@@ -47,8 +47,14 @@ public class EnvUtils {
/** Maps ENV keys to sys prop keys for special/custom mappings */
private static final Map<String, String> CUSTOM_MAPPINGS = new HashMap<>();
+ /**
+ * Maps a legacy/deprecated sys prop key (dot-separated form) to the current property it was
+ * replaced by, and whether the replacement is boolean-inverted relative to the legacy property.
+ */
+ private record DeprecatedMapping(String currentName, boolean inverted) {}
Review Comment:
okay/fine but merely for a boolean you could simply have a parallel set of inverted names. That's what I'd do.
##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -28,9 +29,10 @@
public class ResponseUtils {
private ResponseUtils() {}
- // System property to use if the Solr core does not exist or solr.hideStackTrace is not
- // configured. (i.e.: a lot of unit test).
- private static final boolean SYSTEM_HIDE_STACK_TRACES = Boolean.getBoolean("solr.hideStackTrace");
+ // System property to use if the Solr core does not exist or solr.responses.stacktrace.enabled
Review Comment:
"solr core does not exist" seems very out of place, looking at this property's intuitive purpose
##########
solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java:
##########
@@ -108,24 +120,52 @@ public static String getProperty(String key) {
* @param defaultValue fallback value if property is not found
*/
public static String getProperty(String key, String defaultValue) {
- String value = getPropertyWithCamelCaseFallback(key);
+ String value = resolvePropertyValue(key);
return value != null ? value : defaultValue;
}
/**
- * Get a property from given key or an alias key converted from CamelCase to dot separated.
+ * Resolves {@code key} by trying, in order: the key as given; the key converted from CamelCase to
+ * dot-separated; and -- if {@code key} is itself a legacy/deprecated property name -- the current
+ * property it was replaced by. See {@link #resolveViaDeprecatedMapping} for why that last
+ * fallback is needed.
*
- * @return property value or value of dot-separated alias key or null if not found
+ * @return the resolved value, or null if none of the above are found
*/
- private static String getPropertyWithCamelCaseFallback(String key) {
+ private static String resolvePropertyValue(String key) {
String value = System.getProperty(key);
if (value != null) {
return value;
- } else {
- // Figure out if string is CamelCase and convert to dot separated
- String altKey = camelCaseToDotSeparated(key);
- return System.getProperty(altKey);
}
+ // Figure out if string is CamelCase and convert to dot separated
+ String altKey = camelCaseToDotSeparated(key);
+ value = System.getProperty(altKey);
+ if (value != null) {
+ return value;
+ }
+ return resolveViaDeprecatedMapping(altKey);
+ }
+
+ /**
+ * If {@code dotKey} is a known legacy/deprecated property name, returns the value of the current
+ * property it was replaced by (inverted, if the mapping is boolean-inverted), or null if that
+ * current property hasn't been set either.
+ *
+ * <p>This matters for config files (e.g. {@code solr.xml}) that still contain a {@code
+ * ${legacyName:default}} substitution token from before a property was renamed: without this,
+ * setting only the new property name would silently have no effect on that token, since nothing
+ * else ever rewrites the token's text. See SOLR-17864.
+ */
+ private static String resolveViaDeprecatedMapping(String dotKey) {
+ DeprecatedMapping mapping = DEPRECATED_MAPPINGS.get(dotKey);
+ if (mapping == null) {
+ return null;
+ }
+ String newValue = System.getProperty(mapping.currentName());
Review Comment:
IMO at this juncture, we should log a warning to alert the user to use the new name. See the DeprecationLog to do this elegantly.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17864: Provide backwards support for "${solr.hideStackTrace:false}" [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #4893:
URL: https://github.com/apache/solr/pull/4893#discussion_r3971666701
##########
solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java:
##########
@@ -108,24 +120,52 @@ public static String getProperty(String key) {
* @param defaultValue fallback value if property is not found
*/
public static String getProperty(String key, String defaultValue) {
- String value = getPropertyWithCamelCaseFallback(key);
+ String value = resolvePropertyValue(key);
return value != null ? value : defaultValue;
}
/**
- * Get a property from given key or an alias key converted from CamelCase to dot separated.
+ * Resolves {@code key} by trying, in order: the key as given; the key converted from CamelCase to
+ * dot-separated; and -- if {@code key} is itself a legacy/deprecated property name -- the current
+ * property it was replaced by. See {@link #resolveViaDeprecatedMapping} for why that last
+ * fallback is needed.
*
- * @return property value or value of dot-separated alias key or null if not found
+ * @return the resolved value, or null if none of the above are found
*/
- private static String getPropertyWithCamelCaseFallback(String key) {
+ private static String resolvePropertyValue(String key) {
String value = System.getProperty(key);
if (value != null) {
return value;
- } else {
- // Figure out if string is CamelCase and convert to dot separated
- String altKey = camelCaseToDotSeparated(key);
- return System.getProperty(altKey);
}
+ // Figure out if string is CamelCase and convert to dot separated
+ String altKey = camelCaseToDotSeparated(key);
+ value = System.getProperty(altKey);
+ if (value != null) {
+ return value;
+ }
+ return resolveViaDeprecatedMapping(altKey);
+ }
+
+ /**
+ * If {@code dotKey} is a known legacy/deprecated property name, returns the value of the current
+ * property it was replaced by (inverted, if the mapping is boolean-inverted), or null if that
+ * current property hasn't been set either.
+ *
+ * <p>This matters for config files (e.g. {@code solr.xml}) that still contain a {@code
+ * ${legacyName:default}} substitution token from before a property was renamed: without this,
+ * setting only the new property name would silently have no effect on that token, since nothing
+ * else ever rewrites the token's text. See SOLR-17864.
+ */
+ private static String resolveViaDeprecatedMapping(String dotKey) {
+ DeprecatedMapping mapping = DEPRECATED_MAPPINGS.get(dotKey);
+ if (mapping == null) {
+ return null;
+ }
+ String newValue = System.getProperty(mapping.currentName());
Review Comment:
maybe a slight bit of scope creep on this PR, but also really important to help folks upgrade..
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17864: Provide backwards support for "${solr.hideStackTrace:false}" [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4893:
URL: https://github.com/apache/solr/pull/4893#issuecomment-5607254886
Okay, having second thoughts on deprecation.... On a fresh bin/solr start -e techproducts, there are 2 deprecation warnings, both from the new fallback mechanism we just added:
```
o.a.s.D.z.c.timeout A config file still references the deprecated system property zkClientTimeout; it was replaced by solr.zookeeper.client.timeout. ...
o.a.s.D.host A config file still references the deprecated system property host; it was replaced by solr.host.advertise. ...
```
Worth flagging though: these two aren't a user leaving a stale config behind. They fire because the shipped solr.xml/code intentionally keeps historical token names permanently (host, zkClientTimeout — same category as hideStackTrace, which we deliberately decided not to rename in config), and bin/solr itself always sets -Dsolr.host.advertise=... at startup. So EnvUtils's fallback resolves these on every single default Solr startup, for every user, forever — not just for someone with an outdated custom config. That means this warning will show up in every fresh install's log from day one, which might read as alarming/noisy for something that's actually working exactly as designed.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17864: Provide backwards support for "${solr.hideStackTrace:false}" [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4893:
URL: https://github.com/apache/solr/pull/4893#discussion_r3985737020
##########
solr/solrj/src/java/org/apache/solr/common/util/DeprecationLog.java:
##########
@@ -41,11 +42,25 @@ public class DeprecationLog {
* @return true if logged
*/
public static boolean log(String featureId, String message) {
- if (alreadyLogged.putIfAbsent(featureId, message) != null) {
+ return log(featureId, () -> message);
+ }
+
+ /**
+ * Like {@link #log(String, String)}, but only builds the message if this is the first time {@code
+ * featureId} is logged. Use this when building the message isn't free and the call site runs
+ * often (e.g. on every property lookup).
+ *
+ * @return true if logged
+ */
+ public static boolean log(String featureId, Supplier<String> message) {
+ if (alreadyLogged.containsKey(featureId)) {
+ return false;
+ }
Review Comment:
This first part is needless; surely putIfAbsent will handle this, right?
Is this an optimization? If it is, a comment should clearly say so. But I'm suspicious it's any faster than putIfAbsent; surely that one can bail fast. I looked at the code for it in ConcurrentHashMap and I think it's good.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17864: Provide backwards support for "${solr.hideStackTrace:false}" [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #4893:
URL: https://github.com/apache/solr/pull/4893#issuecomment-6357060563
Still want to see this get delivered, even though missed 10.1.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17864: Provide backwards support for "${solr.hideStackTrace:false}" [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #4893:
URL: https://github.com/apache/solr/pull/4893#issuecomment-6074535580
it'd be good to get this off your backlog ;-) (too bad missing 10.1, sigh)
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17864: Provide backwards support for "${solr.hideStackTrace:false}" [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #4893:
URL: https://github.com/apache/solr/pull/4893#discussion_r3971688236
##########
solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java:
##########
@@ -108,24 +120,52 @@ public static String getProperty(String key) {
* @param defaultValue fallback value if property is not found
*/
public static String getProperty(String key, String defaultValue) {
- String value = getPropertyWithCamelCaseFallback(key);
+ String value = resolvePropertyValue(key);
return value != null ? value : defaultValue;
}
/**
- * Get a property from given key or an alias key converted from CamelCase to dot separated.
+ * Resolves {@code key} by trying, in order: the key as given; the key converted from CamelCase to
+ * dot-separated; and -- if {@code key} is itself a legacy/deprecated property name -- the current
+ * property it was replaced by. See {@link #resolveViaDeprecatedMapping} for why that last
+ * fallback is needed.
*
- * @return property value or value of dot-separated alias key or null if not found
+ * @return the resolved value, or null if none of the above are found
*/
- private static String getPropertyWithCamelCaseFallback(String key) {
+ private static String resolvePropertyValue(String key) {
String value = System.getProperty(key);
if (value != null) {
return value;
- } else {
- // Figure out if string is CamelCase and convert to dot separated
- String altKey = camelCaseToDotSeparated(key);
- return System.getProperty(altKey);
}
+ // Figure out if string is CamelCase and convert to dot separated
+ String altKey = camelCaseToDotSeparated(key);
+ value = System.getProperty(altKey);
+ if (value != null) {
+ return value;
+ }
+ return resolveViaDeprecatedMapping(altKey);
+ }
+
+ /**
+ * If {@code dotKey} is a known legacy/deprecated property name, returns the value of the current
+ * property it was replaced by (inverted, if the mapping is boolean-inverted), or null if that
+ * current property hasn't been set either.
+ *
+ * <p>This matters for config files (e.g. {@code solr.xml}) that still contain a {@code
+ * ${legacyName:default}} substitution token from before a property was renamed: without this,
+ * setting only the new property name would silently have no effect on that token, since nothing
+ * else ever rewrites the token's text. See SOLR-17864.
+ */
+ private static String resolveViaDeprecatedMapping(String dotKey) {
+ DeprecatedMapping mapping = DEPRECATED_MAPPINGS.get(dotKey);
+ if (mapping == null) {
+ return null;
+ }
+ String newValue = System.getProperty(mapping.currentName());
Review Comment:
okayk this requires moving deprecationLog, so will do in sepeate PR...
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17864: Provide backwards support for "${solr.hideStackTrace:false}" [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #4893:
URL: https://github.com/apache/solr/pull/4893#issuecomment-5835307566
ping for 10.1 release
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org