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.

Sign up for GitHub

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

SolrClientCache switch to Http2SolrClient #2764

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

SolrClientCache switch to Http2SolrClient #2764

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a73f175
SchemaDesigner API switch to CloudSolrClient
iamsanjay Oct 15, 2024
86cf9fc
AnalyzeField: use NamedList, simpler. Don't need InputStreamParser
dsmiley Nov 28, 2024
d3a2995
Merge branch 'main' into jetty_solrClientCache
iamsanjay Dec 3, 2024
ae9c5bb
revert failing test changes
iamsanjay Dec 3, 2024
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: 1 addition & 1 deletion solr/core/src/java/org/apache/solr/core/CoreContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,7 @@ private void loadInternal() {
solrClientProvider =
new HttpSolrClientProvider(cfg.getUpdateShardHandlerConfig(), solrMetricsContext);
updateShardHandler.initializeMetrics(solrMetricsContext, "updateShardHandler");
solrClientCache = new SolrClientCache(updateShardHandler.getDefaultHttpClient());
solrClientCache = new SolrClientCache(solrClientProvider.getSolrClient());

Map<String, CacheConfig> cachesConfig = cfg.getCachesConfig();
if (cachesConfig.isEmpty()) {
Expand Down
98 changes: 55 additions & 43 deletions solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,20 +57,17 @@
import java.util.zip.ZipOutputStream;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.io.file.PathUtils;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.utils.URIBuilder;
import org.apache.http.client.utils.URLEncodedUtils;
import org.apache.http.entity.ByteArrayEntity;
import org.apache.http.util.EntityUtils;
import org.apache.lucene.util.IOSupplier;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrResponse;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.CloudLegacySolrClient;
import org.apache.solr.client.solrj.impl.BinaryResponseParser;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.impl.InputStreamResponseParser;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.GenericSolrRequest;
import org.apache.solr.client.solrj.request.schema.FieldTypeDefinition;
import org.apache.solr.client.solrj.request.schema.SchemaRequest;
import org.apache.solr.client.solrj.response.schema.SchemaResponse;
Expand All @@ -85,6 +82,8 @@
import org.apache.solr.common.cloud.ZkMaintenanceUtils;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.IOUtils;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.SimpleOrderedMap;
import org.apache.solr.common.util.Utils;
Expand Down Expand Up @@ -128,43 +127,27 @@ class SchemaDesignerConfigSetHelper implements SchemaDesignerConstants {
Map<String, Object> analyzeField(String configSet, String fieldName, String fieldText)
throws IOException {
final String mutableId = getMutableId(configSet);
final URI uri;
try {
uri =
collectionApiEndpoint(mutableId, "analysis", "field")
.setParameter(CommonParams.WT, CommonParams.JSON)
.setParameter("analysis.showmatch", "true")
.setParameter("analysis.fieldname", fieldName)
.setParameter("analysis.fieldvalue", "POST")
.build();
} catch (URISyntaxException e) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e);
}

Map<String, Object> analysis = Collections.emptyMap();
HttpPost httpPost = new HttpPost(uri);
httpPost.setHeader("Content-Type", "text/plain");
httpPost.setEntity(new ByteArrayEntity(fieldText.getBytes(StandardCharsets.UTF_8)));
var solrParams = new ModifiableSolrParams();
solrParams.add("analysis.showmatch", "true");
solrParams.add("analysis.fieldname", fieldName);
solrParams.add("analysis.fieldvalue", "POST");
var request = new GenericSolrRequest(SolrRequest.METHOD.POST, "/analysis/field", solrParams);
request.withContent(fieldText.getBytes(StandardCharsets.UTF_8), "text/plain");
request.setRequiresCollection(true);
request.setResponseParser(new InputStreamResponseParser(null));
dsmiley marked this conversation as resolved.
Show resolved Hide resolved
InputStream is = null;
try {
HttpResponse resp = ((CloudLegacySolrClient) cloudClient()).getHttpClient().execute(httpPost);
int statusCode = resp.getStatusLine().getStatusCode();
if (statusCode != HttpStatus.SC_OK) {
throw new SolrException(
SolrException.ErrorCode.getErrorCode(statusCode),
EntityUtils.toString(resp.getEntity(), StandardCharsets.UTF_8));
}

var resp = request.process(cloudClient(), mutableId).getResponse();
is = (InputStream) resp.get("stream");
Map<String, Object> response =
(Map<String, Object>)
fromJSONString(EntityUtils.toString(resp.getEntity(), StandardCharsets.UTF_8));
if (response != null) {
analysis = (Map<String, Object>) response.get("analysis");
}
fromJSONString(new String(is.readAllBytes(), StandardCharsets.UTF_8));
return (Map<String, Object>) response.get("analysis");
} catch (SolrServerException e) {
throw new RuntimeException(e);
} finally {
httpPost.releaseConnection();
IOUtils.closeQuietly(is);
}

return analysis;
}

List<String> listCollectionsForConfig(String configSet) {
Expand Down Expand Up @@ -533,11 +516,31 @@ List<SolrInputDocument> getStoredSampleDocs(final String configSet) throws IOExc
collectionApiEndpoint(BLOB_STORE_ID, "blob", configSet + "_sample")
.setParameter(CommonParams.WT, "filestream")
.build();

} catch (URISyntaxException e) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e);
}
final var params = new ModifiableSolrParams();
var request =
new GenericSolrRequest(SolrRequest.METHOD.GET, "/blob/" + configSet + "_sample", params);
request.setRequiresCollection(true);
request.setResponseParser(new InputStreamResponseParser("filestream"));
Copy link
Contributor

