[
https://issues.apache.org/jira/browse/SOLR-13833?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18121775#comment-18121775
]
Nick Shanin edited comment on SOLR-13833 at 10/2/26 4:38 AM:
-------------------------------------------------------------
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)*
I investigated this with the intent of submitting a PR, and I'm now fairly
convinced the duplicate call should stay.
I prototyped the obvious fix: guarding
`DistributedZkUpdateProcessor.setupRequest(UpdateCommand)` so the second
invocation for the same command object becomes a no-op. It deterministically
breaks `TestCloudDeduplication.testRandomDocs` (seed `CDC6046FA2F3FF5E`:
expected 52 docs, got 77; the same seed passes on unpatched main). The second
call is not a pure duplicate: it re-reads live cluster state via
`zkController.getClusterState()` and re-resolves shard leadership
(`getLeaderRetry`), so the two invocations can compute different
`isLeader`/`forwardToLeader`/`nodes` answers, and the update path depends on
the fresher one.
I also measured the remaining `waitForState` contention that motivated this
ticket, since SOLR-17004 added a cluster-state cache fast path in the meantime.
On a 3-node cluster with 8 concurrent add/delete workers over 60s windows: fast
path enabled gave 218.5 and 221.7 ops/s with a 100% cache hit rate (0 watcher
registrations); fast path disabled gave 210.2 and 231.4 ops/s with ~50-55k
watcher registrations. The ranges overlap; run-to-run noise dominates. The fast
path already absorbs the contention, so there is no measurable win left to
chase here.
Since the second call does work the first call's result can't replace, "fixing"
the double invocation is riskier than the wasted work, and the measurement says
the waste is negligible anyway. I'd suggest resolving this as Won't Fix.
was (Author: JIRAUSER314749):
!https://fonts.gstatic.com/s/e/notoemoji/17.0/1f916/32.png! AI text below
!https://fonts.gstatic.com/s/e/notoemoji/17.0/1f916/32.png! (posted on behalf
of Nick Shanin)
I investigated this with the intent of submitting a PR, and I'm now fairly
convinced the duplicate call should stay.
I prototyped the obvious fix: guarding
DistributedZkUpdateProcessor.setupRequest(UpdateCommand) so the second
invocation for the same command object becomes a no-op. It deterministically
breaks TestCloudDeduplication.testRandomDocs (seed CDC6046FA2F3FF5E: expected
52 docs, got 77; the same seed passes on unpatched main). The second call is
not a pure duplicate: it re-reads live cluster state via
zkController.getClusterState() and re-resolves shard leadership
(getLeaderRetry), so the two invocations can compute different
isLeader/forwardToLeader/nodes answers, and the update path depends on the
fresher one.
Since the second call does work the first call's result can't replace, "fixing"
the double invocation is riskier than the wasted work. Unless someone sees a
safe way to narrow it (e.g. skipping only when the cluster-state version is
unchanged between the two calls), I'd suggest resolving this as Won't Fix.
> setupRequest normally called twice
> ----------------------------------
>
> Key: SOLR-13833
> URL: https://issues.apache.org/jira/browse/SOLR-13833
> Project: Solr
> Issue Type: Bug
> Reporter: Yonik Seeley
> Priority: Major
> Attachments: Screenshot 2023-08-31 at 10.18.49.png
>
> Time Spent: 2h 10m
> Remaining Estimate: 0h
>
> I think this was introduced in SOLR-12955, but setupRequest is called
> twice...
> for example a single "add" causes it to be called once in
> DistributedZkUpdateProcessor.processAdd() and then again in
> DistributedUpdateProcessor.processAdd()
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
