You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Christoph Goller <go...@detego-software.de> on 2003/09/12 14:01:47 UTC
PATCH: IndexReaderDelete (Bugzilla Bug 12588)
This is a patch for Bug 12588.
Problem:
If a reader is opened (reader1) on an index without acquiring a write.lock and
later the index is changed by either another reader (reader2) or a writer, the
first reader can still acquire a write.lock and change the index. Thus
the changes by reader1 or the writer are lost. My patch adds an additional check
to Indexreader.delete(int) which checks whether the index has changed since the reader
was opened. Since this check is done with a valid write.lock no other reader/writer
could interfere with the check. If the index has been changed the reader is not
granted write permission. The only way to delete a document in such a case is to
get a new reader and call delete there. The patch for IndexReader and a unit test are
attached.
Christoph
--
*****************************************************************
* Dr. Christoph Goller Tel.: +49 89 203 45734 *
* Detego Software GmbH Mobile: +49 179 1128469 *
* Keuslinstr. 13 Fax.: +49 721 151516176 *
* 80798 München, Germany Email: goller@detego-software.de *
*****************************************************************
Re: PATCH: IndexReaderDelete (Bugzilla Bug 12588)
Posted by Doug Cutting <cu...@lucene.com>.
+1
Thanks again for the patches,
Doug
Christoph Goller wrote:
> This is a patch for Bug 12588.
>
> Problem:
>
> If a reader is opened (reader1) on an index without acquiring a
> write.lock and
> later the index is changed by either another reader (reader2) or a
> writer, the
> first reader can still acquire a write.lock and change the index. Thus
> the changes by reader1 or the writer are lost. My patch adds an
> additional check
> to Indexreader.delete(int) which checks whether the index has changed
> since the reader
> was opened. Since this check is done with a valid write.lock no other
> reader/writer
> could interfere with the check. If the index has been changed the reader
> is not
> granted write permission. The only way to delete a document in such a
> case is to
> get a new reader and call delete there. The patch for IndexReader and a
> unit test are
> attached.
>
> Christoph
Re: PATCH: IndexReaderDelete (Bugzilla Bug 12588)
Posted by Otis Gospodnetic <ot...@yahoo.com>.
Hello Christoph,
Thanks for the fixes and additional unit tests.
I applied your patches and added your test cases to TestIndexReader
unit test class. I will commit TestIndexReader now, but note that the
reader-reader test still fails sometimes/often (see below).
I will therefore wait with the commit of IndexReader.
[junit] Testsuite: org.apache.lucene.index.TestDelete
[junit] Tests run: 2, Failures: 1, Errors: 0, Time elapsed: 0.658
sec
[junit] Testcase:
testDeleteReaderReaderConflict(org.apache.lucene.index.TestDelete):
FAILED
[junit] expected:<0> but was:<100>
[junit] junit.framework.AssertionFailedError: expected:<0> but
was:<100>
[junit] at
org.apache.lucene.index.TestDelete.testDeleteReaderReaderConflict(TestDelete.java:149)
[junit] at sun.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)
[junit] at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
[junit] at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
[junit] TEST org.apache.lucene.index.TestDelete FAILED
Otis
--- Christoph Goller <go...@detego-software.de> wrote:
> This is a patch for Bug 12588.
>
> Problem:
>
> If a reader is opened (reader1) on an index without acquiring a
> write.lock and
> later the index is changed by either another reader (reader2) or a
> writer, the
> first reader can still acquire a write.lock and change the index.
> Thus
> the changes by reader1 or the writer are lost. My patch adds an
> additional check
> to Indexreader.delete(int) which checks whether the index has changed
> since the reader
> was opened. Since this check is done with a valid write.lock no other
> reader/writer
> could interfere with the check. If the index has been changed the
> reader is not
> granted write permission. The only way to delete a document in such a
> case is to
> get a new reader and call delete there. The patch for IndexReader and
> a unit test are
> attached.
>
> Christoph
>
> --
> *****************************************************************
> * Dr. Christoph Goller Tel.: +49 89 203 45734 *
> * Detego Software GmbH Mobile: +49 179 1128469 *
> * Keuslinstr. 13 Fax.: +49 721 151516176 *
> * 80798 M�nchen, Germany Email: goller@detego-software.de *
> *****************************************************************
> > Index: IndexReader.java
> ===================================================================
> RCS file:
>
/home/cvspublic/jakarta-lucene/src/java/org/apache/lucene/index/IndexReader.java,v
> retrieving revision 1.17
> diff -u -r1.17 IndexReader.java
> --- IndexReader.java 12 Aug 2003 15:05:03 -0000 1.17
> +++ IndexReader.java 12 Sep 2003 11:28:14 -0000
> @@ -80,10 +80,14 @@
> public abstract class IndexReader {
> protected IndexReader(Directory directory) {
> this.directory = directory;
> + segmentInfosAge = Long.MAX_VALUE;
> }
>
> Directory directory;
> private Lock writeLock;
> +
> + //used to determine whether index has chaged since reader was
> opened
> + private long segmentInfosAge;
>
> /** Returns an IndexReader reading the index in an FSDirectory in
> the named
> path. */
> @@ -102,15 +106,20 @@
> synchronized (directory) { // in- & inter-process sync
> return (IndexReader)new
> Lock.With(directory.makeLock("commit.lock"),
> IndexWriter.COMMIT_LOCK_TIMEOUT) {
> public Object doBody() throws IOException {
> + IndexReader result = null;
> +
> SegmentInfos infos = new SegmentInfos();
> infos.read(directory);
> if (infos.size() == 1) // index is optimized
> - return new SegmentReader(infos.info(0), true);
> + result = new SegmentReader(infos.info(0), true);
>
> SegmentReader[] readers = new SegmentReader[infos.size()];
> for (int i = 0; i < infos.size(); i++)
> readers[i] = new SegmentReader(infos.info(i),
> i==infos.size()-1);
> - return new SegmentsReader(directory, readers);
> + result = new SegmentsReader(directory, readers);
> +
> + result.segmentInfosAge =
> directory.fileModified("segments");
> + return result;
> }
> }.run();
> }
> @@ -258,6 +267,14 @@
> if (!writeLock.obtain(IndexWriter.WRITE_LOCK_TIMEOUT)) //
> obtain write lock
> throw new IOException("Index locked for write: " +
> writeLock);
> this.writeLock = writeLock;
> +
> + // we have to check whether index has changed since this
> reader was opened.
> + // if so, this reader is no longer valid for deletion
> + if(lastModified(directory) > segmentInfosAge){
> + this.writeLock.release();
> + this.writeLock = null;
> + throw new IOException("IndexReader out of date and no
> longer valid for deletion");
> + }
> }
> doDelete(docNum);
> }
> > /*
> * Created on 11.09.2003
> *
> * To change the template for this generated file go to
> * Window>Preferences>Java>Code Generation>Code and Comments
> */
> package org.apache.lucene.index;
>
> import java.io.IOException;
>
> import org.apache.lucene.analysis.WhitespaceAnalyzer;
> import org.apache.lucene.document.Document;
> import org.apache.lucene.document.Field;
> import org.apache.lucene.search.Hits;
> import org.apache.lucene.search.IndexSearcher;
> import org.apache.lucene.search.Searcher;
> import org.apache.lucene.search.TermQuery;
> import org.apache.lucene.store.Directory;
> import org.apache.lucene.store.RAMDirectory;
>
> import junit.framework.TestCase;
>
> /**
> *
> * @author goller
> */
> public class TestDelete extends TestCase {
>
> public void testDeleteReaderWriterConflict(){
>
> Directory dir = new RAMDirectory();
> IndexWriter writer = null;
> IndexReader reader = null;
> Searcher searcher = null;
> Term searchTerm = new Term("content", "aaa");
> Hits hits = null;
>
> try {
>
> // add 100 documents with term : aaa
> writer = new IndexWriter(dir, new WhitespaceAnalyzer(),
> true);
> for (int i = 0; i < 100; i++) {
> addDoc(writer, "aaa");
> }
> writer.close();
>
> reader = IndexReader.open(dir);
>
> // add 100 documents with term : bbb
> writer = new IndexWriter(dir, new WhitespaceAnalyzer(),
> false);
> for (int i = 0; i < 100; i++) {
> addDoc(writer, "bbb");
> }
> writer.optimize();
> writer.close();
>
> }
> catch (IOException e) {
> e.printStackTrace();
> }
>
> try{
> // delete documents containing term: aaa
> reader.delete(searchTerm);
> reader.close();
> }
> catch (IOException e) {
> try {
> // if reader throws IOException try once more to
> delete documents with a new reader
> reader.close();
> reader = IndexReader.open(dir);
> reader.delete(searchTerm);
> reader.close();
> }
> catch (IOException e1) {
> e1.printStackTrace();
> }
> }
>
> try {
> searcher = new IndexSearcher(dir);
> hits = searcher.search(new TermQuery(searchTerm));
> assertEquals(0, hits.length());
> searcher.close();
> }
> catch (IOException e1) {
> e1.printStackTrace();
> }
>
>
>
> }
>
> public void testDeleteReaderReaderConflict(){
>
> Directory dir = new RAMDirectory();
> IndexWriter writer = null;
> IndexReader reader1 = null;
> IndexReader reader2 = null;
> Searcher searcher = null;
> Hits hits = null;
> Term searchTerm1 = new Term("content", "aaa");
> Term searchTerm2 = new Term("content", "bbb");
>
> try {
>
> // add 100 documents with term : aaa
> // add 100 documents with term : bbb
> // add 100 documents with term : ccc
> writer = new IndexWriter(dir, new WhitespaceAnalyzer(),
> true);
> for (int i = 0; i < 100; i++) {
> addDoc(writer, "aaa");
> addDoc(writer, "bbb");
> addDoc(writer, "ccc");
> }
> writer.optimize();
> writer.close();
>
> }
> catch (IOException e) {
> e.printStackTrace();
> }
>
>
> try{
>
> reader1 = IndexReader.open(dir);
> reader2 = IndexReader.open(dir);
>
> // delete documents containing term: aaa
> reader2.delete(searchTerm1);
> reader2.close();
>
> // delete documents containing term: bbb
> reader1.delete(searchTerm2);
> reader1.close();
> }
> catch (IOException e) {
> try {
> // if reader throws IOException try once more to
> delete documents with a new reader
> reader1.close();
> reader1 = IndexReader.open(dir);
> reader1.delete(searchTerm2);
> reader1.close();
> }
> catch (IOException e1) {
> e1.printStackTrace();
> }
> }
>
> try {
> searcher = new IndexSearcher(dir);
> hits = searcher.search(new TermQuery(searchTerm1));
> assertEquals(0, hits.length());
> hits = searcher.search(new TermQuery(searchTerm2));
> assertEquals(0, hits.length());
> searcher.close();
> }
> catch (IOException e1) {
> e1.printStackTrace();
> }
>
> }
>
>
> private void addDoc(IndexWriter writer, String value)
> {
> Document doc = new Document();
> doc.add(Field.UnStored("content", value));
>
> try {
> writer.addDocument(doc);
> }
> catch (IOException e) {
> e.printStackTrace();
> }
> }
> }
>
> >
---------------------------------------------------------------------
> To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software
http://sitebuilder.yahoo.com