Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-XXXXX: Support different filesystem implementations with EmbeddedSolrServer #3146

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ Improvements

* SOLR-17516: `LBHttp2SolrClient` is now generic, adding support for `HttpJdkSolrClient`. (James Dyer)

* SOLR-17640: Support different filesystem implementations with EmbeddedSolrServer (Andrey Bozhko)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DOn't we have a GH or GITHUB type name for a PR with no JIRA?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe he put this here as a TODO. We have quite a number of existing JIRAs to pick from for this theme :-).


Optimizations
---------------------
* SOLR-17568: The CLI bin/solr export tool now contacts the appropriate nodes directly for data instead of proxying through one.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.lang.invoke.MethodHandles;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.AbstractMap.SimpleImmutableEntry;
import java.util.Map.Entry;
import net.jcip.annotations.NotThreadSafe;
Expand Down Expand Up @@ -148,12 +147,11 @@ public String lookupZKManagedSchemaPath() {
*/
public Path lookupLocalManagedSchemaPath() {
final Path legacyManagedSchemaPath =
Paths.get(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should try to remove usages of Paths.get generally, I remember it having some wonky behaviour in the past. Happy to see this coming in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely; same with Path.of. Ultimately we should ForbiddenApi check this so that it's rare to call this except for a few authorized places.

loader.getConfigPath().toString(),
ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
loader
.getConfigPath()
.resolve(ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);

Path managedSchemaPath =
Paths.get(loader.getConfigPath().toString(), managedSchemaResourceName);
Path managedSchemaPath = loader.getConfigPath().resolve(managedSchemaResourceName);

// check if we are using the legacy managed-schema file name.
if (Files.exists(legacyManagedSchemaPath)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?xml version="1.0" encoding="UTF-8" ?>
<!--
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at

https://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->

<schema name="default-config" version="1.7">
<uniqueKey>id</uniqueKey>

<field name="id" type="string" indexed="true" stored="true" required="true" multiValued="false" />
<fieldType name="string" class="solr.StrField" sortMissingLast="true" />

<field name="_version_" type="plong" indexed="false" stored="false"/>
<fieldType name="plong" class="solr.LongPointField"/>

<fieldType name="booleans" class="solr.BoolField" sortMissingLast="true" multiValued="true"/>
<dynamicField name="*_bs" type="booleans" indexed="true" stored="true"/>

<fieldType name="text_general" class="solr.TextField" positionIncrementGap="100" multiValued="true" />
</schema>
52 changes: 52 additions & 0 deletions solr/core/src/test-files/solr/configsets/zipfs/conf/solrconfig.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?xml version="1.0" encoding="UTF-8" ?>
<!--
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at

https://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->

<config>
<luceneMatchVersion>${tests.luceneMatchVersion:LATEST}</luceneMatchVersion>

<dataDir>${solr.data.dir:}</dataDir>

<directoryFactory name="DirectoryFactory"
class="${solr.directoryFactory:solr.NRTCachingDirectoryFactory}"/>

<codecFactory class="solr.SchemaCodecFactory"/>

<!-- The default high-performance update handler -->
<updateHandler class="solr.DirectUpdateHandler2">
<updateLog enable="${solr.ulog.enable:true}">
<str name="dir">${solr.ulog.dir:}</str>
</updateLog>
</updateHandler>

<!-- Primary search handler, expected by most clients, examples and UI frameworks -->
<requestHandler name="/select" class="solr.SearchHandler">
<lst name="defaults">
<str name="echoParams">explicit</str>
<int name="rows">10</int>
</lst>
</requestHandler>

<restManager>
<!--
IMPORTANT: Due to the Lucene SecurityManager, tests can only write to their runtime directory or below.
But it's easier to just keep everything in memory for testing so no remnants are left behind.
-->
<str name="storageIO">org.apache.solr.rest.ManagedResourceStorage$InMemoryStorageIO</str>
</restManager>

</config>
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
package org.apache.solr.client.solrj.embedded;

import java.io.IOException;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Map;
import org.apache.commons.io.file.PathUtils;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.request.CoreAdminRequest;
Expand Down Expand Up @@ -58,4 +63,64 @@ public void testNodeConfigConstructor() throws Exception {
assertEquals(1, server.query("newcore", new SolrQuery("*:*")).getResults().getNumFound());
}
}

@SuppressWarnings("removal")
@Test
public void testPathConstructorZipFS() throws Exception {
assumeTrue(
"Test only works without Security Manager due to SecurityConfHandlerLocal " +
"missing permission to read /1/2/3/4/security.json file",
System.getSecurityManager() == null);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make the test work regardless of the security manager, this line

securityJsonPath = Paths.get(coreContainer.getSolrHome()).resolve("security.json");
needs to be updated to handle Path in a more idiomatic way.

However, that requires updating CoreContainer#getSolrHome to return a Path instead of a String - so it looks like a much bigger change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CoreContainer.solrHome is a Path so hopefully it won't be too huge to switch it's getter to use Path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I think I will keep this PR as draft for now, and work on updating CoreContainer#getSolrHome to return Path first. Is there an existing JIRA for that?


Path dataDir = createTempDir("data-dir");
Path archive = createTempFile("configset", ".zip");
Files.delete(archive);

// When :
// Prepare a zip archive which contains
// the configset files as shown below:
//
// configset.zip
// └── 1
// └── 2
// └── 3
// └── 4
// ├── data
// │ └── core1
// │ ├── conf
// │ │ ├── managed-schema.xml
// │ │ └── solrconfig.xml
// │ └── core.properties
// └── solr.xml

var zipFs = FileSystems.newFileSystem(archive, Map.of("create", "true"));
try (zipFs) {
var destDir = zipFs.getPath("1", "2", "3", "4");
var confDir = destDir.resolve("data/core1/conf");
Files.createDirectories(confDir);
Files.copy(TEST_PATH().resolve("solr.xml"), destDir.resolve("solr.xml"));
PathUtils.copyDirectory(configset("zipfs"), confDir);

// Need to make sure we circumvent any Solr attempts
// to modify the archive when we point solrHome to
// the archive content. Steps to achieve that:
// - set a custom data dir,
// - disable the update log,
// - configure the rest manager in the solrconfig.xml with InMemoryStorageIO.
Files.writeString(
confDir.resolveSibling("core.properties"),
String.join("\n", "solr.ulog.enable=false", "solr.data.dir=" + dataDir));
}

// Then :
// EmbeddedSolrServer successfully loads the core
// using the configset directly from the archive
var configSetFs = FileSystems.newFileSystem(archive);
try (configSetFs) {
var server = new EmbeddedSolrServer(configSetFs.getPath("/1/2/3/4"), null);
try (server) {
assertEquals(List.of("core1"), server.getCoreContainer().getAllCoreNames());
}
}
}
}
Loading