Re: Synchronization in SolrClientCache looks wrong
Right, thanks. It was a rushed day yesterday and misread "object" in
the javadocs. Also misdirected by the fact that I had a slightly
different version of some code that produces an NPE and was thinking
that the problem was closing a null SolrClient instead of the variable
two lines above it.
Never mind.
On Fri, Aug 11, 2017 at 8:45 AM, Tomas Fernandez Lobbe
wrote:
> The synchronized at the method level blocks any other call to any
> synchronized method on the same object, so this is correct.
>
>> On Aug 10, 2017, at 4:34 PM, Erick Erickson wrote:
>>
>> And if it's impossible for more than one thread to be using the class,
>> why bother to synchronize?
>>
>> Or am I misreading things totally?
>>
>> On Thu, Aug 10, 2017 at 4:21 PM, Erick Erickson
>> wrote:
>>> What prevents one thread from calling a get operation while another is
>>> closing out from underneath? Or is it impossible that more than one thread
>>> have access to this object?
>>>
>>>
>>> On Aug 10, 2017 2:32 PM, "Joel Bernstein" wrote:
>>>
>>> Just took a look at the class and it seems OK. All methods are synchronized,
>>> so we shouldn't have any race conditions. What are your concerns?
>>>
>>> Joel Bernstein
>>> http://joelsolr.blogspot.com/
>>>
>>> On Thu, Aug 10, 2017 at 4:32 PM, Tomas Fernandez Lobbe
>>> wrote:
You mean, remove the synchronization from the method and synchronize the
whole thing on solrClients? How is that different?
> On Aug 10, 2017, at 12:48 PM, Erick Erickson
> wrote:
>
> All the methods are synchronized, but they all operate on the
>
> private final Map solrClients;
>
> member variable, including code like this:
>
> if (solrClients.containsKey(host)) {
> client = (HttpSolrClient) solrClients.get(host);
> }
>
> But the close method goes through the list closing all the entries in
> solrClients. Seems like these ought to be synchronized blocks on
> solrClients
>
> I have another place that needs updating too that I was investigating
> when I saw this so I'll do them both at once if so (SOLR-11224)
>
> Erick
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
>
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
>>>
>>>
>>
>> -
>> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
>> For additional commands, e-mail: dev-h...@lucene.apache.org
>>
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
>
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
Re: Synchronization in SolrClientCache looks wrong
The synchronized at the method level blocks any other call to any synchronized
method on the same object, so this is correct.
> On Aug 10, 2017, at 4:34 PM, Erick Erickson wrote:
>
> And if it's impossible for more than one thread to be using the class,
> why bother to synchronize?
>
> Or am I misreading things totally?
>
> On Thu, Aug 10, 2017 at 4:21 PM, Erick Erickson
> wrote:
>> What prevents one thread from calling a get operation while another is
>> closing out from underneath? Or is it impossible that more than one thread
>> have access to this object?
>>
>>
>> On Aug 10, 2017 2:32 PM, "Joel Bernstein" wrote:
>>
>> Just took a look at the class and it seems OK. All methods are synchronized,
>> so we shouldn't have any race conditions. What are your concerns?
>>
>> Joel Bernstein
>> http://joelsolr.blogspot.com/
>>
>> On Thu, Aug 10, 2017 at 4:32 PM, Tomas Fernandez Lobbe
>> wrote:
>>>
>>> You mean, remove the synchronization from the method and synchronize the
>>> whole thing on solrClients? How is that different?
>>>
>>>
On Aug 10, 2017, at 12:48 PM, Erick Erickson
wrote:
All the methods are synchronized, but they all operate on the
private final Map solrClients;
member variable, including code like this:
if (solrClients.containsKey(host)) {
client = (HttpSolrClient) solrClients.get(host);
}
But the close method goes through the list closing all the entries in
solrClients. Seems like these ought to be synchronized blocks on
solrClients
I have another place that needs updating too that I was investigating
when I saw this so I'll do them both at once if so (SOLR-11224)
Erick
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
>>>
>>>
>>> -
>>> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
>>> For additional commands, e-mail: dev-h...@lucene.apache.org
>>>
>>
>>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
>
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
Re: Synchronization in SolrClientCache looks wrong
And if it's impossible for more than one thread to be using the class,
why bother to synchronize?
Or am I misreading things totally?
On Thu, Aug 10, 2017 at 4:21 PM, Erick Erickson wrote:
> What prevents one thread from calling a get operation while another is
> closing out from underneath? Or is it impossible that more than one thread
> have access to this object?
>
>
> On Aug 10, 2017 2:32 PM, "Joel Bernstein" wrote:
>
> Just took a look at the class and it seems OK. All methods are synchronized,
> so we shouldn't have any race conditions. What are your concerns?
>
> Joel Bernstein
> http://joelsolr.blogspot.com/
>
> On Thu, Aug 10, 2017 at 4:32 PM, Tomas Fernandez Lobbe
> wrote:
>>
>> You mean, remove the synchronization from the method and synchronize the
>> whole thing on solrClients? How is that different?
>>
>>
>> > On Aug 10, 2017, at 12:48 PM, Erick Erickson
>> > wrote:
>> >
>> > All the methods are synchronized, but they all operate on the
>> >
>> > private final Map solrClients;
>> >
>> > member variable, including code like this:
>> >
>> > if (solrClients.containsKey(host)) {
>> > client = (HttpSolrClient) solrClients.get(host);
>> > }
>> >
>> > But the close method goes through the list closing all the entries in
>> > solrClients. Seems like these ought to be synchronized blocks on
>> > solrClients
>> >
>> > I have another place that needs updating too that I was investigating
>> > when I saw this so I'll do them both at once if so (SOLR-11224)
>> >
>> > Erick
>> >
>> > -
>> > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
>> > For additional commands, e-mail: dev-h...@lucene.apache.org
>> >
>>
>>
>> -
>> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
>> For additional commands, e-mail: dev-h...@lucene.apache.org
>>
>
>
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
Re: Synchronization in SolrClientCache looks wrong
What prevents one thread from calling a get operation while another is
closing out from underneath? Or is it impossible that more than one thread
have access to this object?
On Aug 10, 2017 2:32 PM, "Joel Bernstein" wrote:
Just took a look at the class and it seems OK. All methods are
synchronized, so we shouldn't have any race conditions. What are your
concerns?
Joel Bernstein
http://joelsolr.blogspot.com/
On Thu, Aug 10, 2017 at 4:32 PM, Tomas Fernandez Lobbe
wrote:
> You mean, remove the synchronization from the method and synchronize the
> whole thing on solrClients? How is that different?
>
>
> > On Aug 10, 2017, at 12:48 PM, Erick Erickson
> wrote:
> >
> > All the methods are synchronized, but they all operate on the
> >
> > private final Map solrClients;
> >
> > member variable, including code like this:
> >
> > if (solrClients.containsKey(host)) {
> > client = (HttpSolrClient) solrClients.get(host);
> > }
> >
> > But the close method goes through the list closing all the entries in
> > solrClients. Seems like these ought to be synchronized blocks on
> > solrClients
> >
> > I have another place that needs updating too that I was investigating
> > when I saw this so I'll do them both at once if so (SOLR-11224)
> >
> > Erick
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> > For additional commands, e-mail: dev-h...@lucene.apache.org
> >
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
>
>
Re: Synchronization in SolrClientCache looks wrong
Just took a look at the class and it seems OK. All methods are
synchronized, so we shouldn't have any race conditions. What are your
concerns?
Joel Bernstein
http://joelsolr.blogspot.com/
On Thu, Aug 10, 2017 at 4:32 PM, Tomas Fernandez Lobbe
wrote:
> You mean, remove the synchronization from the method and synchronize the
> whole thing on solrClients? How is that different?
>
>
> > On Aug 10, 2017, at 12:48 PM, Erick Erickson
> wrote:
> >
> > All the methods are synchronized, but they all operate on the
> >
> > private final Map solrClients;
> >
> > member variable, including code like this:
> >
> > if (solrClients.containsKey(host)) {
> > client = (HttpSolrClient) solrClients.get(host);
> > }
> >
> > But the close method goes through the list closing all the entries in
> > solrClients. Seems like these ought to be synchronized blocks on
> > solrClients
> >
> > I have another place that needs updating too that I was investigating
> > when I saw this so I'll do them both at once if so (SOLR-11224)
> >
> > Erick
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> > For additional commands, e-mail: dev-h...@lucene.apache.org
> >
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
>
>
Re: Synchronization in SolrClientCache looks wrong
You mean, remove the synchronization from the method and synchronize the whole
thing on solrClients? How is that different?
> On Aug 10, 2017, at 12:48 PM, Erick Erickson wrote:
>
> All the methods are synchronized, but they all operate on the
>
> private final Map solrClients;
>
> member variable, including code like this:
>
> if (solrClients.containsKey(host)) {
> client = (HttpSolrClient) solrClients.get(host);
> }
>
> But the close method goes through the list closing all the entries in
> solrClients. Seems like these ought to be synchronized blocks on
> solrClients
>
> I have another place that needs updating too that I was investigating
> when I saw this so I'll do them both at once if so (SOLR-11224)
>
> Erick
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
>
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
Synchronization in SolrClientCache looks wrong
All the methods are synchronized, but they all operate on the
private final Map solrClients;
member variable, including code like this:
if (solrClients.containsKey(host)) {
client = (HttpSolrClient) solrClients.get(host);
}
But the close method goes through the list closing all the entries in
solrClients. Seems like these ought to be synchronized blocks on
solrClients
I have another place that needs updating too that I was investigating
when I saw this so I'll do them both at once if so (SOLR-11224)
Erick
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
