Re: [PR] SOLR-17043: Remove SolrClient path pattern matching [solr]
kotman12 commented on code in PR #3238: URL: https://github.com/apache/solr/pull/3238#discussion_r1982512886 ## solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java: ## @@ -185,8 +188,31 @@ public void setQueryParams(Set queryParams) { this.queryParams = queryParams; } - /** This method defines the type of this Solr request. */ - public abstract String getRequestType(); + /** + * Defines the intended type of this Solr request. + * + * Subclasses should typically override this method instead of {@link + * SolrRequest#getRequestType}. Note that changing request type can break/impact request routing + * within various clients (i.e. {@link CloudSolrClient}). + */ + protected SolrRequestType getBaseRequestType() { +return SolrRequestType.UNSPECIFIED; + } + + /** + * Pattern matches on the underlying {@link SolrRequest} to identify ADMIN requests and other + * special cases. If no special case is identified, {@link SolrRequest#getBaseRequestType()} is + * returned. + */ + public SolrRequestType getRequestType() { +if (CommonParams.ADMIN_PATHS.contains(getPath())) { + return SolrRequestType.ADMIN; +} else if (this instanceof IsUpdateRequest) { Review Comment: If we are marking as deprecated then maybe it makes sense to stop adding code that relies on this? ## solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java: ## @@ -185,8 +188,31 @@ public void setQueryParams(Set queryParams) { this.queryParams = queryParams; } - /** This method defines the type of this Solr request. */ - public abstract String getRequestType(); + /** + * Defines the intended type of this Solr request. + * + * Subclasses should typically override this method instead of {@link + * SolrRequest#getRequestType}. Note that changing request type can break/impact request routing + * within various clients (i.e. {@link CloudSolrClient}). + */ + protected SolrRequestType getBaseRequestType() { +return SolrRequestType.UNSPECIFIED; + } + + /** + * Pattern matches on the underlying {@link SolrRequest} to identify ADMIN requests and other + * special cases. If no special case is identified, {@link SolrRequest#getBaseRequestType()} is + * returned. + */ + public SolrRequestType getRequestType() { Review Comment: Should this be `final` if you only expect `getBaseRequestType` to be overridden? ## solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java: ## @@ -185,8 +188,31 @@ public void setQueryParams(Set queryParams) { this.queryParams = queryParams; } - /** This method defines the type of this Solr request. */ - public abstract String getRequestType(); + /** + * Defines the intended type of this Solr request. + * + * Subclasses should typically override this method instead of {@link + * SolrRequest#getRequestType}. Note that changing request type can break/impact request routing + * within various clients (i.e. {@link CloudSolrClient}). + */ + protected SolrRequestType getBaseRequestType() { Review Comment: Just thinking out loud: I wonder if it is possible to make this a member of `SolrRequest` itself, i.e. `private SolrRequestType type` The advantage of this is you don't have to expand the interface with this arguably low-level detail. You could imagine a set of overloaded constructors of `SolrRequest`, i.e.: ``` private SolrRequestType type; public SolrRequest(METHOD m, String path) { this(m, path, UNSPECIFIED); } public SolrRequest(METHOD m, String path, SolrRequestType defaultType) { this.method = m; this.path = path; this.type = resolveType(path, defaultType); } public void setPath(String path) { this.path = path; this.type = resolveType(path, type); } public SolrRequestType getRequestType() { return type; } ``` The catch is that calling `setPath` has to update this additional property as well. But because `getRequestType` already depends on `path`, both solutions are equally mutable. It would be nice if `SolrRequest` had a builder which could unburden `SolrRequest` from this mutability but that would be a much bigger change I suppose. ## solr/solrj/src/java/org/apache/solr/client/solrj/request/SolrPing.java: ## @@ -53,8 +53,9 @@ public ModifiableSolrParams getParams() { } @Override - public String getRequestType() { -return SolrRequestType.ADMIN.toString(); + /* This request is not processed as an ADMIN request. */ + protected SolrRequestType getBaseRequestType() { +return SolrRequestType.UNSPECIFIED; Review Comment: We know that this is a ping request so this feels misleading .. can the `SolrRequestType` enum be extended to reflect this, i.e. `SolrRequestType.PING` or `SolrRequestType.NO_SIDE_EFFECT_ADMIN`? -- This is an
Re: [PR] SOLR-17690 Make solr zk tools read ZK_HOST environment [solr]
janhoy commented on code in PR #3240: URL: https://github.com/apache/solr/pull/3240#discussion_r1981064826 ## solr/core/src/java/org/apache/solr/cli/RunExampleTool.java: ## @@ -305,7 +305,7 @@ protected void runExample(CommandLine cli, String exampleName) throws Exception "techproducts".equals(exampleName) ? "sample_techproducts_configs" : "_default"; boolean isCloudMode = !cli.hasOption(USER_MANAGED_OPTION); -String zkHost = cli.getOptionValue(CommonCLIOptions.ZK_HOST_OPTION); +String zkHost = CLIUtils.getZkHostFromCliOrEnv(cli); Review Comment: @epugh Not sure if runExample needs to read ENV. I mean, it is more of a canned experience, where we intentionally spin up an embedded ZK, right? But since it here reads the `--zk-host` option it must have been some thought that you should be able to run an example on an external Zookeeper, and if that even makes sense, then it follows that you should have the choice of also usin ZK_HOST env. ``` -z/--zk-host Zookeeper connection string; only used when running in SolrCloud mode using -c If neither ZK_HOST is defined in solr.in.sh nor the -z parameter is specified, an embedded ZooKeeper instance will be launched. Set the ZK_CREATE_CHROOT environment variable to true if your ZK host has a chroot path, and you want to create it automatically. ``` There is of course a risk that some user has defined a `ZK_HOST´ in her local terminal, and that being picked up by magic when running an example. Thoughts? -- 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-17690) Solr CLI tools like zk cp does not read ZK_HOST
[ https://issues.apache.org/jira/browse/SOLR-17690?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jan Høydahl updated SOLR-17690: --- Fix Version/s: 9.9 > Solr CLI tools like zk cp does not read ZK_HOST > --- > > Key: SOLR-17690 > URL: https://issues.apache.org/jira/browse/SOLR-17690 > Project: Solr > Issue Type: Bug > Components: cli, scripts and tools >Affects Versions: 9.8.0 >Reporter: Jan Høydahl >Assignee: Jan Høydahl >Priority: Major > Labels: pull-request-available > Fix For: 9.9, 9.8.1 > > Time Spent: 1h 10m > Remaining Estimate: 0h > > As reported in solr operator issue > [https://github.com/apache/solr-operator/issues/762] there is a regression in > solr-operator 0.9 which uses {{solr zk cp}} instead of {{{}zkCli.sh{}}}, so > that when using a combo of operator 0.9 and Solr 9.8.0, the script will fail > due to not picking up {{ZK_HOST}} env.var. > Older solr-operator used {{zkCli.sh}} which read the env.var but had other > issues. Pre 9.8 versions of Solr parsed "solr zk cp" command in bash instead > of Java and thus works. > The solution is to enable CLI tools to get ZK_HOST from environment if > {{--zk-host}} is not passed. -- 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-17690 Make solr zk tools read ZK_HOST environment [solr]
janhoy commented on PR #3240: URL: https://github.com/apache/solr/pull/3240#issuecomment-2700380093 I ran crave locally since it does not work in gh-actions: ``` ../crave run Downloading update https://github.com/accupara/crave/releases/download/0.2-7064/crave-0.2-7064-darwin-amd64.bin Selecting project Solr (id:39) Picking up local changes (if any)... 3 files changed, 25 insertions(+), 10 deletions(-) No build commands specified Running preconfigured build commands ./gradlew --console=plain test To run custom build commands use crave [options] run [options] -- "customCommand1; customCommand2; ..." Waiting for build id:185108 to start... Services in this job can be accessed using the following: ServiceRemote Access URL --- shellinabox 5898 shellinabox://localhost:5898 ssh22 ssh://localhost:41528 vscode 5899 vscode://localhost:5899 Setting up workspace (this could take some time)... Pulling container image accupara/openjdk:21... Finished pulling container image accupara/openjdk@sha256:8f36c8fd41df83a9773ac84785b5b16d29fd4eb5c94c1cc49327785bddca183e took 1.088967291s Switched to a new branch 'feature/SOLR-17690-zkToolZKHostFromEnv' setting commitID to e668dceb6bec0b838d5f1e045483432abd6b6612 From /home/admin/.craved/GUnip8pjhjmR-561c264fd117b4e8a8d25c057caef7c71e43099d.patch.gz.b64 * branchHEAD -> FETCH_HEAD Updating e668dceb6be..1580a6c19c6 Fast-forward .../src/java/org/apache/solr/cli/CLIUtils.java | 25 ++ .../java/org/apache/solr/cli/RunExampleTool.java | 4 ++-- .../org/apache/solr/cli/ZkSubcommandsTest.java | 6 ++ 3 files changed, 25 insertions(+), 10 deletions(-) Starting a Gradle Daemon, 1 incompatible and 1 stopped Daemons could not be reused, use --status for details [--snip--] > Task :solr:core:wipeTaskTemp The slowest tests (exceeding 500 ms) during this run: 149.58s BasicAuthIntegrationTest.testBasicAuth (:solr:core) 70.52s PeerSyncReplicationTest.test (:solr:core) 66.55s TestDistributedSearch.test (:solr:core) 65.63s ReplicationFactorTest.test (:solr:core) 64.00s BasicDistributedZkTest.test (:solr:core) 57.93s TestCpuAllowedLimit.testDistribLimit (:solr:core) 54.98s BasicDistributedZk2Test.test (:solr:core) 51.94s SyncSliceTest.test (:solr:core) 50.02s UnloadDistributedZkTest.test (:solr:core) 45.63s SolrAndKafkaReindexTest.testFullCloudToCloud (:solr:cross-dc-manager) The slowest suites (exceeding 1s) during this run: 149.65s BasicAuthIntegrationTest (:solr:core) 113.65s SchemaTest (:solr:solrj) 111.93s TestRecovery (:solr:core) 107.73s TestCpuAllowedLimit (:solr:core) 96.31s MultiThreadedOCPTest (:solr:core) 89.91s TestCollectionAPI (:solr:core) 85.43s TestPullReplica (:solr:core) 82.79s TestCoordinatorRole (:solr:core) 77.73s StreamDecoratorTest (:solr:solrj-streaming) 75.33s PeerSyncReplicationTest (:solr:core) BUILD SUCCESSFUL in 6m 32s 146 actionable tasks: 142 executed, 4 up-to-date Build Successful Total time: 7m22.5s ``` -- 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-17690 Make solr zk tools read ZK_HOST environment [solr]
janhoy commented on PR #3240: URL: https://github.com/apache/solr/pull/3240#issuecomment-2700391514 Hmm, there's no `9.8.1` section in CHANGES.txt on `main`, and not on `branch_9x`, only on `branch_9_8`. Is the correct way for this bugfix to skip CHANGES here and on 9x, and then add one when cherry-picking to the release branch? Then the 9.8.1 RM will reconsile changes to other branches during release? Here is suggested changes text: ``` * SOLR-17690: Make solr CLI tools read ZK_HOST environment as documented (janhoy) ``` -- 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] SmokeTestRelease Added support for multiple JDK [solr]
janhoy commented on PR #2685: URL: https://github.com/apache/solr/pull/2685#issuecomment-2700615649 @iamsanjay I merged in main branch and resolved conflicts the best I could. I also updated README.md with the latest usage text for the tool, and took it for a spin and fixed a bug with an uninitialized variable. However, I don't have a release candidate at hand for testing with. @HoustonPutman are you willing to be the first to test this generalization for the 9.8.1 release? It basically lets you test with one or more alternative JDKs instead of hardcoded versions as in todays script. So you could ```bash dev-tools/scripts/smokeTestRelease.py --revision 9.8.1 --test-alt-java /path/to/java23 --test-alt-java /path/to/java25 ... ``` -- 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
[PR] Make ingress hostname configurable [solr-operator]
ps-muspelkat opened a new pull request, #763: URL: https://github.com/apache/solr-operator/pull/763 This PR addresses the possibility to add an `hostName` to the ingress configuration of a given solrcloud instance as described in #667. The change is quit simple. Unfortunately, the `make prepare` step failed for me at the `fetch-licenses-list` step, so the `dependency_licenses.csv` stays empty in my case. ``` make: *** [fetch-licenses-list] Error 1 ``` To make the e2e-tests start / work, I also needed to change the `RAW_GINKGO` command. Not sure if that is an issue only on my machine. Happy for any feedback! -- 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-17256) Remove SolrRequest.getBasePath setBasePath
[ https://issues.apache.org/jira/browse/SOLR-17256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17932577#comment-17932577 ] Sanjay Dutt commented on SOLR-17256: Is there any easier way to get preferred nodes then this. {code:java} @EndPoint(method = GET, path = "/cluster/node-roles", permission = COLL_READ_PERM) public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { rsp.add( "node-roles", readRecursive( ZkStateReader.NODE_ROLES, collectionsHandler .getCoreContainer() .getZkController() .getSolrCloudManager() .getDistribStateManager(), 3)); } Object readRecursive(String path, DistribStateManager zk, int depth) { if (depth == 0) return null; Map result; try { List children = zk.listData(path); if (children != null && !children.isEmpty()) { result = new HashMap<>(); } else { return Collections.emptySet(); } for (String child : children) { Object c = readRecursive(path + "/" + child, zk, depth - 1); result.put(child, c); } } catch (Exception e) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); } if (depth == 1) { return result.keySet(); } else { return result; } }{code} > Remove SolrRequest.getBasePath setBasePath > -- > > Key: SOLR-17256 > URL: https://issues.apache.org/jira/browse/SOLR-17256 > Project: Solr > Issue Type: Improvement > Components: SolrJ >Reporter: David Smiley >Assignee: Jason Gerlowski >Priority: Minor > Labels: newdev, pull-request-available > Fix For: main (10.0), 9.7 > > Time Spent: 5h 40m > Remaining Estimate: 0h > > SolrRequest has a getBasePath & setBasePath. The naming is poor; it's the > URL base to the Solr node like "http://localhost:8983/solr";. It's only > recognized by HttpSolrClient; LBSolrClient (used by CloudSolrClient) ignores > it and will in fact mutate the passed in request to its liking, which is > rather ugly because it means a request cannot be used concurrently if the > user wants to. But moreover I think there's a conceptual discordance of > placing this concept on SolrRequest given that some clients want to route > requests to nodes *they* choose. I propose removing this from SolrRequest > and instead adding a method specific to HttpSolrClient. Almost all existing > usages of setBasePath immediately execute the request on an HttpSolrClient, > so should be easy to change. -- 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] [Comment Edited] (SOLR-17256) Remove SolrRequest.getBasePath setBasePath
[ https://issues.apache.org/jira/browse/SOLR-17256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17932577#comment-17932577 ] Sanjay Dutt edited comment on SOLR-17256 at 3/5/25 10:18 AM: - Is there any easier way to get preferred nodes than this. {code:java} @EndPoint(method = GET, path = "/cluster/node-roles", permission = COLL_READ_PERM) public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { rsp.add( "node-roles", readRecursive( ZkStateReader.NODE_ROLES, collectionsHandler .getCoreContainer() .getZkController() .getSolrCloudManager() .getDistribStateManager(), 3)); } Object readRecursive(String path, DistribStateManager zk, int depth) { if (depth == 0) return null; Map result; try { List children = zk.listData(path); if (children != null && !children.isEmpty()) { result = new HashMap<>(); } else { return Collections.emptySet(); } for (String child : children) { Object c = readRecursive(path + "/" + child, zk, depth - 1); result.put(child, c); } } catch (Exception e) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); } if (depth == 1) { return result.keySet(); } else { return result; } }{code} was (Author: JIRAUSER305513): Is there any easier way to get preferred nodes then this. {code:java} @EndPoint(method = GET, path = "/cluster/node-roles", permission = COLL_READ_PERM) public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { rsp.add( "node-roles", readRecursive( ZkStateReader.NODE_ROLES, collectionsHandler .getCoreContainer() .getZkController() .getSolrCloudManager() .getDistribStateManager(), 3)); } Object readRecursive(String path, DistribStateManager zk, int depth) { if (depth == 0) return null; Map result; try { List children = zk.listData(path); if (children != null && !children.isEmpty()) { result = new HashMap<>(); } else { return Collections.emptySet(); } for (String child : children) { Object c = readRecursive(path + "/" + child, zk, depth - 1); result.put(child, c); } } catch (Exception e) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); } if (depth == 1) { return result.keySet(); } else { return result; } }{code} > Remove SolrRequest.getBasePath setBasePath > -- > > Key: SOLR-17256 > URL: https://issues.apache.org/jira/browse/SOLR-17256 > Project: Solr > Issue Type: Improvement > Components: SolrJ >Reporter: David Smiley >Assignee: Jason Gerlowski >Priority: Minor > Labels: newdev, pull-request-available > Fix For: main (10.0), 9.7 > > Time Spent: 5h 40m > Remaining Estimate: 0h > > SolrRequest has a getBasePath & setBasePath. The naming is poor; it's the > URL base to the Solr node like "http://localhost:8983/solr";. It's only > recognized by HttpSolrClient; LBSolrClient (used by CloudSolrClient) ignores > it and will in fact mutate the passed in request to its liking, which is > rather ugly because it means a request cannot be used concurrently if the > user wants to. But moreover I think there's a conceptual discordance of > placing this concept on SolrRequest given that some clients want to route > requests to nodes *they* choose. I propose removing this from SolrRequest > and instead adding a method specific to HttpSolrClient. Almost all existing > usages of setBasePath immediately execute the request on an HttpSolrClient, > so should be easy to change. -- 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-17679) Request for Documentation/Feature Improvement on Hybrid Lexical and Vector Search with Score Breakdown and Cutoff Logic
[ https://issues.apache.org/jira/browse/SOLR-17679?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Khaled Alkhouli updated SOLR-17679: --- Status: Patch Available (was: Open) > Request for Documentation/Feature Improvement on Hybrid Lexical and Vector > Search with Score Breakdown and Cutoff Logic > --- > > Key: SOLR-17679 > URL: https://issues.apache.org/jira/browse/SOLR-17679 > Project: Solr > Issue Type: Improvement > Components: search >Affects Versions: 9.6.1 >Reporter: Khaled Alkhouli >Priority: Minor > Labels: hybrid-search, search, solr, vector-based-search > Attachments: Screenshot from 2025-02-20 16-31-48.png > > > Hello Apache Solr team, > I was able to implement a hybrid search engine that combines *lexical search > (edismax)* and *vector search (KNN-based embeddings)* within a single > request. The idea is simple: > * *Lexical Search* retrieves results based on text relevance. > * *Vector Search* retrieves results based on semantic similarity. > * *Hybrid Scoring* sums both scores, where a missing score (if a document > appears in only one search) should be treated as zero. > This approach is working, but *there is a critical lack of documentation* on > how to properly return individual score components of lexical search (score1) > vs. vector search (score2 from cosine similarity). Right now, Solr only > returns the final combined score, but there is no clear way to see {*}how > much of that score comes from lexical search vs. vector search{*}. This is > essential for debugging and for fine-tuning ranking strategies. > > I have implemented the following logic using Python: > {code:java} > def hybrid_search(query, top_k=10): > embedding = np.array(embed([query]), dtype=np.float32 > embedding = list(embedding[0]) > lxq= rf"""{{!type=edismax > qf='text' > q.op=OR > tie=0.1 > bq='' > bf='' > boost='' > }}({query})""" > solr_query = {"params": { > "q": "{!bool filter=$retrievalStage must=$rankingStage}", > "rankingStage": > "{!func}sum(query($normalisedLexicalQuery),query($vectorQuery))", > "retrievalStage":"{!bool should=$lexicalQuery should=$vectorQuery}", > # Union > "normalisedLexicalQuery": "{!func}scale(query($lexicalQuery),0,1)", > "lexicalQuery": lxq, > "vectorQuery": f"{{!knn f=all_v512 topK={top_k}}}{embedding}", > "fl": "text", > "rows": top_k, > "fq": [""], > "rq": "{!rerank reRankQuery=$rqq reRankDocs=100 reRankWeight=3}", > "rqq": "{!frange l=$cutoff}query($rankingStage)", > "sort": "score desc", > }} > response = requests.post(SOLR_URL, headers=HEADERS, json=solr_query) > response = response.json() > return response {code} > h3. *Issues & Missing Documentation* > # *No Way to Retrieve Individual Scores in a Hybrid Search* > There is no clear documentation on how to return: > * > ** The *lexical search score* separately. > ** The *vector search score* separately. > ** The *final combined score* (which Solr already provides). > Right now, we’re left guessing whether the sum of these scores works as > expected, making debugging and tuning unnecessarily difficult. > # *No Clear Way to Implement Cutoff Logic in Solr* > In a hybrid search, I need to filter out results that don’t meet a {*}minimum > score threshold{*}. Right now, I have to implement this in Python, {*}which > defeats the purpose of using Solr for ranking in the first place{*}. > * > ** How can we enforce a {*}score-based cutoff directly in Solr{*}, without > external filtering? > ** The \{!frange} function is mentioned in the documentation but lacks > {*}clear examples on how to apply it to hybrid search{*}. > h3. *Feature Request / Documentation Improvement* > * *Provide a way to return individual scores for lexical and vector search > in the response.* This should be as simple as adding fields like > {{{}fl=score,lexical_score,vector_score{}}}. > * *Clarify how to apply cutoff logic in a hybrid search.* This is an > essential ranking mechanism, and yet, there’s little guidance on how to do > this efficiently within Solr itself. > Looking forward to a response. -- 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-17685: Remove script creation of solr url based on SOLR_TOOL_HOST in favour of java code in CLI tools [solr]
malliaridis commented on code in PR #3223: URL: https://github.com/apache/solr/pull/3223#discussion_r1981850710 ## solr/core/src/java/org/apache/solr/cli/AuthTool.java: ## @@ -75,8 +75,10 @@ public class AuthTool extends ToolBase { .longOpt("solr-include-file") .hasArg() .argName("FILE") + .required() Review Comment: Looking into our source code, this is actually required and always set via the CLI script, and if not set it would likely fail with a NPE I believe? What I am wondering though is, should this be required? Does the file always exist and does it contain information we need? Or should we simply handle cases where it is not defined? ## solr/bin/solr: ## @@ -612,22 +612,11 @@ if [[ "$SCRIPT_CMD" == "auth" ]]; then fi fi - if [ -z "${AUTH_PORT:-}" ]; then -for ID in $(ps auxww | grep java | grep start\.jar | awk '{print $2}' | sort -r) - do -port=$(jetty_port "$ID") -if [ "$port" != "" ]; then - AUTH_PORT=$port - break -fi - done - fi - - run_tool auth $@ --solr-url "$SOLR_URL_SCHEME://$SOLR_TOOL_HOST:${AUTH_PORT:-8983}" --auth-conf-dir "$SOLR_HOME" "--solr-include-file" "$SOLR_INCLUDE" + run_tool auth $@ --auth-conf-dir "$SOLR_HOME" "--solr-include-file" "$SOLR_INCLUDE" exit $? fi -# at this point all tools that have a custom run process, like "status" and "auth" have been run and exited. +# at this point the only tool that has a custom run process, "auth" have been run and exited. Review Comment: ```suggestion # at this point the only tool that has a custom run process, "auth" has been run and exited. ``` ## solr/core/src/java/org/apache/solr/cli/AuthTool.java: ## @@ -96,7 +98,7 @@ public class AuthTool extends ToolBase { .argName("FILE") .required() .desc( - "This is where any authentication related configuration files, if any, would be placed.") + "This is where any authentication related configuration files, if any, would be placed. Defaults to $SOLR_HOME.") Review Comment: If I am not mistaken, this defaults to `SOLR_HOME` only when `AuthTool` is used via the CLI script, but if directly used it does not default to any path. Is that correct? I think we had a discussion before about using the classes without the CLI scripts, but I don't remember anymore if that was a valid scenario. ## solr/bin/solr: ## @@ -612,22 +612,11 @@ if [[ "$SCRIPT_CMD" == "auth" ]]; then fi fi - if [ -z "${AUTH_PORT:-}" ]; then -for ID in $(ps auxww | grep java | grep start\.jar | awk '{print $2}' | sort -r) - do -port=$(jetty_port "$ID") -if [ "$port" != "" ]; then - AUTH_PORT=$port - break -fi - done - fi - - run_tool auth $@ --solr-url "$SOLR_URL_SCHEME://$SOLR_TOOL_HOST:${AUTH_PORT:-8983}" --auth-conf-dir "$SOLR_HOME" "--solr-include-file" "$SOLR_INCLUDE" Review Comment: This eliminates the `AUTH_PORT` and `SOLR_TOOL_HOST` environment variables, right? I believe it is fine, as the behavior is checking the jetty port and falls back to the default Solr port, so no need to distinguish it in the scripts. Still, this is a breaking change. -- 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-17690 Make solr zk tools read ZK_HOST environment [solr]
janhoy commented on PR #3240: URL: https://github.com/apache/solr/pull/3240#issuecomment-2700193133 Refactored a bit, to be able to use the `--zk-host foo --> ZK_HOST=foo` fallback other places that attempt to read only the option. Activated the fix since the Crave tests don't run anyway to show the failing test. I'll run all the tests locally. -- 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-17635: unmarshalling Map to SimpleOrderedMap if key i of type St… [solr]
dsmiley commented on PR #3163: URL: https://github.com/apache/solr/pull/3163#issuecomment-2701019493 Map.get, Map.remove, Map.put -- basic operations are O(1) in a hash based Map, but not in a dense array List that SOM is today. -- 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] fix the solr zk invocation [solr-operator]
gerlowskija commented on code in PR #756: URL: https://github.com/apache/solr-operator/pull/756#discussion_r1981474383 ## controllers/solrcloud_controller_basic_auth_test.go: ## @@ -351,12 +351,12 @@ func expectBasicAuthConfigOnPodTemplateWithGomega(g Gomega, solrCloud *solrv1bet func expectPutSecurityJsonInZkCmd(g Gomega, expInitContainer *corev1.Container) { g.Expect(expInitContainer).To(Not(BeNil()), "Didn't find the setup-zk InitContainer in the sts!") - expCmd := "solr zk cp zk:/security.json /tmp/current_security.json >/dev/null 2>&1; " + + expCmd := "solr zk cp zk:/security.json /tmp/current_security.json --zk-host $ZK_HOST >/dev/null 2>&1; " + Review Comment: [-1] The `--zk-host` syntax is perfect for Solr 9.8, but I think this param/flag changed somewhere in the last few Solr versions. So relying on it in the initContainer would cause failures whenever someone was using e.g. Solr 9.6. Luckily, the "short" version of the opt (`-z`) has stayed the same - so that should be a safer option. Can the PR use that "short" syntax instead? (@epugh - can you double-check me on this. According to the [version matrix docs here](https://github.com/apache/solr-operator/blob/main/docs/upgrade-notes.md#solr-versions), operator 0.9.x currently aims to support >= 8.11, so ideally we'd get a syntax that works through that range. If there's no single syntax that works for the whole range, we could always do the old `||` trick with the minimum set of syntaxes, e.g. ``` solr zk --zk-host $ZK_HOST || solr zk -zkHost $ZK_HOST ``` ) -- 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
[PR] SOLR-17043: Remove SolrClient path pattern matching [solr]
jkmuriithi opened a new pull request, #3238: URL: https://github.com/apache/solr/pull/3238 https://issues.apache.org/jira/browse/SOLR-17043 # Description Currently, some SolrClient implementations (especially our "load-balancing" and "cloud" clients) do pattern-matching on the path to guess the "type" (admin, update, etc. ) of each request. This seems unnecessary though, as SolrRequest already has a "getRequestType" method exposing this. We should use this method where possible instead of the ad-hoc pattern matching we currently do, which is brittle and doesn't map well to the varied paths used by our v2 APIs. # Solution - Use `SolrRequest.getRequestType` to identify request types in SolrJ, and remove other forms of request type identification from the SolrJ codebase. This includes deprecating the `IsUpdateRequest` interface. - Change `getRequestType` to return the actual `SolrRequestType` enum instead of a String. - Some unit tests use `SolrRequest.setPath()` to send a `QueryRequest` to a `/admin` path. This PR adds basic pattern matching in `getRequestType` and adds method `SolrRequest.getBaseRequestType` to promote an inheritance pattern which works with mutable request paths. - `CloudSolrClient` relies heavily on request type to route requests. After modifying the routing in `CloudSolrClient` to use `getRequestType`, several requests started failing because they returned request type `ADMIN` when their handler path was not in `CommonParams.ADMIN_PATHS`. This PR changes the type of those misclassified requests to `UNSPECIFIED`. Some of these changes may seem counterintuitive (i.e. `/admin/ping` and `/admin/luke` are not `ADMIN` requests), but they preserve the existing data flow. # Tests Solr unit tests pass with `./gradlew test`. # 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. - [x] 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`. - [x] I have added tests for my changes. - [x] 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-17690 Make solr zk tools read ZK_HOST environment [solr]
HoustonPutman commented on code in PR #3240: URL: https://github.com/apache/solr/pull/3240#discussion_r1981736330 ## solr/core/src/java/org/apache/solr/cli/RunExampleTool.java: ## @@ -305,7 +305,7 @@ protected void runExample(CommandLine cli, String exampleName) throws Exception "techproducts".equals(exampleName) ? "sample_techproducts_configs" : "_default"; boolean isCloudMode = !cli.hasOption(USER_MANAGED_OPTION); -String zkHost = cli.getOptionValue(CommonCLIOptions.ZK_HOST_OPTION); +String zkHost = CLIUtils.getZkHostFromCliOrEnv(cli); Review Comment: I'm not sure it's too risky, since it's unlikely that it's going to do anything that the user can't undo later. -- 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
[PR] Save model compacted solr 17050 [solr]
babesflorin opened a new pull request, #3242: URL: https://github.com/apache/solr/pull/3242 This pull request includes several updates and improvements to the Solr project, focusing on version updates, bug fixes, and enhancements to the backup functionality. The most important changes include the addition of a new release, updates to the Solr version, and improvements to the backup properties handling. ### Version Updates: * Added a new release `solr-9.4.0` to the `solr.rdf` file. * Updated the Solr version to `9.4.1` in `SolrVersion.java`. ### Backup Functionality Enhancements: * Added support for extra properties in backup details in `CollectionBackupDetails.java`. * Enhanced `BackupProperties` to handle extra properties, including validation and storage. [[1]](diffhunk://#diff-bc5fc6274cb757dc7250efe4451020e77b28da22864d4997998b424dd998a0c3R51-R85) [[2]](diffhunk://#diff-bc5fc6274cb757dc7250efe4451020e77b28da22864d4997998b424dd998a0c3L96-R133) [[3]](diffhunk://#diff-bc5fc6274cb757dc7250efe4451020e77b28da22864d4997998b424dd998a0c3L131-R172) [[4]](diffhunk://#diff-bc5fc6274cb757dc7250efe4451020e77b28da22864d4997998b424dd998a0c3R199-R212) * Updated `BackupCmd` to include extra properties in the backup process. [[1]](diffhunk://#diff-d2e9c4a9811d87e3eccc981a619cbeb8b2ee79ab654f7343229899170a2a5fc7R72) [[2]](diffhunk://#diff-d2e9c4a9811d87e3eccc981a619cbeb8b2ee79ab654f7343229899170a2a5fc7R96-R100) [[3]](diffhunk://#diff-d2e9c4a9811d87e3eccc981a619cbeb8b2ee79ab654f7343229899170a2a5fc7R375-R377) ### Bug Fixes: * Fixed the `json.wrf` parameter issue in `JacksonJsonWriter`. [[1]](diffhunk://#diff-d2bc03d28f4721504e5b0956887dd9223765b8dee702e3e9766a56ef010afc9fR22) [[2]](diffhunk://#diff-d2bc03d28f4721504e5b0956887dd9223765b8dee702e3e9766a56ef010afc9fL37-R39) * Updated `ZkCLI` to use `makePath` instead of `create` for creating paths. [[1]](diffhunk://#diff-e2e943aff04fb27c3583026e8bb5e526eaf6b94d87c3a5e8f0394b12a56fc632L418-R418) [[2]](diffhunk://#diff-e2e943aff04fb27c3583026e8bb5e526eaf6b94d87c3a5e8f0394b12a56fc632L439-R439) ### Dependency Upgrades: * Upgraded multiple dependencies, including `org.glassfish.jersey`, `com.adobe.testing:s3mock-junit4`, `io.netty`, and others in `CHANGES.txt`. ### Documentation and Metadata: * Added new features and improvements to the `CHANGES.txt` file, including the addition of metadata support for backups. These changes collectively enhance the functionality and maintainability of the Solr project, ensuring it stays up-to-date with the latest features and 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] Save model compacted solr 17050 [solr]
babesflorin closed pull request #3242: Save model compacted solr 17050 URL: https://github.com/apache/solr/pull/3242 -- 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
[PR] test pr for copilot [solr]
babesflorin opened a new pull request, #3241: URL: https://github.com/apache/solr/pull/3241 This pull request includes several changes to configuration files, GitHub workflows, and documentation. The most important changes include updates to the `.git-blame-ignore-revs` file, the addition of a new `renovate.json` configuration, and various improvements to GitHub workflows. ### Configuration Updates: * [`.git-blame-ignore-revs`](diffhunk://#diff-f247fbfbe928b907db42554d0b3006b28dd11c25a59be031abda73b11a2c934aR12-R35): Added multiple commit hashes to ignore in git blame annotations. * [`.github/renovate.json`](diffhunk://#diff-f82f7d36a61d22f640c25bdb8b0435bf9cb38679d99d5f460753fa668ee7cdb3R1-R57): Added a new configuration file for Renovate to manage dependency updates with specific rules for grouping and scheduling. ### GitHub Workflow Improvements: * [`.github/workflows/bin-solr-test.yml`](diffhunk://#diff-662880cd7de6908e4d2ac9305af3463dcdd2f50689e2a7773a9c8fb1086825fcR1-R45): Added a new workflow to run Solr script tests on pull requests targeting the main branch. * [`.github/workflows/docker-test.yml`](diffhunk://#diff-baa08c0b7cb0ae6357ecf7ecaa2d7aff0bfa417e10e1ec6fbc29232cb0166da7L10-R10): Updated the workflow to use `actions/setup-java@v2` with `temurin` distribution and fixed the path for the Prometheus exporter. [[1]](diffhunk://#diff-baa08c0b7cb0ae6357ecf7ecaa2d7aff0bfa417e10e1ec6fbc29232cb0166da7L10-R10) [[2]](diffhunk://#diff-baa08c0b7cb0ae6357ecf7ecaa2d7aff0bfa417e10e1ec6fbc29232cb0166da7L28-R32) [[3]](diffhunk://#diff-baa08c0b7cb0ae6357ecf7ecaa2d7aff0bfa417e10e1ec6fbc29232cb0166da7L43-L44) * [`.github/workflows/gradle-precommit.yml`](diffhunk://#diff-682f117c9ca6b36bb3e3ca8bbfc2c70a1d6a64e4949ed8f0e22b4c94a9a23cb2L6-R6): Updated the workflow to use `actions/setup-java@v2` with `temurin` distribution and adjusted branch patterns. [[1]](diffhunk://#diff-682f117c9ca6b36bb3e3ca8bbfc2c70a1d6a64e4949ed8f0e22b4c94a9a23cb2L6-R6) [[2]](diffhunk://#diff-682f117c9ca6b36bb3e3ca8bbfc2c70a1d6a64e4949ed8f0e22b4c94a9a23cb2L19-R23) [[3]](diffhunk://#diff-682f117c9ca6b36bb3e3ca8bbfc2c70a1d6a64e4949ed8f0e22b4c94a9a23cb2L35-L37) ### Documentation and License Updates: * [`CONTRIBUTING.md`](diffhunk://#diff-eca12c0a30e25b4b46522ebf89465a03ba72a03f540796c979137931d8f92055R1-R65): Added detailed instructions and guidelines for contributing to the Apache Solr project. * [`NOTICE.txt`](diffhunk://#diff-b810ebe4ffda4293c79d292059b45b59de7e649e86314ffd5a1ee4eb4b1e08c8L3-R3): Updated the copyright year and added new notices for included software and libraries. [[1]](diffhunk://#diff-b810ebe4ffda4293c79d292059b45b59de7e649e86314ffd5a1ee4eb4b1e08c8L3-R3) [[2]](diffhunk://#diff-b810ebe4ffda4293c79d292059b45b59de7e649e86314ffd5a1ee4eb4b1e08c8R58-R61) [[3]](diffhunk://#diff-b810ebe4ffda4293c79d292059b45b59de7e649e86314ffd5a1ee4eb4b1e08c8L192-R196) [[4]](diffhunk://#diff-b810ebe4ffda4293c79d292059b45b59de7e649e86314ffd5a1ee4eb4b1e08c8R351-R353) [[5]](diffhunk://#diff-b810ebe4ffda4293c79d292059b45b59de7e649e86314ffd5a1ee4eb4b1e08c8L402-R409) [[6]](diffhunk://#diff-b810ebe4ffda4293c79d292059b45b59de7e649e86314ffd5a1ee4eb4b1e08c8L437-R444) [[7]](diffhunk://#diff-b810ebe4ffda4293c79d292059b45b59de7e649e86314ffd5a1ee4eb4b1e08c8R621-R627) These changes enhance the project's configuration management, streamline the CI/CD process, and provide clearer guidelines for contributors. -- 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-17690 Make solr zk tools read ZK_HOST environment [solr]
epugh commented on code in PR #3240: URL: https://github.com/apache/solr/pull/3240#discussion_r1981755145 ## solr/core/src/java/org/apache/solr/cli/CLIUtils.java: ## @@ -216,12 +216,29 @@ public static String normalizeSolrUrl(CommandLine cli) throws Exception { } /** - * Get the ZooKeeper connection string from either the zk-host command-line option or by looking - * it up from a running Solr instance based on the solr-url option. + * Get the ZooKeeper connection string from either the zk-host command-line option or if not + * configured, from the 'zkHost' system property aka the 'ZK_HOST' environment variable. + * + * @param cli the command line + * @return the ZooKeeper connection string or null if not configured + */ + public static String getZkHostFromCliOrEnv(CommandLine cli) { Review Comment: Could we look more closely at how CLIUtils. getDefaultSolrUrl works? It feels like we need to make the looking up of ZK and Solr URL more aligned. Use similiar names for the methods and similar patterns for the look ups. -- 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-17690 Make solr zk tools read ZK_HOST environment [solr]
epugh commented on code in PR #3240: URL: https://github.com/apache/solr/pull/3240#discussion_r1981763825 ## solr/core/src/java/org/apache/solr/cli/RunExampleTool.java: ## @@ -305,7 +305,7 @@ protected void runExample(CommandLine cli, String exampleName) throws Exception "techproducts".equals(exampleName) ? "sample_techproducts_configs" : "_default"; boolean isCloudMode = !cli.hasOption(USER_MANAGED_OPTION); -String zkHost = cli.getOptionValue(CommonCLIOptions.ZK_HOST_OPTION); +String zkHost = CLIUtils.getZkHostFromCliOrEnv(cli); Review Comment: I can't reallyu imagine anyone running the example who wants it from ZK_HOST. That would be a surprise. You know, I think there is a legit argument that the RunExampleTool should NOT take in a ZKHost parameter. We don't need the RunExampleTool to do everything that `bin/solr start` does.. We need it to be really good at running an example solr. So, if we wanted to eliminate standalone support from the RunExampleTool, I would get that. If we wanted it to only do certain other things, I would get that too. One big annonycance in the `bin/solr` script is all the special handling we have for just running an example, and maybe if we narrow down what an example does, we would have less special case code? There are 14 options we pass to the run example!! And honestly, the only option I ever pass is "techproducts" and "films" and "schemaless", every other option could potentially be hard coded.Which would simpilify that tool a LOT. It's an example tool, not a "let me debug solr in thwierd configuration" tool. -- 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-17690 Make solr zk tools read ZK_HOST environment [solr]
epugh commented on code in PR #3240: URL: https://github.com/apache/solr/pull/3240#discussion_r1981764531 ## solr/core/src/test/org/apache/solr/cli/ZkSubcommandsTest.java: ## @@ -507,10 +507,8 @@ public void testGetFile() throws Exception { Path file = tmpDir.resolve("solrtest-getfile-" + this.getClass().getName() + "-" + System.nanoTime()); -String[] args = -new String[] { - "cp", "-z", zkServer.getZkAddress(), "zk:" + getNode, file.toAbsolutePath().toString() -}; +// Not setting --zk-host, will fall back to sysProp 'zkHost' +String[] args = new String[] {"cp", "zk:" + getNode, file.toAbsolutePath().toString()}; Review Comment: :-(. See me rant email ;-) -- 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] Make ingress hostname configurable [solr-operator]
mikeywuu commented on PR #763: URL: https://github.com/apache/solr-operator/pull/763#issuecomment-2701590303 Sorry, had to re-create the PR. #764 -- 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
[PR] Make ingress hostname configurable [solr-operator]
mikeywuu opened a new pull request, #764: URL: https://github.com/apache/solr-operator/pull/764 This PR addresses the possibility to add an `hostName` to the ingress configuration of a given solrcloud instance as described in #667. The change is quit simple. Unfortunately, the `make prepare` step failed for me at the `fetch-licenses-list` step, so the `dependency_licenses.csv` stays empty in my case. ``` make: *** [fetch-licenses-list] Error 1 ``` To make the e2e-tests start / work, I also needed to change the `RAW_GINKGO` command. Not sure if that is an issue only on my machine. Happy for any feedback! -- 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-17685: Remove script creation of solr url based on SOLR_TOOL_HOST in favour of java code in CLI tools [solr]
malliaridis commented on code in PR #3223: URL: https://github.com/apache/solr/pull/3223#discussion_r1981863388 ## solr/bin/solr: ## @@ -612,22 +612,11 @@ if [[ "$SCRIPT_CMD" == "auth" ]]; then fi fi - if [ -z "${AUTH_PORT:-}" ]; then -for ID in $(ps auxww | grep java | grep start\.jar | awk '{print $2}' | sort -r) - do -port=$(jetty_port "$ID") -if [ "$port" != "" ]; then - AUTH_PORT=$port - break -fi - done - fi - - run_tool auth $@ --solr-url "$SOLR_URL_SCHEME://$SOLR_TOOL_HOST:${AUTH_PORT:-8983}" --auth-conf-dir "$SOLR_HOME" "--solr-include-file" "$SOLR_INCLUDE" Review Comment: This eliminates the `AUTH_PORT` and `SOLR_TOOL_HOST` environment variables, right? I believe it is fine, as the behavior is checking the jetty port and falls back to the default Solr port, so no need to distinguish it in the scripts. Still, this is a breaking change. EDIT: I've noticed this eliminates the use of these only in the scripts. -- 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-17635: unmarshalling Map to SimpleOrderedMap if key i of type St… [solr]
dsmiley commented on PR #3163: URL: https://github.com/apache/solr/pull/3163#issuecomment-2701775120 > So what is the point of changing from a LinkedHashMap to LinkedHashMap in a SMO? A LinkedHashMap doesn't implement NamedList. An SMO does. Thus a Solr 10 server could start passing maps and MapWriter things (e.g. SolrParams) to the response in situations that a SolrJ 9 is still anticipating reading a NamedList. With SOM; it's both things in one. The choice of LinkedHashMap vs ArrayList would be a regression for cases where the client code already expected a Map; was assuming O(1) performance of the map if lots of stuff is in it. Because NamedList/Map is nestable, at the time of unmarshalling we have no idea what the actual code looking at the object is casting it as. In a fantasy world if there was just exactly one thing to unmarshall, then there would be maybe readMap vs readNamedList and we wouldn't have concerns that this PR tries to solve. That said, let's not actually change SMO to be based on a LinkedHashMap; at least not soon. I proposed only reading maps as SMO for a more narrow circumstance (9.x talking to 10.x) that would be somewhat temporary / limited, thus ameliorating a performance risk. -- 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-17685: Remove script creation of solr url based on SOLR_TOOL_HOST in favour of java code in CLI tools [solr]
malliaridis commented on code in PR #3223: URL: https://github.com/apache/solr/pull/3223#discussion_r1981863388 ## solr/bin/solr: ## @@ -612,22 +612,11 @@ if [[ "$SCRIPT_CMD" == "auth" ]]; then fi fi - if [ -z "${AUTH_PORT:-}" ]; then -for ID in $(ps auxww | grep java | grep start\.jar | awk '{print $2}' | sort -r) - do -port=$(jetty_port "$ID") -if [ "$port" != "" ]; then - AUTH_PORT=$port - break -fi - done - fi - - run_tool auth $@ --solr-url "$SOLR_URL_SCHEME://$SOLR_TOOL_HOST:${AUTH_PORT:-8983}" --auth-conf-dir "$SOLR_HOME" "--solr-include-file" "$SOLR_INCLUDE" Review Comment: This eliminates the `AUTH_PORT` and `SOLR_TOOL_HOST` environment variables, right? I believe it is fine, as the behavior is checking the jetty port and falls back to the default Solr port, so no need to distinguish it in the scripts. Still, this is a breaking change. EDIT: I've noticed this eliminates the use of these only in the scripts, and `SOLR_TOOL_HOST` is still used in java classes via `solr.tool.host` (but not `auth.port`?). -- 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-16305) MODIFYCOLLECTION with 'property.*' changes can't change values used in config file variables (even though they can be set during collection CREATE)
[ https://issues.apache.org/jira/browse/SOLR-16305?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17932623#comment-17932623 ] Jason Gerlowski commented on SOLR-16305: +1 to what others have already mentioned - that these "property" APIs are difficult to keep straight and reason about. I don't have strong opinions on the underlying implementation, but I'd love to help drive us towards a "single read/write API", as David suggested. Quick summary of the current state-of-play: || ||Write API||ReadAPI|| Visible for Plugins || Visible-for-Config || |Coll Creation Props | /admin/collections?action=CREATE...&property.foo=bar | /admin/collections?action=COLSTATUS | DocCollection | Yep | | Modify Collection Props | /admin/collections?action=MODIFYCOLLECTION&property.foo=bar | /admin/collections?action=COLSTATUS | DocCollection | Only to replicas created after MODIFY | | "COLLECTIONPROP" | /admin/collections?action=COLLECTIONPROP&propertyName=foo&propertyValue=bar | N/A | ZkStateReader.getCollectionProperties | Nope | (The table above might've had a column to represent core.properties persistence, but I omitted that as it seems like an implementation detail that users are unlikely to care about. Am I missing something there?) If the goal is for the most up-to-date property values to be visible to all cores, then I think we've got two main approaches: # Keep "core.properties" as the local "source of truth" for these properties, but periodically update the file (say, at core-load time) to keep it up-to-date with property changes in ZK. # Add the ZK properties to the 'substitutableProperties' map used during config parsing, irrespective of (or in addition to) the contents of core.properties. Personally I lean towards the latter approach. Fetching properties out of ZK right before a core-load feels less creaky and requires less machinery than trying to keep 'N' core.properties files up to date. I'm ambivalent on where specifically in ZK the properties live. State.json is probably the most convenient place as there's already plumbing to populate `DocCollection`, etc. from it. But maybe there's a "scaling" argument to keep props separate from state.json (i.e. in collectionprops.json), so the two can be modified independently of one another. What does everyone else think? If folks are OK with the second approach in general, I can come up with a more detailed writeup plan that covers APIs, etc.? > MODIFYCOLLECTION with 'property.*' changes can't change values used in config > file variables (even though they can be set during collection CREATE) > --- > > Key: SOLR-16305 > URL: https://issues.apache.org/jira/browse/SOLR-16305 > Project: Solr > Issue Type: Bug >Reporter: Chris M. Hostetter >Priority: Major > Attachments: SOLR-16305_test.patch > > > Consider a configset with a {{solrconfig.xml}} that includes a snippet like > this... > {code:java} > ${custom.prop:customDefVal} > {code} > ...this {{custom.prop}} can be set when doing a {{CREATE}} command for a > collection that uses this configset, using the {{property.*}} prefix as noted > in the reg-guide... > {quote}{{property.{_}name{_}={_}value{_}}} > |Optional|Default: none| > Set core property _name_ to {_}value{_}. See the section [Core > Discovery|https://solr.apache.org/guide/solr/latest/configuration-guide/core-discovery.html] > for details on supported properties and values. > {quote} > ...BUT > These values can *not* be changed by using the {{MODIFYCOLLECTION}} command, > in spite of the ref-guide indicating that it can be used to modify custom > {{property.*}} attributes... > {quote}The attributes that can be modified are: > * {{replicationFactor}} > * {{collection.configName}} > * {{readOnly}} > * other custom properties that use a {{property.}} prefix > See the [CREATE > action|https://solr.apache.org/guide/solr/latest/deployment-guide/collection-management.html#create] > section above for details on these attributes. > {quote} -- 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-17690 Make solr zk tools read ZK_HOST environment [solr]
HoustonPutman commented on PR #3240: URL: https://github.com/apache/solr/pull/3240#issuecomment-2701378283 > Hmm, there's no `9.8.1` section in CHANGES.txt on `main`, and not on `branch_9x`, only on `branch_9_8`. Is the correct way for this bugfix to skip CHANGES here and on 9x, and then add one when cherry-picking to the release branch? Then the 9.8.1 RM will reconsile changes to other branches during release? > > Here is suggested changes text: > > ``` > * SOLR-17690: Make solr CLI tools read ZK_HOST environment as documented. There was a regression > in 9.8.0 where this did not work, causing issues also for solr-operator v0.9 (janhoy) > ``` So add it to the changeling for 9.9, and then in branch_9_8, add it to 9.8.1. As a part of the release, I remove the duplicates from 9.9 on branch_9x and main. -- 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] fix the solr zk invocation [solr-operator]
samuelverstraete commented on code in PR #756: URL: https://github.com/apache/solr-operator/pull/756#discussion_r1981717896 ## controllers/solrcloud_controller_basic_auth_test.go: ## @@ -351,12 +351,12 @@ func expectBasicAuthConfigOnPodTemplateWithGomega(g Gomega, solrCloud *solrv1bet func expectPutSecurityJsonInZkCmd(g Gomega, expInitContainer *corev1.Container) { g.Expect(expInitContainer).To(Not(BeNil()), "Didn't find the setup-zk InitContainer in the sts!") - expCmd := "solr zk cp zk:/security.json /tmp/current_security.json >/dev/null 2>&1; " + + expCmd := "solr zk cp zk:/security.json /tmp/current_security.json --zk-host $ZK_HOST >/dev/null 2>&1; " + Review Comment: i changed the invocation to use -z instead. This worked both on solr 9.6.1 and solr 9.8.0 -- 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: [I] Security.json is never copied to Zookeeper while creating Solr 9.8.0 Clusters [solr-operator]
janhoy commented on issue #762: URL: https://github.com/apache/solr-operator/issues/762#issuecomment-2701079910 > There's a PR [here](https://github.com/apache/solr-operator/pull/756) to update the `bin/solr zk` invocation that's seemingly already most of the way there... Fixing it in Solr for 9.8.1 would hopefully make it work for all 9.x versions except 9.8.0, but I have not double checked that "solr zk" in 9.0->9.7 actually pick up ZK_HOST correctly. So a operator fix may not be necessary but would probably not hurt 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
[jira] [Updated] (SOLR-17679) Request for Documentation/Feature Improvement on Hybrid Lexical and Vector Search with Score Breakdown and Cutoff Logic
[ https://issues.apache.org/jira/browse/SOLR-17679?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Khaled Alkhouli updated SOLR-17679: --- Status: Open (was: Patch Available) > Request for Documentation/Feature Improvement on Hybrid Lexical and Vector > Search with Score Breakdown and Cutoff Logic > --- > > Key: SOLR-17679 > URL: https://issues.apache.org/jira/browse/SOLR-17679 > Project: Solr > Issue Type: Improvement > Components: search >Affects Versions: 9.6.1 >Reporter: Khaled Alkhouli >Priority: Minor > Labels: hybrid-search, search, solr, vector-based-search > Attachments: Screenshot from 2025-02-20 16-31-48.png > > > Hello Apache Solr team, > I was able to implement a hybrid search engine that combines *lexical search > (edismax)* and *vector search (KNN-based embeddings)* within a single > request. The idea is simple: > * *Lexical Search* retrieves results based on text relevance. > * *Vector Search* retrieves results based on semantic similarity. > * *Hybrid Scoring* sums both scores, where a missing score (if a document > appears in only one search) should be treated as zero. > This approach is working, but *there is a critical lack of documentation* on > how to properly return individual score components of lexical search (score1) > vs. vector search (score2 from cosine similarity). Right now, Solr only > returns the final combined score, but there is no clear way to see {*}how > much of that score comes from lexical search vs. vector search{*}. This is > essential for debugging and for fine-tuning ranking strategies. > > I have implemented the following logic using Python: > {code:java} > def hybrid_search(query, top_k=10): > embedding = np.array(embed([query]), dtype=np.float32 > embedding = list(embedding[0]) > lxq= rf"""{{!type=edismax > qf='text' > q.op=OR > tie=0.1 > bq='' > bf='' > boost='' > }}({query})""" > solr_query = {"params": { > "q": "{!bool filter=$retrievalStage must=$rankingStage}", > "rankingStage": > "{!func}sum(query($normalisedLexicalQuery),query($vectorQuery))", > "retrievalStage":"{!bool should=$lexicalQuery should=$vectorQuery}", > # Union > "normalisedLexicalQuery": "{!func}scale(query($lexicalQuery),0,1)", > "lexicalQuery": lxq, > "vectorQuery": f"{{!knn f=all_v512 topK={top_k}}}{embedding}", > "fl": "text", > "rows": top_k, > "fq": [""], > "rq": "{!rerank reRankQuery=$rqq reRankDocs=100 reRankWeight=3}", > "rqq": "{!frange l=$cutoff}query($rankingStage)", > "sort": "score desc", > }} > response = requests.post(SOLR_URL, headers=HEADERS, json=solr_query) > response = response.json() > return response {code} > h3. *Issues & Missing Documentation* > # *No Way to Retrieve Individual Scores in a Hybrid Search* > There is no clear documentation on how to return: > * > ** The *lexical search score* separately. > ** The *vector search score* separately. > ** The *final combined score* (which Solr already provides). > Right now, we’re left guessing whether the sum of these scores works as > expected, making debugging and tuning unnecessarily difficult. > # *No Clear Way to Implement Cutoff Logic in Solr* > In a hybrid search, I need to filter out results that don’t meet a {*}minimum > score threshold{*}. Right now, I have to implement this in Python, {*}which > defeats the purpose of using Solr for ranking in the first place{*}. > * > ** How can we enforce a {*}score-based cutoff directly in Solr{*}, without > external filtering? > ** The \{!frange} function is mentioned in the documentation but lacks > {*}clear examples on how to apply it to hybrid search{*}. > h3. *Feature Request / Documentation Improvement* > * *Provide a way to return individual scores for lexical and vector search > in the response.* This should be as simple as adding fields like > {{{}fl=score,lexical_score,vector_score{}}}. > * *Clarify how to apply cutoff logic in a hybrid search.* This is an > essential ranking mechanism, and yet, there’s little guidance on how to do > this efficiently within Solr itself. > Looking forward to a response. -- 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-17635: unmarshalling Map to SimpleOrderedMap if key i of type St… [solr]
renatoh commented on PR #3163: URL: https://github.com/apache/solr/pull/3163#issuecomment-2700819405 > Only an SMO is also a NamedList. A LinkedHashMap itself is not. Just for my understand: What exactly would improve by unmarshalling to a SMO backed by LinkedHashMap compared to just a LinkedHashMap? -- 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-17635: unmarshalling Map to SimpleOrderedMap if key i of type St… [solr]
renatoh commented on PR #3163: URL: https://github.com/apache/solr/pull/3163#issuecomment-2701327325 > Map.get, Map.remove, Map.put -- basic operations are O(1) in a hash based Map, but not in a dense array List that SOM is today. I do understand that part, but today, we are umarshalling to a LinkedHashMap, with the current changes we would unmarshall to a SMO (backed by an ArrayList), I do understand the potential performance issues of that change. But if we now changed the SMO to a LinkedHashMap, we would then unmarshall to an LinkedHashMap in a SMO, compared to unmarshall to a LinkedHashmap, as we do today. So what is the point of changing from a LinkedHashMap to LinkedHashMap in a SMO? -- 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] Make ingress hostname configurable [solr-operator]
ps-muspelkat closed pull request #763: Make ingress hostname configurable URL: https://github.com/apache/solr-operator/pull/763 -- 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] Ref-guide: upgrade notes: fix wrong file reference. [solr]
dsmiley merged PR #3239: URL: https://github.com/apache/solr/pull/3239 -- 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] Ref-guide: upgrade notes: fix wrong file reference. [solr]
HoustonPutman commented on PR #3239: URL: https://github.com/apache/solr/pull/3239#issuecomment-2701967655 Good change, 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] SOLR-17684: SolrJ Reference Guide (Ping) - Incorrect Status Retrieval [solr]
epugh merged PR #3237: URL: https://github.com/apache/solr/pull/3237 -- 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-17684) Issue in SolrJ Reference Guide (Ping) - Incorrect Status Retrieval
[ https://issues.apache.org/jira/browse/SOLR-17684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17932760#comment-17932760 ] ASF subversion and git services commented on SOLR-17684: Commit dc240253180676e9fe301c5f7673c1e68297a51a in solr's branch refs/heads/branch_9x from Umut Saribiyik [ https://gitbox.apache.org/repos/asf?p=solr.git;h=dc240253180 ] SOLR-17684: SolrJ Reference Guide (Ping) - Incorrect Status Retrieval (#3237) - Update ping.adoc with references from unit test. - Add unit test to illustrate the usage of the ping response. > Issue in SolrJ Reference Guide (Ping) - Incorrect Status Retrieval > -- > > Key: SOLR-17684 > URL: https://issues.apache.org/jira/browse/SOLR-17684 > Project: Solr > Issue Type: Bug > Security Level: Public(Default Security Level. Issues are Public) > Components: SolrJ >Affects Versions: 9.8 >Reporter: Umut Saribiyik >Priority: Minor > Labels: guide, monitor, pingrequest, pull-request-available, > reference, solrj > Attachments: image-2025-02-26-13-29-31-289.png > > Time Spent: 2h 10m > Remaining Estimate: 0h > > The SolrJ Reference Guide provides incorrect examples for retrieving the ping > status of a collection. > [https://solr.apache.org/guide/solr/9_8/deployment-guide/ping.html] > *Issues:* > # pingResponse.getStatus() returns the status of the ping request itself > (status=0), not the collection's ping status ("OK"). > # Since getResponse() returns a NamedList, the value must be > explicitly cast to a String or converted using .toString(). > > > *Correction:* > The correct way to retrieve the collection's ping status: > {code:java} > String status = (String) process.getResponse().get("status"); > String status = process.getResponse().get("status").toString(); > {code} > > > -- 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-17684: SolrJ Reference Guide (Ping) - Incorrect Status Retrieval [solr]
epugh commented on PR #3237: URL: https://github.com/apache/solr/pull/3237#issuecomment-2702160424 Thank you for your contribution @umut-sar... I appreciate you open a PR, and then took the feedback and came back with an even better solution! -- 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-17690 Make solr zk tools read ZK_HOST environment [solr]
janhoy commented on PR #3240: URL: https://github.com/apache/solr/pull/3240#issuecomment-2702257270 > I feel that the users may be overwhelmed with the amount of options we give them for defining a property (CLI option, env vars and file), which makes our code more complex as well Adding `ZK_HOST=foo` to solr.in.sh has the same effect as adding `export ZK_HOST=foo` to your shell. It comes very handy when e.g. running Solr as a container and you can configure a lot with just env instead of having to patch the `CMD` part. The CLI tools like `zk cp` behaves the same as when starting solr itself. So you can set `SOLR_URL` or `ZK_HOST` variable and then run much shorter commands to work with your cluster. Perhaps we could do a better job in SolrCLI of wrapping such common behavior in more generic utils like ```java ``` -- 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
[PR] Simplify BinaryResponseWriter.getParsedResponse [solr]
dsmiley opened a new pull request, #3243: URL: https://github.com/apache/solr/pull/3243 This method should lean on two other classes to handle the details. Tests pass, and there are a couple testing this method including the particulars of "omit". -- 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-17690 Make solr zk tools read ZK_HOST environment [solr]
janhoy commented on code in PR #3240: URL: https://github.com/apache/solr/pull/3240#discussion_r1982302027 ## solr/core/src/test/org/apache/solr/cli/ZkSubcommandsTest.java: ## @@ -507,10 +507,8 @@ public void testGetFile() throws Exception { Path file = tmpDir.resolve("solrtest-getfile-" + this.getClass().getName() + "-" + System.nanoTime()); -String[] args = -new String[] { - "cp", "-z", zkServer.getZkAddress(), "zk:" + getNode, file.toAbsolutePath().toString() -}; +// Not setting --zk-host, will fall back to sysProp 'zkHost' +String[] args = new String[] {"cp", "zk:" + getNode, file.toAbsolutePath().toString()}; Review Comment: Did not find your mail. I like env vars also in cli tools :) -- 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-17690 Make solr zk tools read ZK_HOST environment [solr]
janhoy commented on code in PR #3240: URL: https://github.com/apache/solr/pull/3240#discussion_r1982317593 ## solr/core/src/java/org/apache/solr/cli/RunExampleTool.java: ## @@ -305,7 +305,7 @@ protected void runExample(CommandLine cli, String exampleName) throws Exception "techproducts".equals(exampleName) ? "sample_techproducts_configs" : "_default"; boolean isCloudMode = !cli.hasOption(USER_MANAGED_OPTION); -String zkHost = cli.getOptionValue(CommonCLIOptions.ZK_HOST_OPTION); +String zkHost = CLIUtils.getZkHostFromCliOrEnv(cli); Review Comment: Let's improve the examples in a followup issue. Perhaps we should not take away capabilities in 9.x, bu make this a 10.0 improvement. For now, this change here brings parity with how `--zk-host` option is documented and supported in the rest of the CLI. -- 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-17685: Remove script creation of solr url based on SOLR_TOOL_HOST in favour of java code in CLI tools [solr]
HoustonPutman commented on PR #3223: URL: https://github.com/apache/solr/pull/3223#issuecomment-2702105858 So instead of using SOLR_TOOL_HOST, and instead using SOLR_HOST in the java code, I think we can remove the SOLR_TOOL_HOST all together, since we set it in `bin/solr`: ```bash SOLR_TOOL_HOST="${SOLR_HOST:-localhost}" export SOLR_TOOL_HOST ``` So since we are already defaulting to `localhost` in the code, I think we can remove `SOLR_TOOL_HOST` altogether and change `String host = EnvUtils.getProperty("solr.tool.host", "localhost");` to `String host = EnvUtils.getProperty("solr.host", "localhost");` in `CLIUtils` But I agree that unfortunately the AUTH_PORT is a backwards incompatible change. Maybe instead we still check for the `auth` command in the 9.x code and do `export SOLR_PORT="${AUTH_PORT}"`. And we can keep the code to get that port the same. But in 10, we can remove it and the additional export. -- 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] fix the solr zk invocation [solr-operator]
HoustonPutman commented on PR #756: URL: https://github.com/apache/solr-operator/pull/756#issuecomment-2702120082 Has anyone tested this with Solr 8.11 (This is still supported for the 0.9 line) -- 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] fix the solr zk invocation [solr-operator]
epugh commented on code in PR #756: URL: https://github.com/apache/solr-operator/pull/756#discussion_r1982211735 ## controllers/solrcloud_controller_basic_auth_test.go: ## @@ -351,12 +351,12 @@ func expectBasicAuthConfigOnPodTemplateWithGomega(g Gomega, solrCloud *solrv1bet func expectPutSecurityJsonInZkCmd(g Gomega, expInitContainer *corev1.Container) { g.Expect(expInitContainer).To(Not(BeNil()), "Didn't find the setup-zk InitContainer in the sts!") - expCmd := "solr zk cp zk:/security.json /tmp/current_security.json >/dev/null 2>&1; " + + expCmd := "solr zk cp zk:/security.json /tmp/current_security.json --zk-host $ZK_HOST >/dev/null 2>&1; " + Review Comment: Yeah -z has been the short version for forever! -- 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-17690 Make solr zk tools read ZK_HOST environment [solr]
HoustonPutman commented on PR #3240: URL: https://github.com/apache/solr/pull/3240#issuecomment-2702130634 So I think there is good discussion to be had here about the use of EnvVars in the SolrCLI logic (which currently we do use for default Solr URLs). But in the meantime, I do believe that there is a fix for https://github.com/apache/solr-operator/issues/762 that will support 9.8.0 and 9.8.1 even if this PR isn't included. I think that's probably the way to go, so we can spend the time and come up with the best strategy for CLI EnvVars. -- 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-16305) MODIFYCOLLECTION with 'property.*' changes can't change values used in config file variables (even though they can be set during collection CREATE)
[ https://issues.apache.org/jira/browse/SOLR-16305?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17932750#comment-17932750 ] David Smiley commented on SOLR-16305: - I definitely also strongly prefer the 2nd approach, thus don't touch core.properties. I'm glad you proposed this! To be clear though, I don't think this involves any new files in ZK. It's some combination of state.json and collection properties. Perhaps the internal/built-in ones go to state.json and everything else to collection properties, so it's both. I'd ultimately like to see core.properties maybe go away in SolrCloud. Just a straw-man bold proposal; may not be realistic. > MODIFYCOLLECTION with 'property.*' changes can't change values used in config > file variables (even though they can be set during collection CREATE) > --- > > Key: SOLR-16305 > URL: https://issues.apache.org/jira/browse/SOLR-16305 > Project: Solr > Issue Type: Bug >Reporter: Chris M. Hostetter >Priority: Major > Attachments: SOLR-16305_test.patch > > > Consider a configset with a {{solrconfig.xml}} that includes a snippet like > this... > {code:java} > ${custom.prop:customDefVal} > {code} > ...this {{custom.prop}} can be set when doing a {{CREATE}} command for a > collection that uses this configset, using the {{property.*}} prefix as noted > in the reg-guide... > {quote}{{property.{_}name{_}={_}value{_}}} > |Optional|Default: none| > Set core property _name_ to {_}value{_}. See the section [Core > Discovery|https://solr.apache.org/guide/solr/latest/configuration-guide/core-discovery.html] > for details on supported properties and values. > {quote} > ...BUT > These values can *not* be changed by using the {{MODIFYCOLLECTION}} command, > in spite of the ref-guide indicating that it can be used to modify custom > {{property.*}} attributes... > {quote}The attributes that can be modified are: > * {{replicationFactor}} > * {{collection.configName}} > * {{readOnly}} > * other custom properties that use a {{property.}} prefix > See the [CREATE > action|https://solr.apache.org/guide/solr/latest/deployment-guide/collection-management.html#create] > section above for details on these attributes. > {quote} -- 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-17684) Issue in SolrJ Reference Guide (Ping) - Incorrect Status Retrieval
[ https://issues.apache.org/jira/browse/SOLR-17684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17932759#comment-17932759 ] ASF subversion and git services commented on SOLR-17684: Commit ac3d349dac530cf1001d5113fc21b0fd641cc9d5 in solr's branch refs/heads/main from Umut Saribiyik [ https://gitbox.apache.org/repos/asf?p=solr.git;h=ac3d349dac5 ] SOLR-17684: SolrJ Reference Guide (Ping) - Incorrect Status Retrieval (#3237) - Update ping.adoc with references from unit test. - Add unit test to illustrate the usage of the ping response. > Issue in SolrJ Reference Guide (Ping) - Incorrect Status Retrieval > -- > > Key: SOLR-17684 > URL: https://issues.apache.org/jira/browse/SOLR-17684 > Project: Solr > Issue Type: Bug > Security Level: Public(Default Security Level. Issues are Public) > Components: SolrJ >Affects Versions: 9.8 >Reporter: Umut Saribiyik >Priority: Minor > Labels: guide, monitor, pingrequest, pull-request-available, > reference, solrj > Attachments: image-2025-02-26-13-29-31-289.png > > Time Spent: 2h 10m > Remaining Estimate: 0h > > The SolrJ Reference Guide provides incorrect examples for retrieving the ping > status of a collection. > [https://solr.apache.org/guide/solr/9_8/deployment-guide/ping.html] > *Issues:* > # pingResponse.getStatus() returns the status of the ping request itself > (status=0), not the collection's ping status ("OK"). > # Since getResponse() returns a NamedList, the value must be > explicitly cast to a String or converted using .toString(). > > > *Correction:* > The correct way to retrieve the collection's ping status: > {code:java} > String status = (String) process.getResponse().get("status"); > String status = process.getResponse().get("status").toString(); > {code} > > > -- 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