dsmiley commented on code in PR #1314:
URL: https://github.com/apache/solr/pull/1314#discussion_r1088560118
##########
solr/core/src/java/org/apache/solr/core/SolrConfig.java:
##########
@@ -1162,4 +1176,19 @@ public ConfigNode get(String name) {
public ConfigNode get(String name, Predicate<ConfigNode> test) {
return root.get(name, test);
}
+
+ /**
+ * Generates a String ID to represent the {@link ConfigSet}
+ *
+ * <p>Relies on the name of the ConfigSet, the name of the SolrConfig, the
{@link
+ * String#hashCode()} to generate a "unique" id for the solr.xml data
(including substitutions),
+ * and the version of the overlay. These 3 peices of data should combine to
make a "unique"
+ * identifier for SolrConfigs, since those are ultimately all inputs to
modifying the solr.xml
+ * result.
+ *
+ * @param configSetName the name of the configSet that this SolrConfig
belongs to.
+ */
+ public String effectiveId(String configSetName) {
+ return configSetName + "-" + getName() + "-" + znodeVersion + "-" +
rootDataHashCode;
+ }
Review Comment:
I'm good with it being as a part of the ID so long as configSet is a field
in the SolrConfig so that it's more clearly a part of its identity. Adding as
a parameter on this method is suspicious.
##########
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##########
@@ -1136,10 +1137,23 @@ private SolrCore(
updateProcessorChains = loadUpdateProcessorChains();
reqHandlers = new RequestHandlers(this);
reqHandlers.initHandlersFromConfig(solrConfig);
- jerseyAppHandler =
- (V2ApiUtils.isEnabled())
- ? new
ApplicationHandler(reqHandlers.getRequestHandlers().getJerseyEndpoints())
- : null;
+ if (V2ApiUtils.isEnabled()) {
+ final String effectiveSolrConfigId =
solrConfig.effectiveId(configSet.getName());
+ jerseyAppHandler =
+ coreContainer
+ .getAppHandlerCache()
+ .computeIfAbsent(
+ effectiveSolrConfigId,
+ () -> {
+ log.debug(
Review Comment:
Why? Note we have checks that can tell when it's helpful and when it isn't.
It isn't helpful in this circumstance because the interpolated arguments are
plain references -- not method calls that might do expensive stuff. The
log.isDebugEnabled is verbose IMO for too little gain.
##########
solr/core/src/java/org/apache/solr/core/SolrConfig.java:
##########
@@ -178,8 +179,12 @@ private class ResourceProvider implements Function<String,
InputStream> {
ZkSolrResourceLoader.ZkByteArrayInputStream zkin =
(ZkSolrResourceLoader.ZkByteArrayInputStream) in;
zkVersion = zkin.getStat().getVersion();
- hash = Objects.hash(zkin.getStat().getCtime(), zkVersion,
overlay.getZnodeVersion());
+ hash = Objects.hash(zkin.getStat().getCtime(), zkVersion,
overlay.getVersion());
Review Comment:
Note that a re-created file (removed then added) will have its zkVersion
reset to 0. So incorporating CTime guards against this.
--
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]
