stillalex commented on PR #1504:
URL: https://github.com/apache/solr/pull/1504#issuecomment-1492441639
I have pushed a tentative patch for review and more importantly for
discussion.
I have ran this test extensively (hundreds of beast jobs) and it looks like
this is the way to go, all 'add' exceptions are gone once the isLeader flag is
updated. BUT I am not completely convinced this is the correct patch. possibly
something could be made better in the DistributedZkUpdateProcessor area instead
(where a lot of leader calculations are being made but the result is not
correct in this edge case).
While I am still evaluating a patch for DistributedZkUpdateProcessor, I
would love some feedback on the observations so far.
Together with the patch I have added a check which will not allow additions
with no version. this is now consistent with the delete flows. although I must
admit I am confused by the fact that this check was missing on this execution
path, while the delete path had this from at least 8 years ago.
Other points
- in the current test, there are a few operations (add and deletes) that
run parallel to the split operation. these can fail and this is where the bug
is. because the exceptions are not correctly accounted for, the NPE popped up
at a weird place. I added tracking and assertions on this part too (except for
deletes, see below)
- deletes can also cause exceptions, I don't have a good handle on how to
fix this, so I removed the strict error check. this can be added once the
confidence level is higher and this flow is accounted for. until then I will
focus on the addition parts.
- DistributedZkUpdateProcessor will call setupRequest twice. this is
probably a minor nuisance for some future improvement PR.
- finding the code very hard to read I took the decision to remove one
variable that is not really needed (the `updateCommand`), this change is purely
cosmetic and I can revert it if it adds too much noise to the current patch.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
