[ 
https://issues.apache.org/jira/browse/SOLR-8889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15209810#comment-15209810
 ] 

David Smiley commented on SOLR-8889:
------------------------------------

RE broadcast:  There might not be enough time for the 6.0 release to make it 
broadcast, but be my guest to go for it if you have the time.  An exception is 
reasonable if we can't do the right thing; it's way better then silently doing 
the wrong thing!  In the case of the implicit router, I don't think a delete 
should be broadcast because it's a do-it-yourself router requiring you to know 
where to send things by definition.  That said, the ability to explicitly tell 
Solr to broadcast would be a nice convenience feature, apart from this issue.

I looked at some of the related issues you pointed at.  Arguably this issue is 
the same as SOLR-5890 but IMO SOLR-5890 isn't "done" because it only helps for 
those people who know about UpdateRequest.setRoute -- and few would notice that 
given the SolrClient.deleteById method.  Indeed I did try setRoute and it works 
(yay) but, wow, what a gotcha for those that don't know.

Even with UpdateRequest.setRoute() working (I tweaked my patch to try it and 
observe it worked), I see a bug, verified via debugging & stepping through.  
CloudSolrClient fails to honor the route and thus sends the delete-by-id 
requests based on hashing the ID (wrong).  Interestingly, ultimately it appears 
Solr on the server side somehow knows what to do since the test passes.  What 
happens is, CSC.directUpdate calls UpdateRequest.getRoutes.  At line 288 when 
it calls router.getTargetSlice, it does not provide "_route_" from the map.  
Doh!  So instead it hashes on the ID (what CompositeIdRouter falls back on).

Also, don't you think the MDC.put & MDC.remove (line 603 & 611) should be moved 
to be _within_ the callable, not outside? Right?

As an aside, I don't see why the route must end in an exclamation -- that's 
weird and easy to forget.  I forgot the first time and didn't get it to work.  
That's another gotcha, we should perhaps automatically add one?  [~noble.paul] 
do you know why it's this way?  If I recall, you added this feature.

> SolrCloud deleteById is broken when router.field is set
> -------------------------------------------------------
>
>                 Key: SOLR-8889
>                 URL: https://issues.apache.org/jira/browse/SOLR-8889
>             Project: Solr
>          Issue Type: Bug
>          Components: SolrCloud, SolrJ
>            Reporter: David Smiley
>         Attachments: SOLR_8889_investigation.patch
>
>
> If you set router.field on your collection to shard by something other than 
> the ID, then deleting documents by ID fails some of the time (how much 
> depends on how sharded the collection is).  I suspect that it'd work if the 
> IDs provided when deleting by ID were prefixed using the composite key syntax 
> -- "routekey!id" though I didn't check.  This is terrible.  Internally Solr 
> should broadcast to all the shards if there is no composite key prefix.
> Some affected code is UpdateRequest.getRoutes.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to