You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucy.apache.org by nw...@apache.org on 2017/04/16 10:28:41 UTC

[03/16] lucy git commit: Always check for stale locks in Is_Locked

Always check for stale locks in Is_Locked

Note that checking for stale locks isn't necessary when requesting
shared locks.


Project: http://git-wip-us.apache.org/repos/asf/lucy/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucy/commit/1a8e02df
Tree: http://git-wip-us.apache.org/repos/asf/lucy/tree/1a8e02df
Diff: http://git-wip-us.apache.org/repos/asf/lucy/diff/1a8e02df

Branch: refs/heads/master
Commit: 1a8e02df0a0257e4ba70de4a41504507b0ba8a47
Parents: bb83d0d
Author: Nick Wellnhofer <we...@aevum.de>
Authored: Thu Feb 16 16:47:03 2017 +0100
Committer: Nick Wellnhofer <we...@aevum.de>
Committed: Mon Feb 20 16:26:21 2017 +0100

----------------------------------------------------------------------
 core/Lucy/Index/BackgroundMerger.c |  2 --
 core/Lucy/Index/FilePurger.c       |  5 ----
 core/Lucy/Index/Indexer.c          |  1 -
 core/Lucy/Index/PolyReader.c       |  2 --
 core/Lucy/Store/Lock.c             | 48 +++++++++------------------------
 core/Lucy/Store/Lock.cfh           | 10 -------
 go/build.go                        |  1 -
 go/lucy/store.go                   |  7 -----
 go/lucy/store_test.go              |  5 ----
 perl/t/106-locking.t               |  1 -
 perl/t/110-shared_lock.t           |  5 ++--
 11 files changed, 14 insertions(+), 73 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/core/Lucy/Index/BackgroundMerger.c
