diff --git a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/ExternalizerIds.java b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/ExternalizerIds.java index 6073f77ef838..b601d4d0b56c 100644 --- a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/ExternalizerIds.java +++ b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/ExternalizerIds.java @@ -57,4 +57,8 @@ public interface ExternalizerIds { */ static final Integer FILE_LIST_DELTA_DEL = 1308; + /** + * @see org.infinispan.lucene.FileDeleteMarkerKey.Externalizer + */ + static final Integer FILE_DELETE_MARKER_KEY = 1309; } diff --git a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/FileDeleteMarkerKey.java b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/FileDeleteMarkerKey.java new file mode 100644 index 000000000000..d725ee398ea6 --- /dev/null +++ b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/FileDeleteMarkerKey.java @@ -0,0 +1,111 @@ +package org.infinispan.lucene; + +import java.io.IOException; +import java.io.ObjectInput; +import java.io.ObjectOutput; +import java.util.Set; + +import org.infinispan.commons.marshall.AbstractExternalizer; +import org.infinispan.commons.util.Util; + +/** + * Lucene's index segment files are chunked, for safe deletion of elements a read lock is + * implemented so that all chunks are deleted only after the usage counter is decremented to zero. + * FileDeleteMarkerKey is used as a key for the reference counters; a special purpose key was needed to + * make atomic operation possible. + */ +public class FileDeleteMarkerKey implements IndexScopedKey { + + private final String indexName; + private final String fileName; + private final int hashCode; + + public FileDeleteMarkerKey(final String indexName, final String fileName) { + if (indexName == null) + throw new IllegalArgumentException("indexName shall not be null"); + if (fileName == null) + throw new IllegalArgumentException("fileName shall not be null"); + this.indexName = indexName; + this.fileName = fileName; + this.hashCode = generateHashCode(); + } + + /** + * Get the indexName. + * + * @return the indexName. + */ + @Override + public String getIndexName() { + return indexName; + } + + /** + * Get the fileName. + * + * @return the fileName. + */ + public String getFileName() { + return fileName; + } + + @Override + public Object accept(KeyVisitor visitor) throws Exception { + return visitor.visit(this); + } + + @Override + public int hashCode() { + return hashCode; + } + + private int generateHashCode() { + final int prime = 31; + int result = prime + fileName.hashCode(); + return prime * result + indexName.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (FileDeleteMarkerKey.class != obj.getClass()) + return false; + FileDeleteMarkerKey other = (FileDeleteMarkerKey) obj; + return fileName.equals(other.fileName) && indexName.equals(other.indexName); + } + + @Override + public String toString() { + return fileName + "|D|"+ indexName; + } + + public static final class Externalizer extends AbstractExternalizer { + + @Override + public void writeObject(final ObjectOutput output, final FileDeleteMarkerKey key) throws IOException { + output.writeUTF(key.indexName); + output.writeUTF(key.fileName); + } + + @Override + public FileDeleteMarkerKey readObject(final ObjectInput input) throws IOException { + String indexName = input.readUTF(); + String fileName = input.readUTF(); + return new FileDeleteMarkerKey(indexName, fileName); + } + + @Override + public Integer getId() { + return ExternalizerIds.FILE_DELETE_MARKER_KEY; + } + + @Override + public Set> getTypeClasses() { + return Util.>asSet(FileDeleteMarkerKey.class); + } + + } +} diff --git a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/KeyVisitor.java b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/KeyVisitor.java index 5ad0d523ccf5..515e64a86a0c 100644 --- a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/KeyVisitor.java +++ b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/KeyVisitor.java @@ -18,4 +18,6 @@ public interface KeyVisitor { T visit(FileReadLockKey fileReadLockKey) throws Exception; + T visit(FileDeleteMarkerKey fileDeleteMarkerKey) throws Exception; + } diff --git a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/cacheloader/DirectoryLoaderAdaptor.java b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/cacheloader/DirectoryLoaderAdaptor.java index 62230ea81c59..681906c1b11b 100644 --- a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/cacheloader/DirectoryLoaderAdaptor.java +++ b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/cacheloader/DirectoryLoaderAdaptor.java @@ -10,10 +10,9 @@ import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexInput; import org.infinispan.commons.marshall.StreamingMarshaller; -import org.infinispan.marshall.core.MarshalledEntryImpl; -import org.infinispan.marshall.core.MarshalledEntry; import org.infinispan.lucene.ChunkCacheKey; import org.infinispan.lucene.FileCacheKey; +import org.infinispan.lucene.FileDeleteMarkerKey; import org.infinispan.lucene.FileListCacheKey; import org.infinispan.lucene.FileMetadata; import org.infinispan.lucene.FileReadLockKey; @@ -21,6 +20,8 @@ import org.infinispan.lucene.KeyVisitor; import org.infinispan.lucene.impl.FileListCacheValue; import org.infinispan.lucene.logging.Log; +import org.infinispan.marshall.core.MarshalledEntry; +import org.infinispan.marshall.core.MarshalledEntryImpl; import org.infinispan.util.logging.LogFactory; /** @@ -294,6 +295,12 @@ public Object visit(final FileReadLockKey fileReadLockKey) { //ReadLocks should not leak to the actual storage return null; } + + @Override + public Object visit(FileDeleteMarkerKey fileDeleteMarkerKey) throws Exception { + //Delete markers should not leak to the actual storage + return null; + } } /** @@ -323,6 +330,12 @@ public Boolean visit(final FileReadLockKey fileReadLockKey) { //ReadLocks should not leak to the actual storage return Boolean.FALSE; } + + @Override + public Boolean visit(FileDeleteMarkerKey fileDeleteMarkerKey) throws Exception { + //Delete markers should not leak to the actual storage + return Boolean.FALSE; + } } } diff --git a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/impl/DirectoryImplementor.java b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/impl/DirectoryImplementor.java index 12565fb6f6ee..0632032da083 100644 --- a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/impl/DirectoryImplementor.java +++ b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/impl/DirectoryImplementor.java @@ -62,7 +62,7 @@ boolean fileExists(final String name) { void deleteFile(final String name) { fileOps.deleteFileName(name); - readLocks.deleteOrReleaseReadLock(name); + readLocks.markForDeletion(name); if (log.isDebugEnabled()) { log.debugf("Removed file: %s from index: %s", name, indexName); } @@ -91,7 +91,7 @@ void renameFile(final String from, final String to) { fileOps.removeAndAdd(from, to); // now trigger deletion of old file chunks: - readLocks.deleteOrReleaseReadLock(from); + readLocks.markForDeletion(from); if (log.isTraceEnabled()) { log.tracef("Renamed file from: %s to: %s in index %s", from, to, indexName); } diff --git a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/impl/InfinispanIndexInput.java b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/impl/InfinispanIndexInput.java index 109117e41090..d62e61f394cc 100644 --- a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/impl/InfinispanIndexInput.java +++ b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/impl/InfinispanIndexInput.java @@ -97,7 +97,7 @@ public void close() { currentLoadedChunk = -1; buffer = null; if (isClone) return; - readLocks.deleteOrReleaseReadLock(filename); + readLocks.releaseReadLock(filename); if (trace) { log.tracef("Closed IndexInput for file:%s in index: %s", filename, fileKey.getIndexName()); } diff --git a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/DistributedSegmentReadLocker.java b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/DistributedSegmentReadLocker.java index c744862f5b65..04eb7fd269ab 100644 --- a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/DistributedSegmentReadLocker.java +++ b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/DistributedSegmentReadLocker.java @@ -5,6 +5,7 @@ import org.infinispan.context.Flag; import org.infinispan.lucene.ChunkCacheKey; import org.infinispan.lucene.FileCacheKey; +import org.infinispan.lucene.FileDeleteMarkerKey; import org.infinispan.lucene.FileMetadata; import org.infinispan.lucene.FileReadLockKey; import org.infinispan.util.logging.Log; @@ -67,7 +68,7 @@ public DistributedSegmentReadLocker(Cache cache, String indexName) { * @see org.apache.lucene.store.Directory#deleteFile(String) */ @Override - public void deleteOrReleaseReadLock(final String filename) { + public void releaseReadLock(final String filename) { if (isMultiChunked(filename)) { int newValue = 0; FileReadLockKey readLockKey = new FileReadLockKey(indexName, filename); @@ -173,7 +174,7 @@ public boolean acquireReadLock(String filename) { * The {@link org.apache.lucene.store.Directory#deleteFile(String)} is not deleting the elements from the cache * but instead flagging the file as deletable. * This method will really remove the elements from the cache; should be invoked only - * by {@link #deleteOrReleaseReadLock(String)} after having verified that there + * by {@link #releaseReadLock(String)} after having verified that there * are no users left in need to read these chunks. * * @param indexName the index name to delete the file from @@ -188,6 +189,12 @@ static void realFileDelete(String indexName, String fileName, AdvancedCache cache) { throw new IllegalArgumentException("DistributedSegmentReadLocker is not reliable when using a cache with eviction enabled, disable eviction on this cache instance"); } + + /** + * Adds a value to the locks cache to indicate that the file is in need of deleting. + * An actual file delete may not occur until all read locks are released. + * @see InfinispanDirectory#deleteFile(String) + */ + @Override + public void markForDeletion(String fileName) { + FileDeleteMarkerKey key = new FileDeleteMarkerKey(indexName, fileName); + if (log.isTraceEnabled()) log.tracef("Marking for deletion: %s", key); + + locksCache.put(key, 1); + releaseReadLock(fileName); + } } diff --git a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/LocalLockMergingSegmentReadLocker.java b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/LocalLockMergingSegmentReadLocker.java index 455d25d462c2..94cca45fbd5f 100644 --- a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/LocalLockMergingSegmentReadLocker.java +++ b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/LocalLockMergingSegmentReadLocker.java @@ -1,8 +1,9 @@ package org.infinispan.lucene.readlocks; -import org.infinispan.Cache; import java.util.HashMap; +import org.infinispan.Cache; + /** * LocalLockMergingSegmentReadLocker decorates the {@link DistributedSegmentReadLocker} to minimize * remote operations in case several IndexReaders are opened on the same Infinispan based {@link org.apache.lucene.store.Directory}. @@ -73,7 +74,7 @@ private LocalReadLock getLocalLockByName(String name) { * {@inheritDoc} */ @Override - public synchronized void deleteOrReleaseReadLock(String name) { + public synchronized void releaseReadLock(String name) { getLocalLockByName(name).release(); } @@ -112,9 +113,14 @@ void release() { value--; if (value <= 0) { localLocks.remove(name); - delegate.deleteOrReleaseReadLock(name); + delegate.releaseReadLock(name); } } } + + @Override + public void markForDeletion(String fileName) { + delegate.markForDeletion(fileName); + } } diff --git a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/NoopSegmentReadLocker.java b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/NoopSegmentReadLocker.java index a38382151425..239dac7958c5 100644 --- a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/NoopSegmentReadLocker.java +++ b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/NoopSegmentReadLocker.java @@ -22,8 +22,15 @@ public boolean acquireReadLock(String filename) { * doesn't do anything */ @Override - public void deleteOrReleaseReadLock(String filename) { + public void releaseReadLock(String filename) { return; } + /** + * doesn't do anything + */ + @Override + public void markForDeletion(String fileName) { + return; + } } diff --git a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/SegmentReadLocker.java b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/SegmentReadLocker.java index 6c5e914f18a0..abd8081426c3 100644 --- a/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/SegmentReadLocker.java +++ b/lucene/lucene-directory/src/main/java/org/infinispan/lucene/readlocks/SegmentReadLocker.java @@ -1,13 +1,15 @@ package org.infinispan.lucene.readlocks; + + /** *

SegmentReadLocker implementations have to make sure that segments are not deleted while they are * being used by an IndexReader.

- *

When an {@link org.infinispan.lucene.impl.InfinispanIndexInput} is opened on a file which is split in smaller chunks, - * {@link #acquireReadLock(String)} is invoked; then the {@link #deleteOrReleaseReadLock(String)} is + *

When an {@link org.infinispan.lucene.InfinispanIndexInput} is opened on a file which is split in smaller chunks, + * {@link #acquireReadLock(String)} is invoked; then the {@link #releaseReadLock(String)} is * invoked when the stream is closed.

- *

The same {@link #deleteOrReleaseReadLock(String)} is invoked when a file is deleted, so if this invocation is not balancing - * a lock acquire this implementation must delete all segment chunks and the associated metadata.

+ *

{@link #markForDeletion(String)} is invoked when a file is deleted, so {@link #releaseReadLock(String)} + * is invoked for the last time this implementation must delete all segment chunks and the associated metadata.

*

Note that if you can use and tune the {@link org.apache.lucene.index.LogByteSizeMergePolicy} you could avoid the need * for readlocks by setting a maximum segment size to equal the chunk size used by the InfinispanDirectory; readlocks * will be skipped automatically when not needed, so it's advisable to still configure an appropriate SegmentReadLocker @@ -18,25 +20,31 @@ */ public interface SegmentReadLocker { + /** + * Set file to be deleted when all acquired read locks have been released. The file will be deleted immediately + * if the file is not locked. + * @param fileName of the file to mark for deletion + * @see InfinispanDirectory#deleteFile(String) + */ + void markForDeletion(String fileName); + /** - * It will release a previously acquired readLock, or - * if no readLock was acquired it will mark the file to be deleted as soon - * as all pending locks are releases. - * If it's invoked on a file without pending locks the file is deleted. + * It will release a previously acquired readLock. + * If it's invoked on a file without pending locks and the file is marked for deletion it will be deleted. * * @param fileName of the file to release or delete - * @see org.apache.lucene.store.Directory#deleteFile(String) + * @see markForDeletion() */ - void deleteOrReleaseReadLock(String fileName); + void releaseReadLock(String fileName); /** - * Acquires a readlock, in order to prevent other invocations to {@link #deleteOrReleaseReadLock(String)} + * Acquires a readlock, in order to prevent other invocations to {@link #releaseReadLock(String)} * from deleting the file. * - * @param filename of the file to acquire the lock at + * @param filename * @return true if the lock was acquired, false if the implementation * detects the file does not exist, or that it's being deleted by some other thread. - * @see org.apache.lucene.store.Directory#openInput + * @see InfinispanDirectory#openInput(String) */ boolean acquireReadLock(String filename); diff --git a/lucene/lucene-directory/src/test/java/org/infinispan/lucene/readlocks/LocalLockStressTest.java b/lucene/lucene-directory/src/test/java/org/infinispan/lucene/readlocks/LocalLockStressTest.java index 024650013786..a335689a6da6 100644 --- a/lucene/lucene-directory/src/test/java/org/infinispan/lucene/readlocks/LocalLockStressTest.java +++ b/lucene/lucene-directory/src/test/java/org/infinispan/lucene/readlocks/LocalLockStressTest.java @@ -13,8 +13,8 @@ import org.infinispan.manager.EmbeddedCacheManager; import org.infinispan.test.SingleCacheManagerTest; import org.infinispan.test.fwk.TestCacheManagerFactory; -import org.testng.annotations.Test; import org.testng.AssertJUnit; +import org.testng.annotations.Test; /** * Stress test for {@link org.infinispan.lucene.readlocks.LocalLockMergingSegmentReadLocker}. See also ISPN-4497 for an @@ -57,7 +57,7 @@ public void run() { locker.acquireReadLock("fileName"); Thread.sleep(2); - locker.deleteOrReleaseReadLock("fileName"); + locker.releaseReadLock("fileName"); // Take a break every now and a again to try and avoid the same LocalReadLock being used constantly if (counter++ % 900 == 0) {