[
https://issues.apache.org/jira/browse/SOLR-5754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18131116#comment-18131116
]
Nick Shanin edited comment on SOLR-5754 at 10/23/26, 8:03 PM:
--------------------------------------------------------------
🤖 _AI text below_ 🤖 _(reviewed by Nick Shanin)_
I am closing PR #5073 without building this. The ticket noted in 2014 that the
error list is only accessed in a single-threaded manner, and tracing the
current code shows that is still what protects it:
* The only production writer of the list in {{StreamingSolrClients}} is
{{ErrorReportingConcurrentUpdateSolrClient.handleError}}, which runs on the
client's Runner threads.
* {{SolrCmdDistributor.doRetriesIfNeeded}}, the one place that copies the list
and clears it later, runs only after {{blockUntilFinished()}} returns, when no
Runner thread that could add an error is still alive. New errors can only
arrive through the retry pass's own resubmission, which happens after the clear.
So the copy-then-clear sequence cannot lose an error: the interleaving the
change guarded against cannot occur. Returning a copy would be harmless, but it
fixes nothing, so I am not going to push it. Closing the PR.
I recommend closing this as Not A Problem.
was (Author: JIRAUSER314749):
🤖 _AI text below_ 🤖 _(posted on behalf of Nick Shanin)_
I am closing PR #5073 without building this. The ticket noted in 2014 that the
error list is only accessed in a single-threaded manner, and tracing the
current code shows that is still what protects it:
* The only production writer of the list in {{StreamingSolrClients}} is
{{ErrorReportingConcurrentUpdateSolrClient.handleError}}, which runs on the
client's Runner threads.
* {{SolrCmdDistributor.doRetriesIfNeeded}}, the one place that copies the list
and clears it later, runs only after {{blockUntilFinished()}} returns, when no
Runner thread that could add an error is still alive. New errors can only
arrive through the retry pass's own resubmission, which happens after the clear.
So the copy-then-clear sequence cannot lose an error: the interleaving the
change guarded against cannot occur. Returning a copy would be harmless, but it
fixes nothing, so I am not going to push it. Closing the PR.
I recommend closing this as Not A Problem.
> SolrStreamingServers returns a synchronizedList of Errors.
> ----------------------------------------------------------
>
> Key: SOLR-5754
> URL: https://issues.apache.org/jira/browse/SOLR-5754
> Project: Solr
> Issue Type: Bug
> Reporter: Mark Miller
> Assignee: Mark Miller
> Priority: Major
> Labels: pull-request-available
> Fix For: 4.9, 6.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> [~gchanan] pointed this out to me. I'm not sure how easy it would be to see
> an issue - SolrStreamingServers was made to be thread safe, but we actually
> only currently access it in a single threaded manner.
> We should return a copy of the list.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
