[jira] Issue Comment Edited: (SOLR-647) Do SolrCore.close() in a refcounted way
[
https://issues.apache.org/jira/browse/SOLR-647?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12622575#action_12622575
]
noble.paul edited comment on SOLR-647 at 8/14/08 8:36 AM:
--
I fail to see the need for this code in SolrCore#close()
{code:java}
int count;
do {
count = refCount.get();
if (count = 0)
return;
} while (!refCount.compareAndSet(count, count - 1));
{code}
If you do refCount.decrementAndGet() it should be fine.
similarly in SolrCore$open() also
you can do a refCount.incrementAndGet() and will be fine.
What you have written is a duplication of code in AtomicInteger
Even the RefCounted class does increment and decrement in the same way
Yonik's patch does it simply and I do not see anything wrong with that.
was (Author: noble.paul):
I fail to see the need for this code in SolrCore#close()
{code:java}
int count;
do {
count = refCount.get();
if (count = 0)
return;
} while (!refCount.compareAndSet(count, count - 1));
{code}
If you do refCount.decrementAndGet() it should be fine.
similarly in SolrCore$open() also
you can do a refCount.incrementAndGet() and will be fine.
What you have written is a duplication of code in AtomicInteger
Even the RefCounted class does increment and decrement in the same way
Do SolrCore.close() in a refcounted way
---
Key: SOLR-647
URL: https://issues.apache.org/jira/browse/SOLR-647
Project: Solr
Issue Type: Bug
Affects Versions: 1.3
Reporter: Noble Paul
Assignee: Grant Ingersoll
Fix For: 1.3
Attachments: refcount_example.patch, solr-647.patch, solr-647.patch,
solr-647.patch, solr-647.patch, solr-647.patch, solr-647.patch,
solr-647.patch, SOLR-647.patch, SOLR-647.patch
The method _SolrCore.close()_ directly closes the core . It can cause
Exceptions for in-flight requests. The _close()_ method should just do a
decrement on refcount and the actual close must happen when the last request
being processed by that core instance is completed
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (SOLR-647) Do SolrCore.close() in a refcounted way
[
https://issues.apache.org/jira/browse/SOLR-647?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12622583#action_12622583
]
henrib edited comment on SOLR-647 at 8/14/08 8:46 AM:
-
About the SolrCore.open() close():
If we were to do an incrementAndGet(), we could end up opening a closed core;
We thus must check the refcount is not =0 first.
The close could use a decrementAndGet() but the current code ensures the count
will never go 0 and is symmetrical to open.
In both cases, it is the test in between the get() compareAndSet() that makes
the whole difference with {in,de]crementAndGet.
My understanding of Yonik's version is that, as a premise, opening a core is
always performed under the CoreContainer#cores synchronized protection; as I
explained in a previous comment, the assumption can not be strictly met.
was (Author: henrib):
About the SolrCore.open() close():
If we were to do an incrementAndGet(), we could end up opening a closed core
since we can not always be under the CoreContainer#cores synchronized
protection.
We thus must check the refcount is not =0 first.
The close could use a decrementAndGet() but the current code ensures the count
will never go 0.
Itn both cases, it is the test in between the get() compareAndSet() that
makes the whole difference with {in,de]crementAndGet.
Do SolrCore.close() in a refcounted way
---
Key: SOLR-647
URL: https://issues.apache.org/jira/browse/SOLR-647
Project: Solr
Issue Type: Bug
Affects Versions: 1.3
Reporter: Noble Paul
Assignee: Grant Ingersoll
Fix For: 1.3
Attachments: refcount_example.patch, solr-647.patch, solr-647.patch,
solr-647.patch, solr-647.patch, solr-647.patch, solr-647.patch,
solr-647.patch, SOLR-647.patch, SOLR-647.patch
The method _SolrCore.close()_ directly closes the core . It can cause
Exceptions for in-flight requests. The _close()_ method should just do a
decrement on refcount and the actual close must happen when the last request
being processed by that core instance is completed
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (SOLR-647) Do SolrCore.close() in a refcounted way
[ https://issues.apache.org/jira/browse/SOLR-647?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12621867#action_12621867 ] henrib edited comment on SOLR-647 at 8/12/08 10:13 AM: -- Grant, bq.At a minimum, all that should be documented I'll adapt the previous comments accordingly to make the Javadoc more obvious. bq. the MultiCoreHandler uses getCore() in places, while the SolrDispatchFilter uses the Reference Good catch; the admin core (the one used to generate output) is protected but is this enough? The MultiCoreHandler only deals with CoreDescriptor (not SolrCore); it does not perform real queries although getStatus comes close to it. However, it would seem preferable to avoid manipulating the (multicore) cores map while we are are processing such requests. I'll fix it in the next upload. Thanks for reviewing this. was (Author: henrib): Grant, bq.At a minimum, all that should be documented I'll adapt the previous comments accordingly. bq. the MultiCoreHandler uses getCore() in places, while the SolrDispatchFilter uses the Reference Good catch; the admin core is protected but not the targeted core... I'll fix it in the next upload. Thanks for reviewing this. Do SolrCore.close() in a refcounted way --- Key: SOLR-647 URL: https://issues.apache.org/jira/browse/SOLR-647 Project: Solr Issue Type: Bug Affects Versions: 1.3 Reporter: Noble Paul Assignee: Grant Ingersoll Fix For: 1.3 Attachments: solr-647.patch, solr-647.patch, solr-647.patch, solr-647.patch, solr-647.patch, SOLR-647.patch, SOLR-647.patch The method _SolrCore.close()_ directly closes the core . It can cause Exceptions for in-flight requests. The _close()_ method should just do a decrement on refcount and the actual close must happen when the last request being processed by that core instance is completed -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (SOLR-647) Do SolrCore.close() in a refcounted way
[
https://issues.apache.org/jira/browse/SOLR-647?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12620276#action_12620276
]
henrib edited comment on SOLR-647 at 8/6/08 10:13 AM:
-
Looking at both versions of the patch, it seems you did not upload the intended
one..
was (Author: henrib):
It seems there is still the possibility that between the multicore.getCore()
core.incrementRef(), some other thread gets the core to close.
May be we can have:
{code}
public SolrCore incrementRef() {
return (refCount.incrementAndGet() 1)? this : null;
}
{code}
And in SolrCoreDispatcher do:
{code}
core = multicore.getCore(name);
if (core != null)
core = core.incrementRef();
{code}
Oh, looking at both versions of the patch, it seems you did not upload the
intended one..
Do SolrCore.close() in a refcounted way
---
Key: SOLR-647
URL: https://issues.apache.org/jira/browse/SOLR-647
Project: Solr
Issue Type: Bug
Affects Versions: 1.3
Reporter: Noble Paul
Fix For: 1.3
Attachments: SOLR-647.patch, SOLR-647.patch
The method _SolrCore.close()_ directly closes the core . It can cause
Exceptions for in-flight requests. The _close()_ method should just do a
decrement on refcount and the actual close must happen when the last request
being processed by that core instance is completed
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
