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 <[EMAIL PROTECTED]> 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: [EMAIL PROTECTED] *
> *****************************************************************
> > 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: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software
http://sitebuilder.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
