You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Shai Erera (Created) (JIRA)" <ji...@apache.org> on 2012/02/16 12:21:00 UTC
[jira] [Created] (LUCENE-3793) Use ReferenceManager in
DirectoryTaxonomyReader
Use ReferenceManager in DirectoryTaxonomyReader
-----------------------------------------------
Key: LUCENE-3793
URL: https://issues.apache.org/jira/browse/LUCENE-3793
Project: Lucene - Java
Issue Type: Improvement
Components: modules/facet
Reporter: Shai Erera
Assignee: Shai Erera
Priority: Minor
Fix For: 3.6, 4.0
DirTaxoReader uses hairy code to protect its indexReader instance from
being modified while threads use it. It maintains a ReentrantLock
(indexReaderLock) which is obtained on every 'read' access, while
refresh() locks it for 'write' operations (refreshing the IndexReader).
Instead of all that, now that we have ReferenceManager in place, I think
that we can write a ReaderManager<IndexReader> which will be used by
DirTR. Every method that requires access to the indexReader will
acquire/release (not too different than obtaining/releasing the read
lock), and refresh() will call ReaderManager.maybeRefresh(). It will
simplify the code and remove some rather long comments, that go into
great length explaining why does the code looks like that.
This ReaderManager cannot be used for every IndexReader, because DirTR's
refresh() logic is special -- it reopens the indexReader, and then
verifies that the createTime still matches on the reopened reader as
well. Otherwise, it closes the reopened reader and fails with an exception.
Therefore, this ReaderManager.refreshIfNeeded will need to take the
createTime into consideration and fail if they do not match.
And while we're at it ... I wonder if we should have a manager for an
IndexReader/ParentArray pair? I think that it makes sense because we
don't want DirTR to use a ParentArray that does not match the IndexReader.
Today this can happen in refresh() if e.g. after the indexReader instance
has been replaced, parentArray.refresh(indexReader) fails. DirTR will be
left with a newer IndexReader instance, but old (or worse, corrupt?)
ParentArray ... I think it'll be good if we introduce clone() on ParentArray,
or a new ctor which takes an int[].
I'll work on a patch once I finish with LUCENE-3786.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Updated] (LUCENE-3793) Use ReferenceManager in
DirectoryTaxonomyReader
Posted by "Shai Erera (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-3793?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Shai Erera updated LUCENE-3793:
-------------------------------
Fix Version/s: (was: 3.6)
Removing 3.6 Fix version. If I'll make it by the release, I'll put it back.
> Use ReferenceManager in DirectoryTaxonomyReader
> -----------------------------------------------
>
> Key: LUCENE-3793
> URL: https://issues.apache.org/jira/browse/LUCENE-3793
> Project: Lucene - Java
> Issue Type: Improvement
> Components: modules/facet
> Reporter: Shai Erera
> Assignee: Shai Erera
> Priority: Minor
> Fix For: 4.0
>
>
> DirTaxoReader uses hairy code to protect its indexReader instance from
> being modified while threads use it. It maintains a ReentrantLock
> (indexReaderLock) which is obtained on every 'read' access, while
> refresh() locks it for 'write' operations (refreshing the IndexReader).
> Instead of all that, now that we have ReferenceManager in place, I think
> that we can write a ReaderManager<IndexReader> which will be used by
> DirTR. Every method that requires access to the indexReader will
> acquire/release (not too different than obtaining/releasing the read
> lock), and refresh() will call ReaderManager.maybeRefresh(). It will
> simplify the code and remove some rather long comments, that go into
> great length explaining why does the code looks like that.
> This ReaderManager cannot be used for every IndexReader, because DirTR's
> refresh() logic is special -- it reopens the indexReader, and then
> verifies that the createTime still matches on the reopened reader as
> well. Otherwise, it closes the reopened reader and fails with an exception.
> Therefore, this ReaderManager.refreshIfNeeded will need to take the
> createTime into consideration and fail if they do not match.
> And while we're at it ... I wonder if we should have a manager for an
> IndexReader/ParentArray pair? I think that it makes sense because we
> don't want DirTR to use a ParentArray that does not match the IndexReader.
> Today this can happen in refresh() if e.g. after the indexReader instance
> has been replaced, parentArray.refresh(indexReader) fails. DirTR will be
> left with a newer IndexReader instance, but old (or worse, corrupt?)
> ParentArray ... I think it'll be good if we introduce clone() on ParentArray,
> or a new ctor which takes an int[].
> I'll work on a patch once I finish with LUCENE-3786.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Resolved] (LUCENE-3793) Use ReferenceManager in
DirectoryTaxonomyReader
Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-3793?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Shai Erera resolved LUCENE-3793.
--------------------------------
Resolution: Implemented
This issue was taken care of as part of LUCENE-3441.
> Use ReferenceManager in DirectoryTaxonomyReader
> -----------------------------------------------
>
> Key: LUCENE-3793
> URL: https://issues.apache.org/jira/browse/LUCENE-3793
> Project: Lucene - Core
> Issue Type: Improvement
> Components: modules/facet
> Reporter: Shai Erera
> Assignee: Shai Erera
> Priority: Minor
> Fix For: 4.1
>
>
> DirTaxoReader uses hairy code to protect its indexReader instance from
> being modified while threads use it. It maintains a ReentrantLock
> (indexReaderLock) which is obtained on every 'read' access, while
> refresh() locks it for 'write' operations (refreshing the IndexReader).
> Instead of all that, now that we have ReferenceManager in place, I think
> that we can write a ReaderManager<IndexReader> which will be used by
> DirTR. Every method that requires access to the indexReader will
> acquire/release (not too different than obtaining/releasing the read
> lock), and refresh() will call ReaderManager.maybeRefresh(). It will
> simplify the code and remove some rather long comments, that go into
> great length explaining why does the code looks like that.
> This ReaderManager cannot be used for every IndexReader, because DirTR's
> refresh() logic is special -- it reopens the indexReader, and then
> verifies that the createTime still matches on the reopened reader as
> well. Otherwise, it closes the reopened reader and fails with an exception.
> Therefore, this ReaderManager.refreshIfNeeded will need to take the
> createTime into consideration and fail if they do not match.
> And while we're at it ... I wonder if we should have a manager for an
> IndexReader/ParentArray pair? I think that it makes sense because we
> don't want DirTR to use a ParentArray that does not match the IndexReader.
> Today this can happen in refresh() if e.g. after the indexReader instance
> has been replaced, parentArray.refresh(indexReader) fails. DirTR will be
> left with a newer IndexReader instance, but old (or worse, corrupt?)
> ParentArray ... I think it'll be good if we introduce clone() on ParentArray,
> or a new ctor which takes an int[].
> I'll work on a patch once I finish with LUCENE-3786.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-3793) Use ReferenceManager in
DirectoryTaxonomyReader
Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-3793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13500222#comment-13500222 ]
Shai Erera commented on LUCENE-3793:
------------------------------------
The changes in LUCENE-3441 kind of eliminate the need for this issue. I'll cancel it once I commit the changes in LUCENE-3441.
> Use ReferenceManager in DirectoryTaxonomyReader
> -----------------------------------------------
>
> Key: LUCENE-3793
> URL: https://issues.apache.org/jira/browse/LUCENE-3793
> Project: Lucene - Core
> Issue Type: Improvement
> Components: modules/facet
> Reporter: Shai Erera
> Assignee: Shai Erera
> Priority: Minor
> Fix For: 4.1
>
>
> DirTaxoReader uses hairy code to protect its indexReader instance from
> being modified while threads use it. It maintains a ReentrantLock
> (indexReaderLock) which is obtained on every 'read' access, while
> refresh() locks it for 'write' operations (refreshing the IndexReader).
> Instead of all that, now that we have ReferenceManager in place, I think
> that we can write a ReaderManager<IndexReader> which will be used by
> DirTR. Every method that requires access to the indexReader will
> acquire/release (not too different than obtaining/releasing the read
> lock), and refresh() will call ReaderManager.maybeRefresh(). It will
> simplify the code and remove some rather long comments, that go into
> great length explaining why does the code looks like that.
> This ReaderManager cannot be used for every IndexReader, because DirTR's
> refresh() logic is special -- it reopens the indexReader, and then
> verifies that the createTime still matches on the reopened reader as
> well. Otherwise, it closes the reopened reader and fails with an exception.
> Therefore, this ReaderManager.refreshIfNeeded will need to take the
> createTime into consideration and fail if they do not match.
> And while we're at it ... I wonder if we should have a manager for an
> IndexReader/ParentArray pair? I think that it makes sense because we
> don't want DirTR to use a ParentArray that does not match the IndexReader.
> Today this can happen in refresh() if e.g. after the indexReader instance
> has been replaced, parentArray.refresh(indexReader) fails. DirTR will be
> left with a newer IndexReader instance, but old (or worse, corrupt?)
> ParentArray ... I think it'll be good if we introduce clone() on ParentArray,
> or a new ctor which takes an int[].
> I'll work on a patch once I finish with LUCENE-3786.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org