----------------------------------------------------------------------
diff --git a/core/Lucy/Index/BackgroundMerger.c b/core/Lucy/Index/BackgroundMerger.c
index 661b9a6..e9a1ae8 100644
--- a/core/Lucy/Index/BackgroundMerger.c
+++ b/core/Lucy/Index/BackgroundMerger.c
@@ -529,7 +529,6 @@ static void
 S_obtain_write_lock(BackgroundMerger *self) {
     BackgroundMergerIVARS *const ivars = BGMerger_IVARS(self);
     Lock *write_lock = IxManager_Make_Write_Lock(ivars->manager);
-    Lock_Clear_Stale(write_lock);
     if (Lock_Obtain_Exclusive(write_lock)) {
         // Only assign if successful, otherwise DESTROY unlocks -- bad!
         ivars->write_lock = write_lock;
@@ -543,7 +542,6 @@ static void
 S_obtain_merge_lock(BackgroundMerger *self) {
     BackgroundMergerIVARS *const ivars = BGMerger_IVARS(self);
     Lock *merge_lock = IxManager_Make_Merge_Lock(ivars->manager);
-    Lock_Clear_Stale(merge_lock);
     if (Lock_Obtain_Exclusive(merge_lock)) {
         // Only assign if successful, same rationale as above.
         ivars->merge_lock = merge_lock;

http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/core/Lucy/Index/FilePurger.c
----------------------------------------------------------------------
diff --git a/core/Lucy/Index/FilePurger.c b/core/Lucy/Index/FilePurger.c
index 3980c8f..161f030 100644
--- a/core/Lucy/Index/FilePurger.c
+++ b/core/Lucy/Index/FilePurger.c
@@ -78,7 +78,6 @@ FilePurger_Purge_Snapshots_IMP(FilePurger *self, Snapshot *current) {
     Lock *deletion_lock = IxManager_Make_Deletion_Lock(ivars->manager);
 
     // Obtain deletion lock, purge files, release deletion lock.
-    Lock_Clear_Stale(deletion_lock);
     if (Lock_Obtain_Exclusive(deletion_lock)) {
         Folder *folder    = ivars->folder;
         Hash   *failures  = Hash_new(16);
@@ -147,7 +146,6 @@ FilePurger_Purge_Aborted_Merge_IMP(FilePurger *self) {
     IndexManager *manager    = ivars->manager;
     Lock         *merge_lock = IxManager_Make_Merge_Lock(manager);
 
-    Lock_Clear_Stale(merge_lock);
     if (!Lock_Is_Locked_Exclusive(merge_lock)) {
         Hash *merge_data = IxManager_Read_Merge_Data(manager);
         Obj  *cutoff = merge_data
@@ -213,9 +211,6 @@ S_discover_unused(FilePurger *self, Snapshot *current, Hash *spared,
 
             // DON'T obtain the lock -- only see whether another
             // entity holds a lock on the snapshot file.
-            if (lock) {
-                Lock_Clear_Stale(lock);
-            }
             if (lock && Lock_Is_Locked(lock)) {
                 // The snapshot file is locked, which means someone's using
                 // that version of the index -- protect all of its entries.

http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/core/Lucy/Index/Indexer.c
----------------------------------------------------------------------
diff --git a/core/Lucy/Index/Indexer.c b/core/Lucy/Index/Indexer.c
index 97763e3..2381b26 100644
--- a/core/Lucy/Index/Indexer.c
+++ b/core/Lucy/Index/Indexer.c
@@ -97,7 +97,6 @@ Indexer_init(Indexer *self, Schema *schema, Obj *index,
 
     // Get a write lock for this folder.
     Lock *write_lock = IxManager_Make_Write_Lock(ivars->manager);
-    Lock_Clear_Stale(write_lock);
     if (Lock_Obtain_Exclusive(write_lock)) {
         // Only assign if successful, otherwise DESTROY unlocks -- bad!
         ivars->write_lock = write_lock;

http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/core/Lucy/Index/PolyReader.c
----------------------------------------------------------------------
diff --git a/core/Lucy/Index/PolyReader.c b/core/Lucy/Index/PolyReader.c
index d4166c6..ea5e35f 100644
--- a/core/Lucy/Index/PolyReader.c
+++ b/core/Lucy/Index/PolyReader.c
@@ -470,7 +470,6 @@ static bool
 S_obtain_deletion_lock(PolyReader *self) {
     PolyReaderIVARS *const ivars = PolyReader_IVARS(self);
     ivars->deletion_lock = IxManager_Make_Deletion_Lock(ivars->manager);
-    Lock_Clear_Stale(ivars->deletion_lock);
     if (!Lock_Obtain_Exclusive(ivars->deletion_lock)) {
         DECREF(ivars->deletion_lock);
         ivars->deletion_lock = NULL;
@@ -485,7 +484,6 @@ S_obtain_read_lock(PolyReader *self, String *snapshot_file_name) {
     ivars->read_lock = IxManager_Make_Snapshot_Read_Lock(ivars->manager,
                                                          snapshot_file_name);
 
-    Lock_Clear_Stale(ivars->read_lock);
     if (!Lock_Obtain_Shared(ivars->read_lock)) {
         DECREF(ivars->read_lock);
         ivars->read_lock = NULL;

http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/core/Lucy/Store/Lock.c
----------------------------------------------------------------------
diff --git a/core/Lucy/Store/Lock.c b/core/Lucy/Store/Lock.c
index 0eb42da..6d39362 100644
--- a/core/Lucy/Store/Lock.c
+++ b/core/Lucy/Store/Lock.c
@@ -341,7 +341,8 @@ bool
 LFLock_Is_Locked_Exclusive_IMP(LockFileLock *self) {
     LockFileLockIVARS *const ivars = LFLock_IVARS(self);
 
-    return Folder_Exists(ivars->folder, ivars->lock_path);
+    return Folder_Exists(ivars->folder, ivars->lock_path)
+           && !S_maybe_delete_file(ivars, ivars->lock_path, false, true);
 }
 
 bool
@@ -349,7 +350,9 @@ LFLock_Is_Locked_IMP(LockFileLock *self) {
     LockFileLockIVARS *const ivars = LFLock_IVARS(self);
 
     // Check for exclusive lock.
-    if (Folder_Exists(ivars->folder, ivars->lock_path)) {
+    if (Folder_Exists(ivars->folder, ivars->lock_path)
+        && !S_maybe_delete_file(ivars, ivars->lock_path, false, true)
+       ) {
         return true;
     }
 
@@ -360,51 +363,24 @@ LFLock_Is_Locked_IMP(LockFileLock *self) {
         return false;
     }
 
+    bool locked = false;
     DirHandle *dh = Folder_Open_Dir(ivars->folder, lock_dir_name);
     if (!dh) { RETHROW(INCREF(Err_get_error())); }
 
     while (DH_Next(dh)) {
         String *entry = DH_Get_Entry(dh);
         if (S_is_shared_lock_file(ivars, entry)) {
-            DECREF(entry);
-            DECREF(dh);
-            return true;
+            String *candidate = Str_newf("%o/%o", lock_dir_name, entry);
+            if (!S_maybe_delete_file(ivars, candidate, false, true)) {
+                locked = true;
+            }
+            DECREF(candidate);
         }
         DECREF(entry);
     }
 
     DECREF(dh);
-    return false;
-}
-
-void
-LFLock_Clear_Stale_IMP(LockFileLock *self) {
-    LockFileLockIVARS *const ivars = LFLock_IVARS(self);
-
-    if (ivars->shared_lock_path) {
-        String *lock_dir_name = SSTR_WRAP_C("locks");
-        if (!Folder_Find_Folder(ivars->folder, lock_dir_name)) {
-            return;
-        }
-
-        DirHandle *dh = Folder_Open_Dir(ivars->folder, lock_dir_name);
-        if (!dh) { RETHROW(INCREF(Err_get_error())); }
-
-        while (DH_Next(dh)) {
-            String *entry = DH_Get_Entry(dh);
-            if (S_is_shared_lock_file(ivars, entry)) {
-                String *candidate = Str_newf("%o/%o", lock_dir_name, entry);
-                S_maybe_delete_file(ivars, candidate, false, true);
-                DECREF(candidate);
-            }
-            DECREF(entry);
-        }
-
-        DECREF(dh);
-    }
-    else {
-        S_maybe_delete_file(ivars, ivars->lock_path, false, true);
-    }
+    return locked;
 }
 
 static bool

http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/core/Lucy/Store/Lock.cfh
----------------------------------------------------------------------
diff --git a/core/Lucy/Store/Lock.cfh b/core/Lucy/Store/Lock.cfh
index 3f93dc0..598c777 100644
--- a/core/Lucy/Store/Lock.cfh
+++ b/core/Lucy/Store/Lock.cfh
@@ -112,13 +112,6 @@ public abstract class Lucy::Store::Lock inherits Clownfish::Obj {
     public abstract bool
     Is_Locked_Exclusive(Lock *self);
 
-    /** Release all locks that meet the following three conditions: the lock
-     * name matches, the host id matches, and the process id that the lock
-     * was created under no longer identifies an active process.
-     */
-    public abstract void
-    Clear_Stale(Lock *self);
-
     String*
     Get_Name(Lock *self);
 
@@ -159,9 +152,6 @@ class Lucy::Store::LockFileLock nickname LFLock
     public bool
     Is_Locked_Exclusive(LockFileLock *self);
 
-    public void
-    Clear_Stale(LockFileLock *self);
-
     String*
     Get_Lock_Path(LockFileLock *self);
 

http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/go/build.go
----------------------------------------------------------------------
diff --git a/go/build.go b/go/build.go
index 16c31f2..d64cbd2 100644
--- a/go/build.go
+++ b/go/build.go
@@ -458,7 +458,6 @@ func specClasses(parcel *cfc.Parcel) {
 	lockBinding.SpecMethod("Obtain_Shared", "ObtainShared() error")
 	lockBinding.SpecMethod("Obtain_Exclusive", "ObtainExclusive() error")
 	lockBinding.SpecMethod("Release", "Release() error")
-	lockBinding.SpecMethod("Clear_Stale", "ClearStale() error")
 	lockBinding.Register()
 
 	cfWriterBinding := cfc.NewGoClass(parcel, "Lucy::Store::CompoundFileWriter")

http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/go/lucy/store.go
----------------------------------------------------------------------
diff --git a/go/lucy/store.go b/go/lucy/store.go
index 9b9d8c3..7f00a94 100644
--- a/go/lucy/store.go
+++ b/go/lucy/store.go
@@ -766,13 +766,6 @@ func (lock *LockIMP) Release() error {
 }
 
 
-func (lock *LockIMP) ClearStale() error {
-	return clownfish.TrapErr(func() {
-		self := (*C.lucy_Lock)(clownfish.Unwrap(lock, "lock"))
-		C.LUCY_Lock_Clear_Stale(self)
-	})
-}
-
 func OpenCompoundFileReader(folder Folder) (reader CompoundFileReader, err error) {
 	err = clownfish.TrapErr(func() {
 		folderC := (*C.lucy_Folder)(clownfish.Unwrap(folder, "Folder"))

http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/go/lucy/store_test.go
----------------------------------------------------------------------
diff --git a/go/lucy/store_test.go b/go/lucy/store_test.go
index 680b9b2..ec51244 100644
--- a/go/lucy/store_test.go
+++ b/go/lucy/store_test.go
@@ -710,11 +710,6 @@ func TestLockFileLockAll(t *testing.T) {
 		t.Errorf("Obtain: %v", err)
 	}
 	lock.Release()
-
-	err = lock.ClearStale()
-	if err != nil {
-		t.Errorf("Nothing for ClearStale to do, but should still suceed: %v", err)
-	}
 }
 
 func TestLockFactoryAll(t *testing.T) {

http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/perl/t/106-locking.t
----------------------------------------------------------------------
diff --git a/perl/t/106-locking.t b/perl/t/106-locking.t
index 3a1670c..15c6a3a 100644
--- a/perl/t/106-locking.t
+++ b/perl/t/106-locking.t
@@ -50,7 +50,6 @@ Dead_locks_are_removed: {
             exclusive_only => 1,
             @_
         );
-        $lock->clear_stale;
         $lock->obtain_exclusive() or die "no dice";
         return $lock;
     }

http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/perl/t/110-shared_lock.t
----------------------------------------------------------------------
diff --git a/perl/t/110-shared_lock.t b/perl/t/110-shared_lock.t
index 5281c9f..77c6862 100644
--- a/perl/t/110-shared_lock.t
+++ b/perl/t/110-shared_lock.t
@@ -74,9 +74,8 @@ $lock->release;
 $another_lock->release;
 $ya_lock->release;
 
-ok( $lock->is_locked(), "failed to release a lock with a different pid" );
-$lock->clear_stale;
-ok( !$lock->is_locked(), "clear_stale" );
+ok( $lock->get_lock_path, "failed to release a lock with a different pid" );
+ok( !$lock->is_locked(), "is_locked clears stale locks" );
 
 ok( $lock->obtain_shared(), "got lock again" );
 ok( $lock->is_locked(), "it's locked" );