[ 
https://issues.apache.org/jira/browse/SOLR-11921?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Chris M. Hostetter updated SOLR-11921:
--------------------------------------
    Attachment: SOLR-11921.patch
      Assignee: Chris M. Hostetter
        Status: Open  (was: Open)

I've been looking into this a bit...

first off: the problem is not specific to elevateIds, it is a general 
incompatibility with how the QueryElevationComponent is designed, and how the 
CursorMark functionality is designed.
 * QueryElevationComponent is intended to be used as a {{last-component}} so 
that it's {{prepare()}} method runs after the {{prepare()}} method of all other 
components, and it can modify the {{SortSpec}} produced in the {{preapre()}} 
method of any other component (typically QueryComponent)
 * CurorMark logic is initialized in {{QueryComponent.prepare()}} – just after 
the {{sort}} param is parsed into a {{SortSpec}} – so that it can validate & 
parse the {{cursorMark}} param against the {{SortSpec}} to produce the 
{{SearchAfter}} before any request "process"ing is done by QueryComponent.

The way this particular bug manifests – as a ClassCastException – comes from 
the fact that while a {{cursorMark=*}} param (on the initial request) can be 
parsed & validated against any {{SortSpec}}, once it's time to generate the 
{{nextCursorMark}} for inclusion in the response, the {{SortSpec}} the 
CursorMark was initialized with no longer matches the {{SortSpec}} that was 
used to process the request, and there is an extra "sort value" (from QEC) 
prepended to the list that doesn't match up with the type expected based on the 
first sort clause CursorMark knows about.

(There is actually a sanity check that the number of "sort values" matches the 
number expected, but it's currently implemented as an assertion, so it doesn't 
trip unless SOlr is running with assertions enabled)
----
Once I realized the root of the problem, I spend some time brainstorming 
possible solutions ... for the record, here are some notes on ideas I ruled 
out...
 * move the CursorMark parsing into QueryComponent.process + distributedProcess
 ** kind of defeats the point of the prepare/process abstraction
 ** would need to be done carefully to ensure we still validate the cursorMark 
locally before trying to federate to multiple shards - but also that we don't 
inadvertantly re-parse hte cursorMark during every "stage" of a 
distributedProcess loop
 * change CursorMark so that parsing & validating the current cursor against 
the sort spec happens "lazy" the first time it’s needed in SolrIndexSearcher
 ** again defeats the point of the prepare/process abstraction
 *** would mean really late failure reporting if someone provides an invalid 
cursorMark value
 ** would still be tricky to get right given how QEC completley replaces the 
SortSpec object ... CursorMark would basically need to hold a ref to the entire 
ResponseBuilder
 ** feels very hackish
 * make QEC (and or any other future component that mucks with the SortSpec) 
responsible for "re-initializing" the CursorMark after changing the SortSpec
 ** could potentialy bundle the logic into ResponseBuilder.setSortSpec ?
 ** EXCEPT: this will only work on the initial {{cursormark=*}} ?
 ** for any request after that, when QueryComponent.prepare() tries to do the 
"initial" sort param + cursor parsing, that SortSpec won't work with the values 
encoded in the cursorMark mark param at the end of the previuos request by the 
QEC modified SortSpec.
 * go back in time...
 ** re-implement CursorMark "parsing" as it's own (optional) SearchComponent:
 *** make people configure it as a 'last-component' (even after QEC)
 *** CursorMarkComponent.prepare() handles the {{cursorMark}} param parsing
 *** CursorMarkComponent.process/finish handles serialzing {{nextCursorMark}}
 ** EXCEPT: would totally break cursor behavor for all existing users who don't 
care about QEC

Here's the best idea i've come up with to fix this (that doesn't require back 
compat break)....
 * move CursorMark init into SearchHandler
 ** after the prepare() loop and before the process/distributedProcess loops
 * feels a little unclean since QueryComponent is the "user" of CursorMark
 ** but ... SearchHandler is already "aware" of the CursorMark because of how 
it handles special case situations of cursorMark searches that "time out"
 * if we do this, gate the (new) SearchHandler behavior it on whether 
ResponseBuilder.getSortSpec() is null
 ** if someone has a handler configuration that doesn't use QueryComponent (or 
any other component that sets the SortSpec) it shouldn't cause failures if a 
"useless" cursorMark param is also provided in the request

----
I'm attaching a patch that:
 * fixes CursorMark to throw a SERVER_ERROR (nt assertion check) if the some 
component still manages to produce "sort values" that don't match the SortSpec
 * implements the logic change to make SearchHandler responsible for CursorMark 
initialization
 * updates tests to use cursor w/ QEC...
 ** some very basic updates to the (single core) QEC tests to show a basic 
