Thanks, Doug. Unfortunately, as you know, IndexWriter isn't the only one using these locks:

1) IndexReader uses the "commit.lock" when opening.

2) IndexReader needs to create a "write.lock" during the delete process.

So, although perhaps not a perfect solution, it seems to me that Lock is still at least a common place to define these and less confusing than putting them on IndexWriter -- which is only one of the classes that use then contants.

Scott

Doug Cutting wrote:

Overall, this sounds good to me.

The main change I'd propose is moving the timeout constants from Lock.java to IndexWriter.java. These timeouts are specific to indexes, not directories, so they belong with the index code.

Other than that, this looks great. +1.

Doug


Scott Ganyo wrote:


As suggested in previous emails, I have implemented the ability to wait on any kind of lock. I've attached the diffs, but here are the highlights:

1) added the following statics to Lock:

   public static long WRITE_LOCK_TIMEOUT = 1000;
   public static long COMMIT_LOCK_TIMEOUT = 10000;
   public static long LOCK_POLL_INTERVAL = 1000;

2) added the following method to Lock:

/** Attempt to obtain an exclusive lock within amount
* of time given. Currently polls once per second until
* lockWaitTimeout is passed.
* @param lockWaitTimeout length of time to wait in ms
* @return true if lock was obtained
* @throws IOException if lock wait times out or obtain() throws an IOException
*/
public boolean obtain(long lockWaitTimeout) throws IOException


3) added the following method to Lock.With:

   /** Constructs an executor that will grab the named lock. */
   public With(Lock lock, long lockWaitTimeout)

4) cleaned up Lock.With to use the new obtain(long) method on Lock.

5) changed callers obtaining "write.lock" to pass Lock.WRITE_LOCK_TIMEOUT

6) changed callers obtaining "commit.lock" to pass Lock.COMMIT_LOCK_TIMEOUT

The net effect is to expose the timeout and change the write.lock to have a 1 second timeout as a default instead of immediately throwing an IOException.

So that there can be a review and comment period, I won't plan to check it in Wednesday of next week.

Scott


------------------------------------------------------------------------


? lock.diff
? lib/JavaCC.zip
? src/java/org/apache/lucene/analysis/.nbattrs
? src/java/org/apache/lucene/search/.nbattrs
Index: src/java/org/apache/lucene/index/IndexReader.java
===================================================================
RCS file: /home/cvs/jakarta-lucene/src/java/org/apache/lucene/index/IndexReader.java,v


