You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sh...@apache.org on 2012/06/19 14:30:39 UTC
svn commit: r1351672 - in /lucene/dev/trunk/lucene/facet/src:
java/org/apache/lucene/facet/taxonomy/directory/
java/org/apache/lucene/facet/taxonomy/writercache/lru/
test/org/apache/lucene/facet/taxonomy/directory/
Author: shaie
Date: Tue Jun 19 12:30:38 2012
New Revision: 1351672
URL: http://svn.apache.org/viewvc?rev=1351672&view=rev
Log:
LUCENE-4061: fix another concurrency issue with DirTaxoWriter
Modified:
lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java
lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/writercache/lru/NameIntCacheLRU.java
lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyWriter.java
Modified: lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java?rev=1351672&r1=1351671&r2=1351672&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java (original)
+++ lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java Tue Jun 19 12:30:38 2012
@@ -300,7 +300,8 @@ public class DirectoryTaxonomyWriter imp
// verify that the taxo-writer hasn't been closed on us.
ensureOpen();
if (readerManager == null) {
- readerManager = new ReaderManager(indexWriter, false);
+ readerManager = new ReaderManager(indexWriter, false);
+ shouldRefreshReaderManager = false;
}
}
}
@@ -400,16 +401,19 @@ public class DirectoryTaxonomyWriter imp
return res;
}
+ // if we get here, it means the category is not in the cache, and it is not
+ // complete, and therefore we must look for the category on disk.
+
// We need to get an answer from the on-disk index.
initReaderManager();
int doc = -1;
DirectoryReader reader = readerManager.acquire();
try {
+ final BytesRef catTerm = new BytesRef(categoryPath.toString(delimiter));
int base = 0;
for (AtomicReader r : reader.getSequentialSubReaders()) {
- DocsEnum docs = r.termDocsEnum(null, Consts.FULL,
- new BytesRef(categoryPath.toString(delimiter)), false);
+ DocsEnum docs = r.termDocsEnum(null, Consts.FULL, catTerm, false);
if (docs != null) {
doc = docs.nextDoc() + base;
break;
@@ -449,10 +453,10 @@ public class DirectoryTaxonomyWriter imp
int doc = -1;
DirectoryReader reader = readerManager.acquire();
try {
+ final BytesRef catTerm = new BytesRef(categoryPath.toString(delimiter, prefixLen));
int base = 0;
for (AtomicReader r : reader.getSequentialSubReaders()) {
- DocsEnum docs = r.termDocsEnum(null, Consts.FULL,
- new BytesRef(categoryPath.toString(delimiter, prefixLen)), false);
+ DocsEnum docs = r.termDocsEnum(null, Consts.FULL, catTerm, false);
if (docs != null) {
doc = docs.nextDoc() + base;
break;
@@ -498,7 +502,7 @@ public class DirectoryTaxonomyWriter imp
/**
* Add a new category into the index (and the cache), and return its new
* ordinal.
- * <P>
+ * <p>
* Actually, we might also need to add some of the category's ancestors
* before we can add the category itself (while keeping the invariant that a
* parent is always added to the taxonomy before its child). We do this by
@@ -565,11 +569,11 @@ public class DirectoryTaxonomyWriter imp
indexWriter.addDocument(d);
int id = nextID++;
- addToCache(categoryPath, length, id);
-
// added a category document, mark that ReaderManager is not up-to-date
shouldRefreshReaderManager = true;
+ addToCache(categoryPath, length, id);
+
// also add to the parent array
getParentArray().add(id, parent);
Modified: lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/writercache/lru/NameIntCacheLRU.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/writercache/lru/NameIntCacheLRU.java?rev=1351672&r1=1351671&r2=1351672&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/writercache/lru/NameIntCacheLRU.java (original)
+++ lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/writercache/lru/NameIntCacheLRU.java Tue Jun 19 12:30:38 2012
@@ -4,7 +4,6 @@ import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import org.apache.lucene.facet.taxonomy.CategoryPath;
-import org.apache.lucene.facet.taxonomy.directory.DirectoryTaxonomyWriter; // javadocs
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
@@ -115,14 +114,13 @@ class NameIntCacheLRU {
String stats() {
return "#miss="+nMisses+" #hit="+nHits;
}
-
+
/**
- * If cache is full remove least recently used entries from cache.
- * Return true if anything was removed, false otherwise.
+ * If cache is full remove least recently used entries from cache. Return true
+ * if anything was removed, false otherwise.
*
- * See comment in {@link DirectoryTaxonomyWriter#addToCache(CategoryPath, int)}
- * for an explanation why we clean 2/3rds of the cache, and not just one
- * entry.
+ * See comment in DirectoryTaxonomyWriter.addToCache(CategoryPath, int) for an
+ * explanation why we clean 2/3rds of the cache, and not just one entry.
*/
boolean makeRoomLRU() {
if (!isCacheFull()) {
Modified: lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyWriter.java?rev=1351672&r1=1351671&r2=1351672&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyWriter.java (original)
+++ lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyWriter.java Tue Jun 19 12:30:38 2012
@@ -44,16 +44,14 @@ public class TestDirectoryTaxonomyWriter
// A No-Op TaxonomyWriterCache which always discards all given categories, and
// always returns true in put(), to indicate some cache entries were cleared.
- private static class NoOpCache implements TaxonomyWriterCache {
-
- NoOpCache() { }
+ private static TaxonomyWriterCache NO_OP_CACHE = new TaxonomyWriterCache() {
@Override
public void close() {}
@Override
public int get(CategoryPath categoryPath) { return -1; }
@Override
- public int get(CategoryPath categoryPath, int length) { return get(categoryPath); }
+ public int get(CategoryPath categoryPath, int length) { return -1; }
@Override
public boolean put(CategoryPath categoryPath, int ordinal) { return true; }
@Override
@@ -63,14 +61,14 @@ public class TestDirectoryTaxonomyWriter
@Override
public void clear() {}
- }
+ };
@Test
public void testCommit() throws Exception {
// Verifies that nothing is committed to the underlying Directory, if
// commit() wasn't called.
Directory dir = newDirectory();
- DirectoryTaxonomyWriter ltw = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, new NoOpCache());
+ DirectoryTaxonomyWriter ltw = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, NO_OP_CACHE);
assertFalse(DirectoryReader.indexExists(dir));
ltw.commit(); // first commit, so that an index will be created
ltw.addCategory(new CategoryPath("a"));
@@ -86,7 +84,7 @@ public class TestDirectoryTaxonomyWriter
public void testCommitUserData() throws Exception {
// Verifies taxonomy commit data
Directory dir = newDirectory();
- DirectoryTaxonomyWriter taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, new NoOpCache());
+ DirectoryTaxonomyWriter taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, NO_OP_CACHE);
taxoWriter.addCategory(new CategoryPath("a"));
taxoWriter.addCategory(new CategoryPath("b"));
Map <String, String> userCommitData = new HashMap<String, String>();
@@ -104,7 +102,7 @@ public class TestDirectoryTaxonomyWriter
// open DirTaxoWriter again and commit, INDEX_CREATE_TIME should still exist
// in the commit data, otherwise DirTaxoReader.refresh() might not detect
// that the taxonomy index has been recreated.
- taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, new NoOpCache());
+ taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, NO_OP_CACHE);
taxoWriter.addCategory(new CategoryPath("c")); // add a category so that commit will happen
taxoWriter.commit(new HashMap<String, String>(){{
put("just", "data");
@@ -164,7 +162,7 @@ public class TestDirectoryTaxonomyWriter
// DirTaxoReader succeeding to refresh().
Directory dir = newDirectory();
- DirectoryTaxonomyWriter taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, new NoOpCache());
+ DirectoryTaxonomyWriter taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, NO_OP_CACHE);
touchTaxo(taxoWriter, new CategoryPath("a"));
DirectoryTaxonomyReader taxoReader = new DirectoryTaxonomyReader(dir);
@@ -176,11 +174,11 @@ public class TestDirectoryTaxonomyWriter
// now recreate the taxonomy, and check that the timestamp is preserved after opening DirTW again.
taxoWriter.close();
- taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE, new NoOpCache());
+ taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE, NO_OP_CACHE);
touchTaxo(taxoWriter, new CategoryPath("c"));
taxoWriter.close();
- taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, new NoOpCache());
+ taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, NO_OP_CACHE);
touchTaxo(taxoWriter, new CategoryPath("d"));
taxoWriter.close();
@@ -205,7 +203,7 @@ public class TestDirectoryTaxonomyWriter
// create an empty index first, so that DirTaxoWriter initializes createTime to null.
new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, null)).close();
- DirectoryTaxonomyWriter taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, new NoOpCache());
+ DirectoryTaxonomyWriter taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, NO_OP_CACHE);
// we cannot commit null keys/values, this ensures that if DirTW.createTime is null, we can still commit.
taxoWriter.close();
@@ -230,7 +228,7 @@ public class TestDirectoryTaxonomyWriter
} else if (TEST_NIGHTLY && d > 0.98) {
// this is the slowest, but tests the writer concurrency when no caching is done.
// only pick it during NIGHTLY tests, and even then, with very low chances.
- cache = new NoOpCache();
+ cache = NO_OP_CACHE;
} else {
// this is slower than CL2O, but less memory consuming, and exercises finding categories on disk too.
cache = new LruTaxonomyWriterCache(ncats / 10);
@@ -307,4 +305,18 @@ public class TestDirectoryTaxonomyWriter
input.close();
}
+ @Test
+ public void testReaderFreshness() throws Exception {
+ // ensures that the internal index reader is always kept fresh. Previously,
+ // this simple scenario failed, if the cache just evicted the category that
+ // is being added.
+ Directory dir = newDirectory();
+ DirectoryTaxonomyWriter taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE, NO_OP_CACHE);
+ int o1 = taxoWriter.addCategory(new CategoryPath("a"));
+ int o2 = taxoWriter.addCategory(new CategoryPath("a"));
+ assertTrue("ordinal for same category that is added twice should be the same !", o1 == o2);
+ taxoWriter.close();
+ dir.close();
+ }
+
}