[SOLR-18114] CloudSolrClient fails deleteById with directUpdatesToLeadersOnly but no route passed#4219
[SOLR-18114] CloudSolrClient fails deleteById with directUpdatesToLeadersOnly but no route passed#4219mlbiscoc wants to merge 4 commits intoapache:mainfrom
Conversation
| SolrException.ErrorCode.SERVICE_UNAVAILABLE, | ||
| "directUpdatesToLeadersOnly==true but could not find leader(s)"); | ||
| } else { | ||
| // we could not find a leader or routes yet - use unoptimized general path |
There was a problem hiding this comment.
I only ran into this bug because it was forgotten to pass the the routing info on delete. On the flip side, if this exception didn't occur, deletes would have passed and I never would have known deletes we're being sent in an unoptimized way. One way to catch is to add a log here to a SolrJ client but I think that'd be way to noisy. Maybe a debug log? Or maybe that is the correct way and just never log.
There was a problem hiding this comment.
I think this is definitely worth a debug log at least; even "info" is defensible since, after all, people use CSC because it will route optimally. So if it can't, it's noteworthy. We can test for this via LogListener.
There was a problem hiding this comment.
If we're ok with this, then I added a single log.warn message there and added the LogListener test.
There was a problem hiding this comment.
you one-upped me and went to warn :-) LOL sure, what the heck. But you're on the hook if anyone complains ;-)
dsmiley
left a comment
There was a problem hiding this comment.
Just some minor feedback but I like the idea of a log
| return results; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Thanks for adding much-needed javadocs!
| .build()) { | ||
|
|
||
| UpdateRequest addRequest = new UpdateRequest(); | ||
| addRequest.add("id", "doc1", "router_field_s", "testRoute"); |
There was a problem hiding this comment.
let's pick a doc ID and router_field_s value randomly, unless you need a consistent value for some reason?
|
Is there something relevant to my PR that won't let SolrJ CI run? I have doubts because it looks like a permission issue. |
|
Your PR couldn't have anything to do with that. merge anyway. But thanks for bringing it to my attention. It's a chore when sporadically CI hiccups or something, which seems increasingly so. I'll look into this specific matter. |
https://issues.apache.org/jira/browse/SOLR-18114
CloudSolrClient fails when collection has router field configure deleteById and has
directUpdatesToLeadersOnly=truebut is sent without routing info.This is because of a bug where
hasInfoToFindLeaders()method never actually checked if the deleteById documents had routing info, causing the method to incorrectly returntrue. This results in a SERVICE_UNAVAILABLE exception instead of falling back to the unoptimized general path to broadcast deletes to all shards.