retrieving revision 1.16
diff -u -b -B -r1.16 IndexReader.java
--- src/java/org/apache/lucene/index/IndexReader.java 1 May 2003 19:50:17 -0000 1.16
+++ src/java/org/apache/lucene/index/IndexReader.java 8 Aug 2003 19:53:03 -0000
@@ -100,7 +100,7 @@
/** Returns an IndexReader reading the index in the given Directory. */
public static IndexReader open(final Directory directory) throws IOException{
synchronized (directory) { // in- & inter-process sync
- return (IndexReader)new Lock.With(directory.makeLock("commit.lock")) {
+ return (IndexReader)new Lock.With(directory.makeLock("commit.lock"), Lock.COMMIT_LOCK_TIMEOUT) {
public Object doBody() throws IOException {
SegmentInfos infos = new SegmentInfos();
infos.read(directory);
@@ -255,7 +255,7 @@
public final synchronized void delete(int docNum) throws IOException {
if (writeLock == null) {
Lock writeLock = directory.makeLock("write.lock");
- if (!writeLock.obtain()) // obtain write lock
+ if (!writeLock.obtain(Lock.WRITE_LOCK_TIMEOUT)) // obtain write lock
throw new IOException("Index locked for write: " + writeLock);
this.writeLock = writeLock;
}
Index: src/java/org/apache/lucene/index/IndexWriter.java
===================================================================
RCS file: /home/cvs/jakarta-lucene/src/java/org/apache/lucene/index/IndexWriter.java,v


retrieving revision 1.13
diff -u -b -B -r1.13 IndexWriter.java
--- src/java/org/apache/lucene/index/IndexWriter.java 11 Jul 2003 22:13:13 -0000 1.13
+++ src/java/org/apache/lucene/index/IndexWriter.java 8 Aug 2003 19:53:03 -0000
@@ -141,12 +141,12 @@
analyzer = a;
Lock writeLock = directory.makeLock("write.lock");
- if (!writeLock.obtain()) // obtain write lock
+ if (!writeLock.obtain(Lock.WRITE_LOCK_TIMEOUT)) // obtain write lock
throw new IOException("Index locked for write: " + writeLock);
this.writeLock = writeLock; // save it
synchronized (directory) { // in- & inter-process sync
- new Lock.With(directory.makeLock("commit.lock")) {
+ new Lock.With(directory.makeLock("commit.lock"), Lock.COMMIT_LOCK_TIMEOUT) {
public Object doBody() throws IOException {
if (create)
segmentInfos.write(directory);
@@ -365,7 +365,7 @@
directory));
synchronized (directory) { // in- & inter-process sync
- new Lock.With(directory.makeLock("commit.lock")) {
+ new Lock.With(directory.makeLock("commit.lock"), Lock.COMMIT_LOCK_TIMEOUT) {
public Object doBody() throws IOException {
segmentInfos.write(directory); // commit before deleting
deleteSegments(segmentsToDelete); // delete now-unused segments
Index: src/java/org/apache/lucene/index/SegmentReader.java
===================================================================
RCS file: /home/cvs/jakarta-lucene/src/java/org/apache/lucene/index/SegmentReader.java,v


retrieving revision 1.8
diff -u -b -B -r1.8 SegmentReader.java
--- src/java/org/apache/lucene/index/SegmentReader.java 1 May 2003 01:09:15 -0000 1.8
+++ src/java/org/apache/lucene/index/SegmentReader.java 8 Aug 2003 19:53:03 -0000
@@ -119,7 +119,7 @@
final synchronized void doClose() throws IOException {
if (deletedDocsDirty) {
synchronized (directory) { // in- & inter-process sync
- new Lock.With(directory.makeLock("commit.lock")) {
+ new Lock.With(directory.makeLock("commit.lock"), Lock.COMMIT_LOCK_TIMEOUT) {
public Object doBody() throws IOException {
deletedDocs.write(directory, segment + ".tmp");
directory.renameFile(segment + ".tmp", segment + ".del");
Index: src/java/org/apache/lucene/store/Lock.java
===================================================================
RCS file: /home/cvs/jakarta-lucene/src/java/org/apache/lucene/store/Lock.java,v
retrieving revision 1.3
diff -u -b -B -r1.3 Lock.java
--- src/java/org/apache/lucene/store/Lock.java 1 May 2003 19:50:17 -0000 1.3
+++ src/java/org/apache/lucene/store/Lock.java 8 Aug 2003 19:53:03 -0000
@@ -70,12 +70,41 @@
*/
public abstract class Lock {
- /** Attempt to obtain exclusive access.
- *
+ public static long WRITE_LOCK_TIMEOUT = 1000;
+ public static long COMMIT_LOCK_TIMEOUT = 10000;
+ public static long LOCK_POLL_INTERVAL = 1000;
+
+ /** Attempt to obtain exclusive access and immediately return
+ * upon success or failure.
* @return true iff exclusive access is obtained
*/
public abstract boolean obtain() throws IOException;
+ /** Attempt to obtain an exclusive lock within amount
+ * of time given. Currently polls once per second until
+ * lockWaitTimeout is passed.
+ * @param lockWaitTimeout length of time to wait in ms
+ * @return true if lock was obtained
+ * @throws IOException if lock wait times out or obtain() throws an IOException
+ */
+ public boolean obtain(long lockWaitTimeout) throws IOException {
+ boolean locked = obtain();
+ int maxSleepCount = (int)(lockWaitTimeout / LOCK_POLL_INTERVAL);
+ int sleepCount = 0;
+ while (!locked) {
+ if (++sleepCount == maxSleepCount) {
+ throw new IOException("Lock obtain timed out");
+ }
+ try {
+ Thread.sleep(LOCK_POLL_INTERVAL);
+ } catch (InterruptedException e) {
+ throw new IOException(e.toString());
+ }
+ locked = obtain();
+ }
+ return locked;
+ }
+
/** Release exclusive access. */
public abstract void release();
@@ -87,12 +116,21 @@
/** Utility class for executing code with exclusive access. */
public abstract static class With {
private Lock lock;
- private int sleepInterval = 1000;
- private int maxSleeps = 10;
+ private long lockWaitTimeout;
+
+ /** Constructs an executor that will grab the named lock.
+ * Defaults lockWaitTimeout to Lock.COMMIT_LOCK_TIMEOUT.
+ * @deprecated Kept only to avoid breaking existing code.
+ */
+ public With(Lock lock)
+ {
+ this(lock, COMMIT_LOCK_TIMEOUT);
+ }
/** Constructs an executor that will grab the named lock. */
- public With(Lock lock) {
+ public With(Lock lock, long lockWaitTimeout) {
this.lock = lock;
+ this.lockWaitTimeout = lockWaitTimeout;
}
/** Code to execute with exclusive access. */
@@ -100,26 +138,13 @@
/** Calls [EMAIL PROTECTED] #doBody} while <i>lock</i> is obtained. Blocks if lock
* cannot be obtained immediately. Retries to obtain lock once per second
- * until it is obtained, or until it has tried ten times. */
+ * until it is obtained, or until it has tried ten times. Lock is released when
+ * [EMAIL PROTECTED] #doBody} exits. */
public Object run() throws IOException {
boolean locked = false;
try {
- locked = lock.obtain();
- int sleepCount = 0;
- while (!locked) {
- if (++sleepCount == maxSleeps) {
- throw new IOException("Timed out waiting for: " + lock);
- }
- try {
- Thread.sleep(sleepInterval);
- } catch (InterruptedException e) {
- throw new IOException(e.toString());
- }
- locked = lock.obtain();
- }
-
+ locked = lock.obtain(lockWaitTimeout);
return doBody();
- } finally {
if (locked)
lock.release();




------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]


--
All progress is initiated by challenging current conceptions, and executed by 
supplanting existing institutions. - George Bernard Shaw



---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to