[PR] in tests replace deprecated IndexSearcher.doc() calls [solr]
cpoerschke opened a new pull request, #3149: URL: https://github.com/apache/solr/pull/3149 The API is deprecated since Lucene 9.5 - https://github.com/apache/lucene/blob/releases/lucene/9.5.0/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java#L382-L391 -- and remove in Lucene 10. So the replacement here can go to both `main` and `branch_9x` branches with no JIRA or solr/CHANGES.txt entry needed in my opinion. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17631: Upgrade main to Lucene 10.x [solr]
cpoerschke commented on code in PR #3053:
URL: https://github.com/apache/solr/pull/3053#discussion_r1937402513
##
solr/modules/ltr/src/test/org/apache/solr/ltr/TestLTRReRankingPipeline.java:
##
@@ -145,8 +145,8 @@ public void testRescorer() throws Exception {
hits = rescorer.rescore(searcher, hits, 2);
// rerank using the field finalScore
- assertEquals("1", searcher.doc(hits.scoreDocs[0].doc).get("id"));
- assertEquals("0", searcher.doc(hits.scoreDocs[1].doc).get("id"));
+ assertEquals("1",
searcher.storedFields().document(hits.scoreDocs[0].doc).get("id"));
+ assertEquals("0",
searcher.storedFields().document(hits.scoreDocs[1].doc).get("id"));
Review Comment:
Elsewhere outside of `modules/ltr` similar changes are needed too, #3149
proposes to separately replace.
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Created] (SOLR-17644) Collection creation fails when replica placement plugin and basic auth are enabled
Colvin Cowie created SOLR-17644:
---
Summary: Collection creation fails when replica placement plugin
and basic auth are enabled
Key: SOLR-17644
URL: https://issues.apache.org/jira/browse/SOLR-17644
Project: Solr
Issue Type: Bug
Security Level: Public (Default Security Level. Issues are Public)
Affects Versions: 9.8, main (10.0)
Reporter: Colvin Cowie
In Solr 9.8 and latest, collection creation fails when the replica placement
plugin and authentication are both configured. See
[https://the-asf.slack.com/archives/CEKUCUNE9/p1738152813677179] for more info
Stacktrace:
{quote}2025-01-29 11:42:40.638 WARN (OverseerThreadFactory-22-thread-1)
[c:main_index s: r: x: t:] o.a.s.c.s.i.SolrClientNodeStateProvider could not
get tags from node localhost:8983_solr =>
org.apache.solr.client.solrj.impl.BaseHttpSolrClient$RemoteSolrException: Error
from server at
http://localhost:8983/solr/admin/metrics
: Expected mime type in [application/octet-stream,
application/vnd.apache.solr.javabin] but got text/html.
Error 401 require authentication
HTTP ERROR 401 require authentication
URI:/solr/admin/metrics
STATUS:401
MESSAGE:require authentication
SERVLET:default
at
org.apache.solr.client.solrj.impl.HttpSolrClientBase.checkContentType(HttpSolrClientBase.java:341)
org.apache.solr.client.solrj.impl.BaseHttpSolrClient$RemoteSolrException: Error
from server at
http://localhost:8983/solr/admin/metrics
: Expected mime type in [application/octet-stream,
application/vnd.apache.solr.javabin] but got text/html.
Error 401 require authentication
HTTP ERROR 401 require authentication
URI:/solr/admin/metrics
STATUS:401
MESSAGE:require authentication
SERVLET:default
at
org.apache.solr.client.solrj.impl.HttpSolrClientBase.checkContentType(HttpSolrClientBase.java:341)
~[?:?]
at
org.apache.solr.client.solrj.impl.HttpSolrClientBase.processErrorsAndResponse(HttpSolrClientBase.java:227)
~[?:?]
at
org.apache.solr.client.solrj.impl.Http2SolrClient.processErrorsAndResponse(Http2SolrClient.java:621)
~[?:?]
at
org.apache.solr.client.solrj.impl.Http2SolrClient.request(Http2SolrClient.java:542)
~[?:?]
at org.apache.solr.client.solrj.SolrRequest.process(SolrRequest.java:279) ~[?:?]
at org.apache.solr.client.solrj.SolrRequest.process(SolrRequest.java:295) ~[?:?]
at
org.apache.solr.client.solrj.impl.Http2SolrClient.requestWithBaseUrl(Http2SolrClient.java:604)
~[?:?]
at
org.apache.solr.client.solrj.impl.SolrClientNodeStateProvider$RemoteCallCtx.invoke(SolrClientNodeStateProvider.java:292)
~[?:?]
at
org.apache.solr.client.solrj.impl.SolrClientNodeStateProvider$RemoteCallCtx.invokeWithRetry(SolrClientNodeStateProvider.java:255)
~[?:?]
at
org.apache.solr.client.solrj.impl.SolrClientNodeStateProvider.fetchReplicaMetrics(SolrClientNodeStateProvider.java:190)
~[?:?]
at
org.apache.solr.client.solrj.impl.NodeValueFetcher.getRemotePropertiesAndMetrics(NodeValueFetcher.java:125)
~[?:?]
at
org.apache.solr.client.solrj.impl.NodeValueFetcher.getTags(NodeValueFetcher.java:187)
~[?:?]
at
org.apache.solr.client.solrj.impl.SolrClientNodeStateProvider.fetchTagValues(SolrClientNodeStateProvider.java:114)
~[?:?]
at
org.apache.solr.client.solrj.impl.SolrClientNodeStateProvider.getNodeValues(SolrClientNodeStateProvider.java:106)
~[?:?]
at
org.apache.solr.cluster.placement.impl.AttributeFetcherImpl.fetchAttributes(AttributeFetcherImpl.java:149)
~[?:?]
at
org.apache.solr.cluster.placement.plugins.AffinityPlacementFactory$AffinityPlacementPlugin.getBaseWeightedNodes(AffinityPlacementFactory.java:284)
~[?:?]
at
org.apache.solr.cluster.placement.plugins.OrderedNodePlacementPlugin.getWeightedNodes(OrderedNodePlacementPlugin.java:316)
~[?:?]
at
org.apache.solr.cluster.placement.plugins.OrderedNodePlacementPlugin.computePlacements(OrderedNodePlacementPlugin.java:88)
~[?:?]
at
org.apache.solr.cluster.placement.impl.PlacementPluginAssignStrategy.assign(PlacementPluginAssignStrategy.java:84)
~[?:?]
at
org.apache.solr.cloud.api.collections.Assign$AssignStrategy.assign(Assign.java:432)
~[?:?]
at
org.apache.solr.cloud.api.collections.CreateCollectionCmd.buildReplicaPositions(CreateCollectionCmd.java:537)
~[?:?]
at
org.apache.solr.cloud.api.collections.CreateCollectionCmd.call(CreateCollectionCmd.java:234)
~[?:?]
at
org.apache.solr.cloud.api.collections.CollApiCmds$TraceAwareCommand.call(CollApiCmds.java:225)
~[?:?]
at
org.apache.solr.cloud.api.collections.OverseerCollectionMessageHandler.processMessage(OverseerCollectionMessageHandler.java:130)
~[?:?]
at
org.apache.solr.cloud.OverseerTaskProcessor$Runner.run(OverseerTaskProcessor.java:564)
~[?:?]
at
org.apache.solr.common.util.ExecutorUtil$MDCAwareThreadPoolExecutor.lambda$execute$0(ExecutorUtil.java:380)
~[?:?]
at
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
~[?:?]
at
java.base/java.util.
Re: [PR] feat(update-security-json): Update security.json when secret is updated [solr-operator]
gerlowskija commented on code in PR #744:
URL: https://github.com/apache/solr-operator/pull/744#discussion_r1937407517
##
controllers/util/solr_security_util.go:
##
@@ -240,11 +245,16 @@ func cmdToPutSecurityJsonInZk() string {
cmd := " solr zk cp zk:/security.json /tmp/current_security.json
>/dev/null 2>&1; " +
" GET_CURRENT_SECURITY_JSON_EXIT_CODE=$?; " +
"if [ ${GET_CURRENT_SECURITY_JSON_EXIT_CODE} -eq 0 ]; then " +
// JSON already exists
- "if [ ! -s /tmp/current_security.json ] || grep -q '^{}$'
/tmp/current_security.json ]; then " + // File doesn't exist, is empty, or is
just '{}'
+ "if [ ! -s /tmp/current_security.json ] || grep -q '^{}$'
/tmp/current_security.json; then " + // File doesn't exist, is empty, or is
just '{}'
" echo $SECURITY_JSON > /tmp/security.json;" +
" solr zk cp /tmp/security.json zk:/security.json >/dev/null
2>&1; " +
" echo 'Blank security.json found. Put new security.json in
ZK'; " +
- "fi; " + // TODO: Consider checking a diff and still applying
over the top
+ "elif [ \"${SECURITY_JSON_OVERWRITE}\" = true ] && [ \"$(cat
/tmp/current_security.json)\" != \"$(echo $SECURITY_JSON)\" ]; then " + // We
want to overwrite the security config if there's a diff
Review Comment:
[Q] Am I reading this correctly, that the first two blocks both upload the
JSON and only differ in the message they echo out? There might be a better way
to structure that to avoid the duplication...
##
api/v1beta1/solrcloud_types.go:
##
@@ -1621,6 +1624,15 @@ const (
Basic AuthenticationType = "Basic"
)
+type BootstrapSecurityJson struct {
+ SecurityJsonSecret *corev1.SecretKeySelector
`json:"bootstrapSecurityJson,omitempty"`
+
+ // Flag to indicate if the operator should overwrite an existing
security.json if there are changes
+ // as compared to the underlying secret
+ // +optional
+ Overwrite bool `json:"probesRequireAuth,omitempty"`
Review Comment:
[-1] The 'probesRequireAuth' bit looks like a copy/paste error, unless
there's a reason I'm missing to use that name for the yaml property?
##
config/crd/bases/solr.apache.org_solrclouds.yaml:
##
@@ -10205,29 +10205,37 @@ spec:
description: |-
Configure a user-provided security.json from a secret to
allow for advanced security config.
If not specified, the operator bootstraps a
security.json with basic auth enabled.
- This is a bootstrapping config only; once Solr is
initialized, the security config should be managed by the security API.
properties:
- key:
-description: The key of the secret to select from.
Must be
- a valid secret key.
-type: string
- name:
-default: ""
+ bootstrapSecurityJson:
+description: SecretKeySelector selects a key of a
Secret.
+properties:
+ key:
+description: The key of the secret to select from.
Must
+ be a valid secret key.
+type: string
+ name:
+default: ""
+description: |-
+ Name of the referent.
+ This field is effectively required, but due to
backwards compatibility is
+ allowed to be empty. Instances of this type with
an empty value here are
+ almost certainly wrong.
+ More info:
https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+type: string
+ optional:
+description: Specify whether the Secret or its key
must
+ be defined
+type: boolean
+required:
+- key
+type: object
+x-kubernetes-map-type: atomic
+ probesRequireAuth:
Review Comment:
[0] See above comment in solrcloud_types.go about this name likely being a
copy/paste error.
##
tests/scripts/manage_e2e_tests.sh:
##
@@ -130,7 +130,7 @@ function run_tests() {
GINKGO_PARAMS+=("${param}" "${!envName}")
fi
done
- GINKGO_PARAMS+=("${RAW_GINKGO[@]}")
+ GINKGO_PARAMS+=("${RAW_GINKGO[@]:-}")
Review Comment:
[0] I've run into the "RAW_GINKGO unset" error before but was never sure
whether it was a bad/unnecessary assumption made by the script, or whe
[PR] Limit Github Actions runtime and cleanup always [solr]
uvatbc opened a new pull request, #3152: URL: https://github.com/apache/solr/pull/3152 https://issues.apache.org/jira/browse/SOLR-X # Description The Solr tests ran for over 2 days - and I forgot to check them over the weekend. Keeping a 96 core machine running for that long is expensive. # Solution 1. Limit the runtime to 90 minutes 2. At the end, always send a clear kill signal to the job running in Crave # Tests None: This is a Github Actions change # Checklist Please review the following and check all that apply: - [x] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) and my code conforms to the standards described there to the best of my ability. - [ ] I have created a Jira issue and added the issue ID to my pull request title. - [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation) - [x] I have developed this patch against the `main` branch. - [x] I have run `./gradlew check`. - [ ] I have added tests for my changes. - [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide) -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
cpoerschke commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1937653570
##
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##
@@ -0,0 +1,100 @@
+/*
+ * 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
+ *
+ * http://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.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import
org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+private final String inputField;
+private final String outputField;
+private final String model;
+private SolrTextToVectorModel textToVector;
+private ManagedTextToVectorModelStore modelStore = null;
Review Comment:
```suggestion
private final ManagedTextToVectorModelStore modelStore;
```
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
cpoerschke commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1937657567
##
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##
@@ -0,0 +1,100 @@
+/*
+ * 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
+ *
+ * http://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.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import
org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+private final String inputField;
+private final String outputField;
+private final String model;
+private SolrTextToVectorModel textToVector;
+private ManagedTextToVectorModelStore modelStore = null;
+
+public TextToVectorUpdateProcessor(
+String inputField,
+String outputField,
+String model,
+SolrQueryRequest req,
+UpdateRequestProcessor next) {
+super(next);
+this.inputField = inputField;
+this.outputField = outputField;
+this.model = model;
+this.modelStore =
ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+}
+
+/**
+ * @param cmd the update command in input containing the Document to
process
+ * @throws IOException If there is a low-level I/O error
+ */
+@Override
+public void processAdd(AddUpdateCommand cmd) throws IOException {
+this.textToVector = modelStore.getModel(model);
Review Comment:
Wondering re: `textToVector` as class member vs. local variable.
edit: if `model` is configured i.e. always the same it could be a class
member, perhaps, but if perhaps `model` was a field in the `cmd` document then
it would need to be a local variable.
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17640) Support different filesystem implementations with EmbeddedSolrServer
[ https://issues.apache.org/jira/browse/SOLR-17640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922812#comment-17922812 ] David Smiley commented on SOLR-17640: - This is the tip of an iceberg. I'm tempted to suggest to close this but I actually love the goal -- it's just an end-state with many steps (not listed here) to reach it. I filed SOLR-17646 which would be a huge value and where most of the effort is needed. Other stuff is nice, ultimately one day leading to an EmbeddedSolrServer fully from Zip maybe but the real value is in the DirectoryFactory. Maybe you would be interested in looking at that? > Support different filesystem implementations with EmbeddedSolrServer > > > Key: SOLR-17640 > URL: https://issues.apache.org/jira/browse/SOLR-17640 > Project: Solr > Issue Type: Improvement > Components: Embedded >Affects Versions: 9.8 >Reporter: Andrey Bozhko >Priority: Minor > > EmbeddedSolrServer has a public constructor `EmbeddedSolrServer(Path > solrHome, String defaultCoreName)` - which suggests that any Path > implementation is acceptable (e.g., ZipPath, S3Path, and not just > UnixPath/WindowsPath). > It would be good to try this out and see if EmbeddedSolrServer actually works > with solrHome paths in filesystems other than the default one. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Created] (SOLR-17646) DirectoryFactory should support any NIO FileSystemProvider
David Smiley created SOLR-17646: --- Summary: DirectoryFactory should support any NIO FileSystemProvider Key: SOLR-17646 URL: https://issues.apache.org/jira/browse/SOLR-17646 Project: Solr Issue Type: New Feature Reporter: David Smiley Ideally, the DirectoryFactory should support any NIO FileSystemProvider without assuming the local/default FileSystem. There are a variety of implementations out there, such as for cloud storage (S3, GCP, ...), [HDFS|https://github.com/damiencarol/jsr203-hadoop], and even Zip (imagine a read-only directory). This means switching from String & File to Path, and avoiding sneaky methods that assume the default FileSystem. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Limit Github Actions runtime and cleanup always [solr]
HoustonPutman commented on PR #3152: URL: https://github.com/apache/solr/pull/3152#issuecomment-2628057773 I changed it to 40 min, because if everything is working it runs in like 20 min. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17646) DirectoryFactory should support any NIO FileSystemProvider
[ https://issues.apache.org/jira/browse/SOLR-17646?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922820#comment-17922820 ] David Smiley commented on SOLR-17646: - As I write this, the title of this JIRA is scoped to only the DirectoryFactory but that quickly leads you to SolrCore getDataDir and UpdateLog. It's up to the implementer if doing all those in one issue/PR is more straight-forward than separately; hard to say without embarking on the journey. > DirectoryFactory should support any NIO FileSystemProvider > -- > > Key: SOLR-17646 > URL: https://issues.apache.org/jira/browse/SOLR-17646 > Project: Solr > Issue Type: New Feature >Reporter: David Smiley >Priority: Major > > Ideally, the DirectoryFactory should support any NIO FileSystemProvider > without assuming the local/default FileSystem. There are a variety of > implementations out there, such as for cloud storage (S3, GCP, ...), > [HDFS|https://github.com/damiencarol/jsr203-hadoop], and even Zip (imagine a > read-only directory). This means switching from String & File to Path, and > avoiding sneaky methods that assume the default FileSystem. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17609) Remove hdfs module
[ https://issues.apache.org/jira/browse/SOLR-17609?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922823#comment-17922823 ] David Smiley commented on SOLR-17609: - Linking to SOLR-17646, (NIO FileSystemProvider) as what I believe should be the foundation of a Directory oriented implementation, should HDFS return in some form in the future. > Remove hdfs module > -- > > Key: SOLR-17609 > URL: https://issues.apache.org/jira/browse/SOLR-17609 > Project: Solr > Issue Type: Task > Components: hdfs >Affects Versions: main (10.0) >Reporter: Eric Pugh >Priority: Major > Labels: pull-request-available > Time Spent: 1h 20m > Remaining Estimate: 0h > > One of the outcomes of the 2024 Community Survey is that we learned (in our > admittedly fairly unscientific) responses that hdfs module is not used. > This PR is to understand the impact of removing hdfs in Solr 10. > See [https://lists.apache.org/thread/hp6bov79rgrg0gb2ozzbzxxn30k2js0h] for > discussion on Dev. > > I won't merge this PR till we have more consensus. > This builds on work started in > https://issues.apache.org/jira/browse/SOLR-14660 and > https://issues.apache.org/jira/browse/SOLR-14021 -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17646) DirectoryFactory should support any NIO FileSystemProvider
[ https://issues.apache.org/jira/browse/SOLR-17646?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922824#comment-17922824 ] David Smiley commented on SOLR-17646: - Another comment on remote/cloud storage: A FileSystem abstraction/implementation alone will have bad performance, as there's no memory or other caching. But that can be layered above using the "org.apache.solr.hdfs.store.blockcache.BlockDirectory" (completely unrelated to HDFS). Any way, I shouldn't distract this issue with follow-on concerns with real-world usage of a pluggable remote FileSystemProvider. > DirectoryFactory should support any NIO FileSystemProvider > -- > > Key: SOLR-17646 > URL: https://issues.apache.org/jira/browse/SOLR-17646 > Project: Solr > Issue Type: New Feature >Reporter: David Smiley >Priority: Major > > Ideally, the DirectoryFactory should support any NIO FileSystemProvider > without assuming the local/default FileSystem. There are a variety of > implementations out there, such as for cloud storage (S3, GCP, ...), > [HDFS|https://github.com/damiencarol/jsr203-hadoop], and even Zip (imagine a > read-only directory). This means switching from String & File to Path, and > avoiding sneaky methods that assume the default FileSystem. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16903: Migrate off java.io.File to java.nio.file.Path from core files [solr]
epugh commented on PR #2924: URL: https://github.com/apache/solr/pull/2924#issuecomment-2627404639 Thank you @dsmiley for digging through things. Your comments and eyes is probably saving us a lot of test failures down the road ;-). -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Created] (SOLR-17645) Handle executor thread failures more gracefully
Houston Putman created SOLR-17645: - Summary: Handle executor thread failures more gracefully Key: SOLR-17645 URL: https://issues.apache.org/jira/browse/SOLR-17645 Project: Solr Issue Type: Bug Affects Versions: 9.8 Reporter: Houston Putman As of SOLR-17448, we use ExecutorService.execute() much more throughout the codebase. This is a good thing since before we weren't doing anything with the Future objects that ExecutorService.submit() would return. One drawback however, is that in our tests, we use randomizedTesting (the library) to listen for uncaught exceptions in threads. I believe the change to ExecutorService.execute() led to this, because there is no Future to return that an exception was thrown. Usually we do well in catching exceptions in our threads, but certainly not always. We should audit this, to improve error handling in our code as well as limit test failures. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[PR] SOLR-17645: Gracefully handle exceptions in executor threads [solr]
HoustonPutman opened a new pull request, #3150: URL: https://github.com/apache/solr/pull/3150 https://issues.apache.org/jira/browse/SOLR-17645 The main one I see in our tests are threads submitted by ZkStateReader.waitForState(), and ZkShardTerms having issues while a cluster is shutting down. I'll be looking for more and trying to address in this one 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Updated] (SOLR-17645) Handle executor thread failures more gracefully
[ https://issues.apache.org/jira/browse/SOLR-17645?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] ASF GitHub Bot updated SOLR-17645: -- Labels: pull-request-available (was: ) > Handle executor thread failures more gracefully > --- > > Key: SOLR-17645 > URL: https://issues.apache.org/jira/browse/SOLR-17645 > Project: Solr > Issue Type: Bug >Affects Versions: 9.8 >Reporter: Houston Putman >Priority: Major > Labels: pull-request-available > Time Spent: 10m > Remaining Estimate: 0h > > As of SOLR-17448, we use ExecutorService.execute() much more throughout the > codebase. This is a good thing since before we weren't doing anything with > the Future objects that ExecutorService.submit() would return. > One drawback however, is that in our tests, we use randomizedTesting (the > library) to listen for uncaught exceptions in threads. I believe the change > to ExecutorService.execute() led to this, because there is no Future to > return that an exception was thrown. > Usually we do well in catching exceptions in our threads, but certainly not > always. We should audit this, to improve error handling in our code as well > as limit test failures. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17642) Knn queries use multi threaded search unintentionally
[
https://issues.apache.org/jira/browse/SOLR-17642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922811#comment-17922811
]
Christine Poerschke commented on SOLR-17642:
{quote}... The fact we initialised indexSearcherExector means all KNN queries
use that thread pool and there is no way to disable that. ...
{quote}
It's undocumented but after [https://github.com/apache/solr/pull/2570]
configuring a zero or negative thread count would disable the executor i.e. a
null executor would be passed just like before.
*
[https://github.com/apache/solr/blob/releases/solr/9.7.0/solr/core/src/java/org/apache/solr/core/CoreContainer.java#L460]
*
[https://github.com/apache/solr/blob/releases/solr/9.8.0/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java#L228]
Hope that helps.
> Knn queries use multi threaded search unintentionally
> ---
>
> Key: SOLR-17642
> URL: https://issues.apache.org/jira/browse/SOLR-17642
> Project: Solr
> Issue Type: Bug
>Affects Versions: 9.7, 9.8
>Reporter: Varun Thacker
>Priority: Major
>
> First reported on
> [https://lists.apache.org/thread/xn1xg5szxhrs0sbcy4gmx4cvohy6flvh|https://lists.apache.org/thread/xn1xg5szxhrs0sbcy4gmx4cvohy6flvh,],
> Dr. Andreas Moll reported a failing query.
> I was able to reproduce is consistently locally on 9.7 and 9.8 using Dr.
> Andreas Moll's repro steps.
> There are two bugs in play here
> *The Solr side bug:*
> * Solr 9.7 added SOLR-13350 but disabled multi-threaded search by default. I
> can confirm the failing query follows this code path
>
> {code:java}
> if (!MultiThreadedSearcher.allowMT(pf.postFilter, cmd)) {
> log.trace("SINGLE THREADED search, skipping collector manager in
> getDocListNC");{code}
>
> * However when following that code path the query goes to
> AbstractKnnVectorQuery and this is where executor thread pool gets used
> {code:java}
> TimeLimitingKnnCollectorManager knnCollectorManager =
> new TimeLimitingKnnCollectorManager(
> getKnnCollectorManager(k, indexSearcher), indexSearcher.getTimeout());
> TaskExecutor taskExecutor = indexSearcher.getTaskExecutor();
> {code}
> * This executor was added in SOLR-13350 in CoreContainer(
> indexSearcherExecutor) . If you follow the SolrIndexSearcher code that calls
> {code:java}
> In SolrIndexSearcher.java
> super(wrapReader(core, r),
> core.getCoreContainer().getIndexSearcherExecutor());{code}
> {code:java}
> In IndexSearcher.java
> this.executor = executor;
> this.taskExecutor =
> executor == null ? new TaskExecutor(Runnable::run) : new
> TaskExecutor(executor);{code}
> I think the idea in SOLR-13350 was create an indexSearcherExector, but only
> use it when multiThreaded=true as a query param (default=false). The fact we
> initialised indexSearcherExector means all KNN queries use that thread pool
> and there is no way to disable that.
>
> *Now the Lucene Bug:*
> [~benwtrent] noted on
> [https://lists.apache.org/thread/ftm6f32f2nsoys6noxx8p2ygpwnlfjc9] that
> Multi-threaded vector search over multiple segments can lead to inconsistent
> results
>
> So the fact Solr by default always uses multi threaded search 9.7+ triggers
> the lucene bug. But I think in Solr we should also have a way to not use
> multi-threaded KNN like we do for other queries.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
cpoerschke commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1937651836
##
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactory.java:
##
@@ -0,0 +1,97 @@
+/*
+ * 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
+ *
+ * http://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.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.DenseVectorField;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
+
+/**
+ * This class implements an UpdateProcessorFactory for the Text To Vector
Update Processor.
+ */
+public class TextToVectorUpdateProcessorFactory extends
UpdateRequestProcessorFactory {
+private static final String INPUT_FIELD_PARAM = "inputField";
+private static final String OUTPUT_FIELD_PARAM = "outputField";
+private static final String MODEL_NAME = "model";
+
+String inputField;
+String outputField;
+String modelName;
+SolrParams params;
Review Comment:
```suggestion
private String inputField;
private String outputField;
private String modelName;
private SolrParams params;
```
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
cpoerschke commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1937657567
##
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##
@@ -0,0 +1,100 @@
+/*
+ * 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
+ *
+ * http://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.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import
org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+private final String inputField;
+private final String outputField;
+private final String model;
+private SolrTextToVectorModel textToVector;
+private ManagedTextToVectorModelStore modelStore = null;
+
+public TextToVectorUpdateProcessor(
+String inputField,
+String outputField,
+String model,
+SolrQueryRequest req,
+UpdateRequestProcessor next) {
+super(next);
+this.inputField = inputField;
+this.outputField = outputField;
+this.model = model;
+this.modelStore =
ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+}
+
+/**
+ * @param cmd the update command in input containing the Document to
process
+ * @throws IOException If there is a low-level I/O error
+ */
+@Override
+public void processAdd(AddUpdateCommand cmd) throws IOException {
+this.textToVector = modelStore.getModel(model);
Review Comment:
Wondering re: `textToVector` as class member vs. local variable.
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
cpoerschke commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1937732098
##
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##
@@ -0,0 +1,100 @@
+/*
+ * 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
+ *
+ * http://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.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import
org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+private final String inputField;
+private final String outputField;
+private final String model;
+private SolrTextToVectorModel textToVector;
+private ManagedTextToVectorModelStore modelStore = null;
+
+public TextToVectorUpdateProcessor(
+String inputField,
+String outputField,
+String model,
+SolrQueryRequest req,
+UpdateRequestProcessor next) {
+super(next);
+this.inputField = inputField;
+this.outputField = outputField;
+this.model = model;
+this.modelStore =
ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+}
+
+/**
+ * @param cmd the update command in input containing the Document to
process
+ * @throws IOException If there is a low-level I/O error
+ */
+@Override
+public void processAdd(AddUpdateCommand cmd) throws IOException {
+this.textToVector = modelStore.getModel(model);
+if (textToVector == null) {
+throw new SolrException(
+SolrException.ErrorCode.BAD_REQUEST,
+"The model requested '"
++ model
++ "' can't be found in the store: "
++ ManagedTextToVectorModelStore.REST_END_POINT);
+}
+
+SolrInputDocument doc = cmd.getSolrInputDocument();
+SolrInputField inputFieldContent = doc.get(inputField);
+if (!isNullOrEmpty(inputFieldContent, doc, inputField)) {
+String textToVectorise =
inputFieldContent.getValue().toString();//add null checks and
+float[] vector = textToVector.vectorise(textToVectorise);
Review Comment:
So text-to-vector in the search case is ephemeral i.e. in case of errors or
timeouts the user gets an error or exception back and they may or may not
choose to retry.
For text-to-vector in the update case, might some users prefer to index only
documents with (all the) vectors and others would rather have the document
indexed even if some vectors are missing? (I assume but don't know for sure
that the `vectorise` call might throw an exception in certain circumstances and
if it's not caught that might fail the indexing for the entire document.)
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Limit Github Actions runtime and cleanup always [solr]
dsmiley commented on PR #3152: URL: https://github.com/apache/solr/pull/3152#issuecomment-2628129391 Thanks! -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Limit Github Actions runtime and cleanup always [solr]
dsmiley merged PR #3152: URL: https://github.com/apache/solr/pull/3152 -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-14423) static caches in StreamHandler ought to move to CoreContainer lifecycle
[ https://issues.apache.org/jira/browse/SOLR-14423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922888#comment-17922888 ] David Smiley commented on SOLR-14423: - Following up to those here; I'm discussing with others in SOLR-17630 about deprecating CoreContainer.getSolrClientCache in lieu of a node CloudSolrClient and node Http2SolrClient (that can use any URL to talk to whoever) for general inter-node use. SCC then becomes something only needed by the streaming expressions framework, so would likely go back there. If you have thoughts, please raise them there. > static caches in StreamHandler ought to move to CoreContainer lifecycle > --- > > Key: SOLR-14423 > URL: https://issues.apache.org/jira/browse/SOLR-14423 > Project: Solr > Issue Type: Bug > Components: streaming expressions >Reporter: David Smiley >Assignee: Andrzej Bialecki >Priority: Major > Fix For: 8.6 > > Time Spent: 1h 20m > Remaining Estimate: 0h > > StreamHandler (at "/stream") has several statically declared caches. I think > this is problematic, such as in testing wherein multiple nodes could be in > the same JVM. One of them is more serious -- SolrClientCache which is > closed/cleared via a SolrCore close hook. That's bad for performance but > also dangerous since another core might want to use one of these clients! > CC [~jbernste] -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17630) Add CloudSolrClient instance for a Solr node
[
https://issues.apache.org/jira/browse/SOLR-17630?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922889#comment-17922889
]
David Smiley commented on SOLR-17630:
-
The node SolrClientCache instance can be demoted to an object living in the
ObjectCache, thus no longer with a first-class method of CoreContainer that's
too tempting to use.
> Add CloudSolrClient instance for a Solr node
>
>
> Key: SOLR-17630
> URL: https://issues.apache.org/jira/browse/SOLR-17630
> Project: Solr
> Issue Type: Improvement
> Components: SolrCloud
>Reporter: David Smiley
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> There ought to be a general CloudSolrClient instance for the Solr node,
> without each potential user of such needing to create one. The closest
> substitute at the moment is
> {{cc.getSolrClientCache().getCloudSolrClient(cc.getZkController().getZkServerAddress())}}
> which is too verbose, not as discoverable, and it's debatable if
> SolrClientCache should be it's home.
> A scalability/simplicity advantage of a shared one instead of newly
> constructed one is that the existing ZkClientClusterStateProvider (same node
> ZkStateReader instance) can be used, thus improving scalability and
> simplifying interpretation of logs (as all logs from ZkStateReader on a node
> can be assumed to then be from the same instance). SolrClientCache creates
> new ones.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-16903) Use Path instead of File in Java Code
[ https://issues.apache.org/jira/browse/SOLR-16903?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922892#comment-17922892 ] ASF subversion and git services commented on SOLR-16903: Commit b32b6758c704a7b7e538a965af23e1406abe371d in solr's branch refs/heads/main from Matthew Biscocho [ https://gitbox.apache.org/repos/asf?p=solr.git;h=b32b6758c70 ] SOLR-16903: Migrate from java.io.File to java.nio.file.Path in solr-core (#2924) Co-authored-by: Matthew Biscocho > Use Path instead of File in Java Code > - > > Key: SOLR-16903 > URL: https://issues.apache.org/jira/browse/SOLR-16903 > Project: Solr > Issue Type: Improvement >Affects Versions: 9.3 >Reporter: Eric Pugh >Priority: Minor > Labels: newdev, pull-request-available > Time Spent: 4h 50m > Remaining Estimate: 0h > > As a community, we have decided to migrate to using java.nio.file.Path in > place of java.io.File in our codebase. > This ticket is to go through the codebase and make that change. We'd also > like to add the java.io.File pattern to our forbidden-apis setup. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Updated] (SOLR-16903) Use Path instead of File in Java Code
[ https://issues.apache.org/jira/browse/SOLR-16903?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] David Smiley updated SOLR-16903: Fix Version/s: main (10.0) > Use Path instead of File in Java Code > - > > Key: SOLR-16903 > URL: https://issues.apache.org/jira/browse/SOLR-16903 > Project: Solr > Issue Type: Improvement >Affects Versions: 9.3 >Reporter: Eric Pugh >Priority: Minor > Labels: newdev, pull-request-available > Fix For: main (10.0) > > Time Spent: 4h 50m > Remaining Estimate: 0h > > As a community, we have decided to migrate to using java.nio.file.Path in > place of java.io.File in our codebase. > This ticket is to go through the codebase and make that change. We'd also > like to add the java.io.File pattern to our forbidden-apis setup. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16903: Migrate off java.io.File to java.nio.file.Path from core files [solr]
dsmiley merged PR #2924: URL: https://github.com/apache/solr/pull/2924 -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17609: mark deprecation of HDFS module in 9x and removal in 10 [solr]
dsmiley commented on code in PR #3041: URL: https://github.com/apache/solr/pull/3041#discussion_r1938174347 ## solr/solr-ref-guide/modules/deployment-guide/pages/solr-on-hdfs.adoc: ## @@ -20,6 +20,8 @@ The Solr HDFS Module has support for writing and reading Solr's index and transaction log files to the HDFS distributed filesystem. It does not use Hadoop MapReduce to process Solr data. +IMPORTANT: The HDFS Module has been deprecated and will be removed in Solr 10. Review Comment: There is asciidoc markup for important notices -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17609: Remove HDFS module [solr]
dsmiley commented on code in PR #2923: URL: https://github.com/apache/solr/pull/2923#discussion_r1938182884 ## solr/modules/hdfs/src/java/org/apache/solr/hdfs/store/blockcache/package-info.java: ## Review Comment: I propose that the "blockcache" java package, and corresponding tests, be moved into the solr-core module. The javadocs for the BlockCache package is misleading. It says "An HDFS blockcache implementation" but the BlockCache actually has no outwards dependencies aside from Lucene and basic stuff. A more accurate characterization of the package is: "A generic Directory layer/wrapper that caches data, on or off heap as desired". This is a hidden gem with remarkable intellectual property that our project will hopefully use on top of another Directory. For example if DirectoryFactory uses NIO better, we might very well use this with GCP FileSystemProvider, which just so happens to already be in our dependencies. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17623: Simple ordered map should implement map [solr]
renatoh commented on PR #3048: URL: https://github.com/apache/solr/pull/3048#issuecomment-2627271485 > As you are a very new contributor, I can tell by your changes and your last question that you have not been running `./gradlew precommit`, which will detect issues that you can correct before pushing. > BTW all source files need a header. IntelliJ does it automatically for me, as I've configured it to. Ah yes, I've forgot about that one, was for some reason under the impression running the 'tidy' is sufficient, now I know:) -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Updated] (SOLR-17632) Text to Vector Update Request Processor
[ https://issues.apache.org/jira/browse/SOLR-17632?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] ASF GitHub Bot updated SOLR-17632: -- Labels: pull-request-available (was: ) > Text to Vector Update Request Processor > --- > > Key: SOLR-17632 > URL: https://issues.apache.org/jira/browse/SOLR-17632 > Project: Solr > Issue Type: New Feature > Components: UpdateRequestProcessors >Reporter: Alessandro Benedetti >Assignee: Alessandro Benedetti >Priority: Major > Labels: pull-request-available > Time Spent: 10m > Remaining Estimate: 0h > > Scope of this issue is to introduce support for automatic text vectorisation > in Apache Solr, directly in a update request processor. > A LLM fine-tuned for sentence similarity will be accessed to embed the text. > Apache Solr will host the configuration parameters to access embedding > services and the update request processor will use such services to directly > encode the document field as a vector. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[PR] SOLR-17632: Text to Vector Update Request Processor [solr]
alessandrobenedetti opened a new pull request, #3151: URL: https://github.com/apache/solr/pull/3151 https://issues.apache.org/jira/browse/SOLR-17632 # Description Scope of this issue is to introduce support for automatic text vectorisation in Apache Solr. A LLM fine-tuned for sentence similarity will be accessed to vectorise the text. Apache Solr hosts the configuration parameters to access vectorision. # Solution An Update Request Processor that access the managed resource and call the vectorisation service for each document to process. # Tests Unit and integration tests have been added # Checklist Please review the following and check all that apply: - [ ] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) and my code conforms to the standards described there to the best of my ability. - [ ] I have created a Jira issue and added the issue ID to my pull request title. - [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation) - [ ] I have developed this patch against the `main` branch. - [ ] I have run `./gradlew check`. - [ ] I have added tests for my changes. - [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide) -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-XXXXX: Support different filesystem implementations with EmbeddedSolrServer [solr]
AndreyBozhko commented on code in PR #3146:
URL: https://github.com/apache/solr/pull/3146#discussion_r1937640688
##
solr/core/src/test/org/apache/solr/client/solrj/embedded/TestEmbeddedSolrServerConstructors.java:
##
@@ -58,4 +63,63 @@ 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);
Review Comment:
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?
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
alessandrobenedetti commented on PR #3151: URL: https://github.com/apache/solr/pull/3151#issuecomment-2627858272 Let's give it a first round of discussion/brainstorming/improvements. Then I'll adjust the gradle check, documentation and changes. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17642) Knn queries use multi threaded search unintentionally
[
https://issues.apache.org/jira/browse/SOLR-17642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922850#comment-17922850
]
Varun Thacker commented on SOLR-17642:
--
👋 [~cpoerschke] ! Thanks for the tis. Yes that's definitely what I am going to
try to mitigate the bug till we figure out the actual fix
> Knn queries use multi threaded search unintentionally
> ---
>
> Key: SOLR-17642
> URL: https://issues.apache.org/jira/browse/SOLR-17642
> Project: Solr
> Issue Type: Bug
>Affects Versions: 9.7, 9.8
>Reporter: Varun Thacker
>Priority: Major
>
> First reported on
> [https://lists.apache.org/thread/xn1xg5szxhrs0sbcy4gmx4cvohy6flvh|https://lists.apache.org/thread/xn1xg5szxhrs0sbcy4gmx4cvohy6flvh,],
> Dr. Andreas Moll reported a failing query.
> I was able to reproduce is consistently locally on 9.7 and 9.8 using Dr.
> Andreas Moll's repro steps.
> There are two bugs in play here
> *The Solr side bug:*
> * Solr 9.7 added SOLR-13350 but disabled multi-threaded search by default. I
> can confirm the failing query follows this code path
>
> {code:java}
> if (!MultiThreadedSearcher.allowMT(pf.postFilter, cmd)) {
> log.trace("SINGLE THREADED search, skipping collector manager in
> getDocListNC");{code}
>
> * However when following that code path the query goes to
> AbstractKnnVectorQuery and this is where executor thread pool gets used
> {code:java}
> TimeLimitingKnnCollectorManager knnCollectorManager =
> new TimeLimitingKnnCollectorManager(
> getKnnCollectorManager(k, indexSearcher), indexSearcher.getTimeout());
> TaskExecutor taskExecutor = indexSearcher.getTaskExecutor();
> {code}
> * This executor was added in SOLR-13350 in CoreContainer(
> indexSearcherExecutor) . If you follow the SolrIndexSearcher code that calls
> {code:java}
> In SolrIndexSearcher.java
> super(wrapReader(core, r),
> core.getCoreContainer().getIndexSearcherExecutor());{code}
> {code:java}
> In IndexSearcher.java
> this.executor = executor;
> this.taskExecutor =
> executor == null ? new TaskExecutor(Runnable::run) : new
> TaskExecutor(executor);{code}
> I think the idea in SOLR-13350 was create an indexSearcherExector, but only
> use it when multiThreaded=true as a query param (default=false). The fact we
> initialised indexSearcherExector means all KNN queries use that thread pool
> and there is no way to disable that.
>
> *Now the Lucene Bug:*
> [~benwtrent] noted on
> [https://lists.apache.org/thread/ftm6f32f2nsoys6noxx8p2ygpwnlfjc9] that
> Multi-threaded vector search over multiple segments can lead to inconsistent
> results
>
> So the fact Solr by default always uses multi threaded search 9.7+ triggers
> the lucene bug. But I think in Solr we should also have a way to not use
> multi-threaded KNN like we do for other queries.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16903: Migrate off java.io.File to java.nio.file.Path from core files [solr]
mlbiscoc commented on code in PR #2924:
URL: https://github.com/apache/solr/pull/2924#discussion_r1937718233
##
solr/core/src/java/org/apache/solr/core/SolrPaths.java:
##
@@ -94,7 +94,7 @@ public static void assertNoPathTraversal(Path pathToAssert) {
/** Asserts that a path is not a Windows UNC path */
public static void assertNotUnc(Path pathToAssert) {
-if (OS.isFamilyWindows() && pathToAssert.toString().startsWith("")) {
+if (OS.isFamilyWindows() || pathToAssert.toString().startsWith("")) {
Review Comment:
Thats true but currently the assertion is skipped on non-windows even if the
path is a UNC path because of the AND when I thought this function was to only
check UNC but maybe that was for a reason.
I'm going to revert this and revert `openResource` back to its original
assertion.
##
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##
@@ -1354,16 +1353,18 @@ public void initializeMetrics(SolrMetricsContext
parentContext, String scope) {
Category.CORE.toString());
}
-// TODO SOLR-8282 move to PATH
// initialize disk total / free metrics
-Path dataDirPath = Paths.get(dataDir);
-File dataDirFile = dataDirPath.toFile();
-parentContext.gauge(
-() -> dataDirFile.getTotalSpace(), true, "totalSpace",
Category.CORE.toString(), "fs");
-parentContext.gauge(
-() -> dataDirFile.getUsableSpace(), true, "usableSpace",
Category.CORE.toString(), "fs");
+Path dataDirFile = Paths.get(dataDir);
+
+// Do not pre-compute the data directory total/usable space on
initialization. Calculated when
+// metric is fetched
+// TODO Move metrics initialization calls to after the data directory is
created to fetch true
Review Comment:
I had a misunderstanding of how this initialization of gauges worked. I
changed it to what I think should be the correct conversion to Path and seems
to work testing.
##
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java:
##
@@ -357,7 +356,7 @@ public static NamedList getCoreStatus(
if (core != null) {
info.add(NAME, core.getName());
info.add("instanceDir", core.getInstancePath().toString());
-info.add("dataDir", normalizePath(core.getDataDir()));
+info.add("dataDir", Path.of(core.getDataDir()).toString());
Review Comment:
Found a place in `FileUtils` and brought it back. Also just switched to
`FileSystems.getDefault().getSeparator()` and brought all the logic to be
careful of regressions since it seems neither of us can confirm fully on a
windows machine.
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17582) CLUSTERSTATUS API could stream the response to save memory
[ https://issues.apache.org/jira/browse/SOLR-17582?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922863#comment-17922863 ] Matthew Biscocho commented on SOLR-17582: - I saw SOLR-17623 merged in. I'll go ahead and start making the changes for streaming with Solr version. > CLUSTERSTATUS API could stream the response to save memory > -- > > Key: SOLR-17582 > URL: https://issues.apache.org/jira/browse/SOLR-17582 > Project: Solr > Issue Type: Improvement > Components: SolrCloud >Reporter: David Smiley >Priority: Minor > Labels: pull-request-available > Fix For: 9.9 > > Time Spent: 3h 40m > Remaining Estimate: 0h > > The CLUSTERSTATUS API currently creates the entire response before submitting > it. If there are many thousands of collections, this will use up memory. It > could easily be modified to stream the response to compute each collection > information just-in-time. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17334) Minor bugs in Solr dedicated coordinator mode
[
https://issues.apache.org/jira/browse/SOLR-17334?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922900#comment-17922900
]
Noble Paul commented on SOLR-17334:
---
yes, this can be closed now
> Minor bugs in Solr dedicated coordinator mode
> -
>
> Key: SOLR-17334
> URL: https://issues.apache.org/jira/browse/SOLR-17334
> Project: Solr
> Issue Type: Improvement
> Components: SolrCloud
>Affects Versions: 9.6
>Reporter: Torsten Bøgh Köster
>Assignee: Noble Paul
>Priority: Major
> Labels: pull-request-available
> Time Spent: 3h 20m
> Remaining Estimate: 0h
>
> We recently put dedicated Solr coordinator nodes into production and stumbled
> upon some minor bugs/improvements:
> - The Solr root resource cannot be requested on a coordinator node
> - Coordinator requests are enabled for the {{/select}} handler only
> - From outside proxied and coordinator requests cannot be distinguished
> h3. Solr root resource
> We adopted a general fix from the {{HttpSolrCall}} to check whether the given
> {{collectionName}} is {{{}null{}}}. This fixes requesting the root resource
> (and maybe other similar requests)
> h3. {{/select}} handler only
> Coordinator requests are limited to the {{/select}} handler. In our
> environment we make heavy usage of pre-configured Solr handlers. We could not
> find any reason to limit coordinator calls to the {{/select}} handler and
> removed the limitation.
> h3. Coordinator requests cannot be identified by Solr response
> With any Solr response you cannot distinguish coordinator from proxied (or
> regular) requests. While this is great for consistency it makes testing extra
> hard. We added an extra Solr header field with the coordinator node name
> called {{requestCoordinatorNode}} when debug is enabled. This eases testing
> and debugging a lot.
>
> A Pull Request is in the making!
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17623: Simple ordered map should implement map [solr]
dsmiley merged PR #3048: URL: https://github.com/apache/solr/pull/3048 -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17623) SimpleOrderedMap should implement Map
[ https://issues.apache.org/jira/browse/SOLR-17623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922846#comment-17922846 ] ASF subversion and git services commented on SOLR-17623: Commit dac5fe1ea8375eaaa92be37fb2831fbc7cb2c9cc in solr's branch refs/heads/main from Renato Haeberli [ https://gitbox.apache.org/repos/asf?p=solr.git;h=dac5fe1ea83 ] SOLR-17623: SimpleOrderedMap implements Map (#3048) Also added NamedList.indexOf(name) convenience method. Does *not* adjust asShallowMap() yet. Coming soon. - Co-authored-by: David Smiley > SimpleOrderedMap should implement Map > - > > Key: SOLR-17623 > URL: https://issues.apache.org/jira/browse/SOLR-17623 > Project: Solr > Issue Type: Improvement >Reporter: David Smiley >Priority: Major > Labels: newdev, pull-request-available > Time Spent: 3h 10m > Remaining Estimate: 0h > > SimpleOrderedMap is semantically a Map; it should implement Map. > Why: This will help us transition away from NamedList (it's superclass) in a > number of places, since many (most?) places that are defined to return a > NamedList could actually be declared to be a SimpleOrderedMap and eventually > simply Map. > There's some risk that code somewhere gets this Map, a large one, and then > assumes it has better than O(N) lookup, which it doesn't provide. Perhaps a > Javadoc warning will do. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16903: Migrate off java.io.File to java.nio.file.Path from core files [solr]
dsmiley commented on code in PR #2924:
URL: https://github.com/apache/solr/pull/2924#discussion_r1937982261
##
solr/core/src/java/org/apache/solr/core/DirectoryFactory.java:
##
@@ -369,15 +370,17 @@ public void cleanupOldIndexDirectories(
int i = 0;
if (afterCoreReload) {
- log.info("Will not remove most recent old directory after reload {}",
dirsList.getFirst());
+ if (log.isInfoEnabled())
+log.info("Will not remove most recent old directory after reload {}",
dirsList.getFirst());
i = 1;
}
-log.info(
-"Found {} old index directories to clean-up under {} afterReload={}",
-dirsList.size() - i,
-dataDirPath,
-afterCoreReload);
+if (log.isInfoEnabled())
Review Comment:
why did you add this if log.isInfoEnabled check? I confess I hate them.
You may see them commonly in the codebase but it's only in certain scenarios
that a code checker will complains about it (`precommit` will complain if so);
there's no universal requirement or style. If it didn't complain, please leave
it be.
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Resolved] (SOLR-17623) SimpleOrderedMap should implement Map
[ https://issues.apache.org/jira/browse/SOLR-17623?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] David Smiley resolved SOLR-17623. - Fix Version/s: 9.9 Resolution: Fixed Nice; thanks again Renato! If you are interested in more nearby work, you might find the linked issue SOLR-17635 advances a goal that will very much use this. Or maybe work on NamedList.asShallowMap going away by altering its callers. As I look at its callers, it will have varied solutions, so maybe harder to work on. I once sought improvement to NamedList.asShallowMap but as I thought about it, it ought to only be used when we know the NamedList is effectively a map (no repeating keys) but in that case, ought to already be a SimpleOrderedMap (some work needed to transition some impls) and thus SOM should implementing Map; and there are we are today. > SimpleOrderedMap should implement Map > - > > Key: SOLR-17623 > URL: https://issues.apache.org/jira/browse/SOLR-17623 > Project: Solr > Issue Type: Improvement >Reporter: David Smiley >Priority: Major > Labels: newdev, pull-request-available > Fix For: 9.9 > > Time Spent: 3h 10m > Remaining Estimate: 0h > > SimpleOrderedMap is semantically a Map; it should implement Map. > Why: This will help us transition away from NamedList (it's superclass) in a > number of places, since many (most?) places that are defined to return a > NamedList could actually be declared to be a SimpleOrderedMap and eventually > simply Map. > There's some risk that code somewhere gets this Map, a large one, and then > assumes it has better than O(N) lookup, which it doesn't provide. Perhaps a > Javadoc warning will do. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Resolved] (SOLR-17552) NamedList.asShallowMap improvements
[ https://issues.apache.org/jira/browse/SOLR-17552?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] David Smiley resolved SOLR-17552. - Resolution: Won't Fix > NamedList.asShallowMap improvements > --- > > Key: SOLR-17552 > URL: https://issues.apache.org/jira/browse/SOLR-17552 > Project: Solr > Issue Type: Improvement >Reporter: David Smiley >Assignee: David Smiley >Priority: Minor > Labels: pull-request-available > Time Spent: 20m > Remaining Estimate: 0h > > NamedList.asShallowMap could use some improvements. > * javadocs > * putAll doesn't honor allowDups to add new pairs for existing keys > * several // TODO implement more efficiently -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Commented] (SOLR-17623) SimpleOrderedMap should implement Map
[ https://issues.apache.org/jira/browse/SOLR-17623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17922865#comment-17922865 ] ASF subversion and git services commented on SOLR-17623: Commit a6bb507304674f0cc3e5f85fed0ef233de4a2fe5 in solr's branch refs/heads/branch_9x from Renato Haeberli [ https://gitbox.apache.org/repos/asf?p=solr.git;h=a6bb5073046 ] SOLR-17623: SimpleOrderedMap implements Map (#3048) Also added NamedList.indexOf(name) convenience method. Does *not* adjust asShallowMap() yet. Coming soon. - Co-authored-by: David Smiley (cherry picked from commit dac5fe1ea8375eaaa92be37fb2831fbc7cb2c9cc) > SimpleOrderedMap should implement Map > - > > Key: SOLR-17623 > URL: https://issues.apache.org/jira/browse/SOLR-17623 > Project: Solr > Issue Type: Improvement >Reporter: David Smiley >Priority: Major > Labels: newdev, pull-request-available > Time Spent: 3h 10m > Remaining Estimate: 0h > > SimpleOrderedMap is semantically a Map; it should implement Map. > Why: This will help us transition away from NamedList (it's superclass) in a > number of places, since many (most?) places that are defined to return a > NamedList could actually be declared to be a SimpleOrderedMap and eventually > simply Map. > There's some risk that code somewhere gets this Map, a large one, and then > assumes it has better than O(N) lookup, which it doesn't provide. Perhaps a > Javadoc warning will do. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Updated] (SOLR-17641) Support Java 24
[ https://issues.apache.org/jira/browse/SOLR-17641?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] ASF GitHub Bot updated SOLR-17641: -- Labels: pull-request-available (was: ) > Support Java 24 > --- > > Key: SOLR-17641 > URL: https://issues.apache.org/jira/browse/SOLR-17641 > Project: Solr > Issue Type: Bug > Components: SolrCLI, Tests >Reporter: Houston Putman >Priority: Critical > Labels: pull-request-available > Time Spent: 10m > Remaining Estimate: 0h > > The main thing here is to disable the Security Manager for Java 24+, both for > the CLI and testing. > Reference: https://github.com/apache/lucene/issues/14184 -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
[jira] [Updated] (SOLR-17379) ParsingFieldUpdateProcessorsTest failures using CLDR locale provider
[
https://issues.apache.org/jira/browse/SOLR-17379?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated SOLR-17379:
--
Labels: pull-request-available (was: )
> ParsingFieldUpdateProcessorsTest failures using CLDR locale provider
>
>
> Key: SOLR-17379
> URL: https://issues.apache.org/jira/browse/SOLR-17379
> Project: Solr
> Issue Type: Test
>Reporter: Chris M. Hostetter
>Priority: Major
> Labels: pull-request-available
> Attachments: SOLR-17379.test-1.patch, SOLR-17379.test.patch
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Background: https://lists.apache.org/thread/o7xwz8df6j0bx7w2m3w8ptrp4r7q957n
> Test failures from {{ParsingFieldUpdateProcessorsTest.testAKSTZone}} and
> {{ParsingFieldUpdateProcessorsTest.testParseFrenchDate}} are seemingly
> guaranteed on JDK23, due to the removal of the {{COMPAT}} local provider
> option.
> On (some) earlier JDKs, these failures can be reproduced using...
> {noformat}
> ./gradlew test --tests ParsingFieldUpdateProcessorsTest
> -Ptests.jvmargs="-Djava.locale.providers=CLDR -XX:TieredStopAtLevel=1
> -XX:+UseParallelGC -XX:ActiveProcessorCount=1 -XX:ReservedCodeCacheSize=120m"
> {noformat}
> ...to force the use off {{CLDR}} and exclude the use of {{COMPAT}}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
[PR] SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityManager [solr]
HoustonPutman opened a new pull request, #3154: URL: https://github.com/apache/solr/pull/3154 https://issues.apache.org/jira/browse/SOLR-17379 -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16903: Migrate off java.io.File to java.nio.file.Path from core files [solr]
mlbiscoc commented on code in PR #2924:
URL: https://github.com/apache/solr/pull/2924#discussion_r1938046715
##
solr/core/src/java/org/apache/solr/core/DirectoryFactory.java:
##
@@ -369,15 +370,17 @@ public void cleanupOldIndexDirectories(
int i = 0;
if (afterCoreReload) {
- log.info("Will not remove most recent old directory after reload {}",
dirsList.getFirst());
+ if (log.isInfoEnabled())
+log.info("Will not remove most recent old directory after reload {}",
dirsList.getFirst());
i = 1;
}
-log.info(
-"Found {} old index directories to clean-up under {} afterReload={}",
-dirsList.size() - i,
-dataDirPath,
-afterCoreReload);
+if (log.isInfoEnabled())
Review Comment:
Precommit was complaining because of the `getFirst()` call in the log which
was why I added it. I also don't like it either.
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