cursor usage works
 ** some more robust updates to the randomized Cursor tests (both single core 
and distributed) proving that a cursor walk with QEC enabled:
 *** always returns the same docs as the same search w/o elevation
 *** always returns the elevated docs first (required small test HACK to work 
around SOLR-15307)
 *** all of the docs we expect to be elevated get elevated

I'm still doing some beasting .. but curious if anyone has any feedback / 
concerns about this solution?

> cursorMark with elevateIds throws Exception
> -------------------------------------------
>
>                 Key: SOLR-11921
>                 URL: https://issues.apache.org/jira/browse/SOLR-11921
>             Project: Solr
>          Issue Type: Bug
>          Components: SearchComponents - other
>    Affects Versions: 7.2
>            Reporter: Greg Roodt
>            Assignee: Chris M. Hostetter
>            Priority: Minor
>         Attachments: SOLR-11921.patch
>
>
> When cursorMark pagination is used together with elevateIds, an exception is 
> thrown.
>  
> Steps to reproduce described below.
>  
> 1. Start solr with the `demo` core
> {{docker run --name solr_cursor_elevate -d -p 8983:8983 solr:7.2 solr-demo}}
>  
> 2. Add some test documents
> {{curl http://localhost:8983/solr/demo/update?commit=true -d '}}
> {{[}}
> {{ {"id" : "book1",}}
> {{ "title_t" : "book one"}}
> {{ },}}
> {{ {"id" : "book2",}}
> {{ "title_t" : "book two"}}
> {{ },}}
> {{ {"id" : "book3",}}
> {{ "title_t" : "book three"}}
> {{ }}}
> {{]'}}
>  
> 3. Execute a query with cursorMark and elevateIds
> curl 
> '[http://localhost:8983/solr/demo/elevate?cursorMark=*&elevateIds=book3&enableElevation=true&df=title_t&fl=id,title_t&forceElevation=true&q=book&rows=2&sort=id%20asc'|http://localhost:8983/solr/demo/elevate?cursorMark=*&elevateIds=book3&enableElevation=true&df=title_t&fl=id,title_t&forceElevation=true&q=book&rows=2&sort=id%20asc%27]
>  
> Observe the stacktrace:
> null:java.lang.ClassCastException: java.lang.Integer cannot be cast to 
> org.apache.lucene.util.BytesRef
>       at 
> org.apache.solr.schema.FieldType.marshalStringSortValue(FieldType.java:1127)
>       at org.apache.solr.schema.StrField.marshalSortValue(StrField.java:100)
>       at 
> org.apache.solr.search.CursorMark.getSerializedTotem(CursorMark.java:250)
>       at 
> org.apache.solr.handler.component.QueryComponent.doProcessUngroupedSearch(QueryComponent.java:1445)
>       at 
> org.apache.solr.handler.component.QueryComponent.process(QueryComponent.java:375)
>       at 
> org.apache.solr.handler.component.SearchHandler.handleRequestBody(SearchHandler.java:295)
>       at 
> org.apache.solr.handler.RequestHandlerBase.handleRequest(RequestHandlerBase.java:177)
>       at org.apache.solr.core.SolrCore.execute(SolrCore.java:2503)
>       at org.apache.solr.servlet.HttpSolrCall.execute(HttpSolrCall.java:710)
>       at org.apache.solr.servlet.HttpSolrCall.call(HttpSolrCall.java:516)
>       at 
> org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:382)
>       at 
> org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:326)
>       at 
> org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1751)
>       at 
> org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:582)
>       at 
> org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
>       at 
> org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:548)
>       at 
> org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:226)
>       at 
> org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1180)
>       at 
> org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:512)
>       at 
> org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185)
>       at 
> org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1112)
>       at 
> org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
>       at 
> org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:213)
>       at 
> org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:119)
>       at 
> org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:134)
>       at 
> org.eclipse.jetty.rewrite.handler.RewriteHandler.handle(RewriteHandler.java:335)
>       at 
> org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:134)
>       at org.eclipse.jetty.server.Server.handle(Server.java:534)
>       at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:320)
>       at 
> org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:251)
>       at 
> org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:283)
>       at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:108)
>       at 
> org.eclipse.jetty.io.SelectChannelEndPoint$2.run(SelectChannelEndPoint.java:93)
>       at 
> org.eclipse.jetty.util.thread.strategy.ExecuteProduceConsume.executeProduceConsume(ExecuteProduceConsume.java:303)
>       at 
> org.eclipse.jetty.util.thread.strategy.ExecuteProduceConsume.produceConsume(ExecuteProduceConsume.java:148)
>       at 
> org.eclipse.jetty.util.thread.strategy.ExecuteProduceConsume.run(ExecuteProduceConsume.java:136)
>       at 
> org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:671)
>       at 
> org.eclipse.jetty.util.thread.QueuedThreadPool$2.run(QueuedThreadPool.java:589)
>       at java.lang.Thread.run(Thread.java:748)
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to