-
Notifications
You must be signed in to change notification settings - Fork 285
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
CP-51574: Add explicit reentrant lock to Db_lock (#6011)
Update `Db_lock` to use an explicit reentrant (recursive) mutex pattern. This is a pretty standard implementation. The way it works is that threads compete to become the "holder" of the lock. The current holder of the lock is identified using thread identifiers (integers) and read/written from/to atomically (by way of `Atomic.t`). If attempting to acquire the lock (i.e. atomically `compare_and_set` the holder, with the expected current value as physically equal to `None`) fails, then the thread begins to wait. Waiting threads wait using a condition variable to avoid busy waiting. The lock can be acquired several times after it is acquired by the calling thread, but releasing it must ensure that every lock is matched by a corresponding unlock (the thread must relinquish every "hold" it has on the lock). Upon successfully releasing its holds on the lock, the thread that is relinquishing control resets the holder (to `None`) and uses the condition variable to signal a waiting thread to wakeup (and attempt to become the holder). To make this pattern safe and not expose too many details, an interface file (`db_lock.mli`) is introduced. This interface does not expose the details of the underlying lock, but rather the single function: ```ocaml val with_lock : (unit -> 'a) -> 'a ``` which ensures the invocation of its argument is properly sandwiched between code that correctly acquires and releases the lock (taking care to avoid holding onto the lock during exceptional circumstances, e.g. by way of an exception that is unhandled). Code written to use the database lock currently only use this interface to acquire the `Db_lock`, so no other changes are required. A follow-up change may be to enforce the same usage pattern for the global database flush lock, which is seemingly only used in the same way. ----- Ring3: BST+BVT (205351) all looks fine (so far) except an SDK-related test, which appears to be a known issue. I have ran the same suites on the same code a few times, with everything green except known issues. This is really a code cleanup effort, as opposed to new functionality. The current code is an ad-hoc implementation of this idea. The hope is that the style moving forward would be to use atomics and locks, so that we can begin to tackle things that would not be domain-safe in OCaml 5. For example, the current implementation queries `allow_thread_through_dbcache_mutex` (a `ref`) non-atomically. Some code could probably be moved around but I'm unwilling to do any serious functional changes as it would be rather easy to introduce careless errors. This is the kind of simple implementation that you will find in textbooks.
- Loading branch information
Showing
5 changed files
with
161 additions
and
47 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
(* | ||
* Copyright (c) Cloud Software Group | ||
* | ||
* This program is free software; you can redistribute it and/or modify | ||
* it under the terms of the GNU Lesser General Public License as published | ||
* by the Free Software Foundation; version 2.1 only. with the special | ||
* exception on linking described in file LICENSE. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Lesser General Public License for more details. | ||
*) | ||
|
||
val global_flush_mutex : Mutex.t | ||
|
||
val with_lock : (unit -> 'a) -> 'a | ||
(** [with_lock f] executes [f] in a context where the calling thread | ||
holds the database lock. It is safe to nest such calls as the | ||
underlying lock is reentrant (a recursive mutex). *) | ||
|
||
type report = {count: int; avg_time: float; min_time: float; max_time: float} | ||
|
||
val report : unit -> report |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,9 @@ | |
(libraries | ||
forkexec | ||
gzip | ||
mtime | ||
mtime.clock.os | ||
clock | ||
rpclib.core | ||
rpclib.json | ||
safe-resources | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters