You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/02/22 20:25:09 UTC
[GitHub] [lucene-solr] andywebb1975 opened a new pull request #2643: Make Config API work for warming queries
andywebb1975 opened a new pull request #2643:
URL: https://github.com/apache/lucene-solr/pull/2643
This is my attempt at resolving https://issues.apache.org/jira/browse/SOLR-9359 - it's still very work-in-progress, hence all the debug output etc, but if anyone has thoughts on it please let me know.
I don't know if there's a better way to do this without all the `getClass()`/`instanceof` checking?
With this patch in place it becomes possible to send `add/update-listener` commands to the Config API like this, and they take effect as expected rather than throwing a `ClassCastException`:
```
{
"update-listener": {
"name": "warming-queries",
"event": "newSearcher",
"class": "solr.QuerySenderListener",
"queries": [
[
{
"q": "foo"
},
{
"q": "bar"
}
]
]
}
}
```
Note the nested array: without that, only the first query in the list is picked up - the rest don't appear in the `getArgs().get("queries")` response at all. I don't know if that's fixable but I suspect it'd require more widespread changes so I've steered clear of that thus far.
(Also, this class is virtually the same in the new Solr repo - I'd raise a PR for that too.)
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] andywebb1975 commented on pull request #2643: SOLR-9359 Make Config API work for warming queries
Posted by GitBox <gi...@apache.org>.
andywebb1975 commented on pull request #2643:
URL: https://github.com/apache/lucene-solr/pull/2643#issuecomment-1080930412
I've given this some more attention:
* found the `getArgs().getAll()` method which means it now works with a flat list like in my last comment above
* moved most of the change into a `queries2list` method
* various updates inside that method
I'm running this on our prototype stack to battle-test it with our content/configs. Obviously there'd be more to do before this could be merged, like tidying up the code more and adding tests - and also I'm aware 8.11 is end-of-line so this could become a 9.x thing instead. (We're running a custom build so wouldn't need it to be merged into 8.11.)
I'm honestly not sure where to begin with creating tests for this using config provided via the Config API! Is there anything similar I could take a lead from?
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] cpoerschke commented on pull request #2643: SOLR-9359 Make Config API work for warming queries
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on pull request #2643:
URL: https://github.com/apache/lucene-solr/pull/2643#issuecomment-1080947093
> ... Is there anything similar I could take a lead from?
Perhaps these two:
* https://github.com/apache/lucene-solr/blob/releases/lucene-solr/8.11.1/solr/core/src/test/org/apache/solr/HelloWorldSolrCloudTestCase.java
* https://github.com/apache/lucene-solr/blob/releases/lucene-solr/8.11.1/solr/core/src/test/org/apache/solr/cloud/TestCloudSearcherWarming.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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] epugh commented on pull request #2643: SOLR-9359 Make Config API work for warming queries
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #2643:
URL: https://github.com/apache/lucene-solr/pull/2643#issuecomment-1048668234
I like what this does, but I totally agree about all the `getClass()`/`instanceof` stuff! I wonder if burying all that logic into a method with a name like `normalizeArgsToQueries()` or maybe just `bewareHereBeDragons()` ;-) might help someone understand that this logic is odd....
Is there a way to reduce the complexity of `NamedLists` and `ArrayLists` and the variation in the JSON structure????
Do we have a test that at least demonstrates the fix?
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] andywebb1975 commented on pull request #2643: SOLR-9359 Make Config API work for warming queries
Posted by GitBox <gi...@apache.org>.
andywebb1975 commented on pull request #2643:
URL: https://github.com/apache/lucene-solr/pull/2643#issuecomment-1051012958
Thanks Eric! Yes, I'll do some refactoring and try making a test. I don't think we get much control of the shape of the object - I'd like to be able to drop the nested array and use a nice clean list like the below, but for some reason only the first entry makes it into the output of `.get("queries")` :-(
```
{
"update-listener": {
"name": "warming-queries",
"event": "newSearcher",
"class": "solr.QuerySenderListener",
"queries": [
{
"q": "foo"
},
{
"q": "bar"
}
]
}
}
```
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org