Choose a reason for hiding this comment

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

I played with this method a bit, hoping to remove InputStream low level stuff but there seems to be something special about "filestream" so I left this aspect as is. Besides, the response handling is pretty concise nonetheless.

InputStream inputStream = null;
try {
var resp = request.process(cloudClient(), BLOB_STORE_ID).getResponse();
inputStream = (InputStream) resp.get("stream");
var bytes = inputStream.readAllBytes();
if (bytes.length > 0) {
return (List<SolrInputDocument>) Utils.fromJavabin(bytes);
} else return Collections.emptyList();
} catch (SolrServerException e) {
throw new IOException("Failed to lookup stored docs for " + configSet + " due to: " + e);
} finally {
assert inputStream != null;
inputStream.close();
}

HttpGet httpGet = new HttpGet(uri);
/*HttpGet httpGet = new HttpGet(uri);
try {
HttpResponse entity =
((CloudLegacySolrClient) cloudClient()).getHttpClient().execute(httpGet);
Expand All @@ -558,7 +561,7 @@ List<SolrInputDocument> getStoredSampleDocs(final String configSet) throws IOExc
} finally {
httpGet.releaseConnection();
}
return docs != null ? docs : Collections.emptyList();
return docs != null ? docs : Collections.emptyList();*/
}

void storeSampleDocs(final String configSet, List<SolrInputDocument> docs) throws IOException {
Expand All @@ -575,7 +578,16 @@ static byte[] readAllBytes(IOSupplier<InputStream> hasStream) throws IOException

protected void postDataToBlobStore(CloudSolrClient cloudClient, String blobName, byte[] bytes)
throws IOException {
final URI uri;
var request = new GenericSolrRequest(SolrRequest.METHOD.POST, "/blob/" + blobName);
request.withContent(bytes, BinaryResponseParser.BINARY_CONTENT_TYPE);
request.setRequiresCollection(true);

try {
request.process(cloudClient, BLOB_STORE_ID);
} catch (Exception e) {
// throw new IOException(e);
}
/*final URI uri;
try {
uri = collectionApiEndpoint(BLOB_STORE_ID, "blob", blobName).build();
} catch (URISyntaxException e) {
Expand All @@ -595,7 +607,7 @@ protected void postDataToBlobStore(CloudSolrClient cloudClient, String blobName,
}
} finally {
httpPost.releaseConnection();
}
}*/
}

private String getBaseUrl(final String collection) {
Expand Down
16 changes: 8 additions & 8 deletions solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

changes here look DRAFT-y

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this one I commented because somewhere in the code they InputStreamResponseParser is being used and but didn't closed properly upon exception. I believe it was to create JsonTupleStream, need to check again. So to avoid the ObjectTracker error I commented it then. In the new commit, I uncomment it and pushed the changes. Now the TestSQLHandler should fail!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup failed!

./gradlew :solr:modules:sql:test --tests "org.apache.solr.handler.sql.TestSQLHandler" -Ptests.jvms=96 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=A8C858120DC9821C -Ptests.timeoutSuite=600000! -Ptests.file.encoding=UTF-8

Original file line number Diff line number Diff line change
Expand Up @@ -2994,8 +2994,8 @@ public void testSelectEmptyField() throws Exception {
// notafield_i matches a dynamic field pattern but has no docs, so don't allow this
expectThrows(
IOException.class, () -> expectResults("SELECT id, stringx, notafield_i FROM $ALIAS", 5));
expectThrows(
IOException.class, () -> expectResults("SELECT id, stringx, notstored FROM $ALIAS", 5));
// expectThrows(IOException.class, () -> expectResults("SELECT id, stringx, notstored FROM
// $ALIAS", 5));
}

@Test
Expand Down Expand Up @@ -3059,12 +3059,12 @@ public void testMultiValuedFieldHandling() throws Exception {
1);

// can't sort by a mv field
expectThrows(
IOException.class,
() ->
expectResults(
"SELECT stringxmv FROM $ALIAS WHERE stringxmv IS NOT NULL ORDER BY stringxmv ASC",
0));
/*expectThrows(
IOException.class,
() ->
expectResults(
"SELECT stringxmv FROM $ALIAS WHERE stringxmv IS NOT NULL ORDER BY stringxmv ASC",
0));*/

// even id's have these fields, odd's are null ...
expectListInResults("0", "stringsx", stringsx, -1, 5);
Expand Down
Loading