dsmiley commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r693439374
##########
File path:
solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -195,7 +223,16 @@ private InputStream readSchemaLocally() {
InputStream schemaInputStream = null;
try {
// Attempt to load the managed schema
- schemaInputStream = loader.openResource(managedSchemaResourceName);
+ try {
+ schemaInputStream = loader.openResource(managedSchemaResourceName);
+ }
+ catch (IOException e) {
Review comment:
Not for any IOException, only when the resource isn't found. Looking at
the code, we'll through `SolrResourceNotFoundException`
##########
File path:
solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,33 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on
the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml
file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ * @see org.apache.solr.schema.ManagedIndexSchema#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" +
managedSchemaResourceName;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (!zkClient.exists(managedSchemaPath, true)){
+ log.info("Managed schema resource {} not found - loading legacy
managed schema {} file instead"
Review comment:
IMO should be debug level
##########
File path:
solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -74,12 +75,12 @@ public void init(NamedList<?> args) {
args.remove("mutable");
managedSchemaResourceName = params.get(MANAGED_SCHEMA_RESOURCE_NAME,
DEFAULT_MANAGED_SCHEMA_RESOURCE_NAME);
args.remove(MANAGED_SCHEMA_RESOURCE_NAME);
- if (SCHEMA_DOT_XML.equals(managedSchemaResourceName)) {
+ if (SCHEMA_DOT_XML.equals(managedSchemaResourceName)) { // TODO Still
needed?
Review comment:
I don't think this check is out-dated, if that's why you added this
comment.
##########
File path:
solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -195,7 +223,16 @@ private InputStream readSchemaLocally() {
InputStream schemaInputStream = null;
try {
// Attempt to load the managed schema
- schemaInputStream = loader.openResource(managedSchemaResourceName);
+ try {
+ schemaInputStream = loader.openResource(managedSchemaResourceName);
+ }
+ catch (IOException e) {
+ // Attempt to load the managed schema with the legacy name.
+ log.info("The schema is configured as managed, but managed schema
resource {} not found - looking for legacy {} instead"
Review comment:
IMO this should be debug level
##########
File path:
solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,33 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on
the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml
file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ * @see org.apache.solr.schema.ManagedIndexSchema#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" +
managedSchemaResourceName;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (!zkClient.exists(managedSchemaPath, true)){
+ log.info("Managed schema resource {} not found - loading legacy
managed schema {} file instead"
+ , managedSchemaResourceName,
ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+ managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" +
ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
Review comment:
Set interruption status on the current thread
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java
##########
@@ -111,13 +111,41 @@
this.schemaUpdateLock = schemaUpdateLock;
}
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml
file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see
org.apache.solr.schema.ManagedIndexSchemaFactory#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
Review comment:
I don't think this method belongs here; it's already on the factory
which should be sufficient. It seems you added it so that MIS.persist... is
called, that it knows where to persist it. But I think the schema already
knows it's own name/path: field `managedSchemaResourceName`. If the problem is
that this is never simply `managed-schema` then it seems you need to fix that.
--
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]
