lock transactions: avoid redundant reading in write transactions
Our `LockTransaction` class was reading overly aggressively.  In cases
like this:
```
1  with spack.store.db.read_transaction():
2    with spack.store.db.write_transaction():
3      ...
```
The `ReadTransaction` on line 1 would read in the DB, but the
WriteTransaction on line 2 would read in the DB *again*, even though we
had a read lock the whole time.  `WriteTransaction`s were only
considering nested writes to decide when to read, but they didn't know
when we already had a read lock.
- [x] `Lock.acquire_write()` return `False` in cases where we already had
       a read lock.
			
			
This commit is contained in:
		| @@ -275,7 +275,12 @@ def acquire_write(self, timeout=None): | |||||||
|             wait_time, nattempts = self._lock(fcntl.LOCK_EX, timeout=timeout) |             wait_time, nattempts = self._lock(fcntl.LOCK_EX, timeout=timeout) | ||||||
|             self._acquired_debug('WRITE LOCK', wait_time, nattempts) |             self._acquired_debug('WRITE LOCK', wait_time, nattempts) | ||||||
|             self._writes += 1 |             self._writes += 1 | ||||||
|             return True |  | ||||||
|  |             # return True only if we weren't nested in a read lock. | ||||||
|  |             # TODO: we may need to return two values: whether we got | ||||||
|  |             # the write lock, and whether this is acquiring a read OR | ||||||
|  |             # write lock for the first time. Now it returns the latter. | ||||||
|  |             return self._reads == 0 | ||||||
|         else: |         else: | ||||||
|             self._writes += 1 |             self._writes += 1 | ||||||
|             return False |             return False | ||||||
|   | |||||||
| @@ -1087,6 +1087,62 @@ def write(t, v, tb): | |||||||
|         assert vals['wrote'] |         assert vals['wrote'] | ||||||
|  |  | ||||||
|  |  | ||||||
|  | def test_nested_reads(lock_path): | ||||||
|  |     """Ensure that write transactions won't re-read data.""" | ||||||
|  |  | ||||||
|  |     def read(): | ||||||
|  |         vals['read'] += 1 | ||||||
|  |  | ||||||
|  |     vals = collections.defaultdict(lambda: 0) | ||||||
|  |     lock = AssertLock(lock_path, vals) | ||||||
|  |  | ||||||
|  |     # read/read | ||||||
|  |     vals.clear() | ||||||
|  |     assert vals['read'] == 0 | ||||||
|  |     with lk.ReadTransaction(lock, acquire=read): | ||||||
|  |         assert vals['read'] == 1 | ||||||
|  |         with lk.ReadTransaction(lock, acquire=read): | ||||||
|  |             assert vals['read'] == 1 | ||||||
|  |  | ||||||
|  |     # write/write | ||||||
|  |     vals.clear() | ||||||
|  |     assert vals['read'] == 0 | ||||||
|  |     with lk.WriteTransaction(lock, acquire=read): | ||||||
|  |         assert vals['read'] == 1 | ||||||
|  |         with lk.WriteTransaction(lock, acquire=read): | ||||||
|  |             assert vals['read'] == 1 | ||||||
|  |  | ||||||
|  |     # read/write | ||||||
|  |     vals.clear() | ||||||
|  |     assert vals['read'] == 0 | ||||||
|  |     with lk.ReadTransaction(lock, acquire=read): | ||||||
|  |         assert vals['read'] == 1 | ||||||
|  |         with lk.WriteTransaction(lock, acquire=read): | ||||||
|  |             assert vals['read'] == 1 | ||||||
|  |  | ||||||
|  |     # write/read/write | ||||||
|  |     vals.clear() | ||||||
|  |     assert vals['read'] == 0 | ||||||
|  |     with lk.WriteTransaction(lock, acquire=read): | ||||||
|  |         assert vals['read'] == 1 | ||||||
|  |         with lk.ReadTransaction(lock, acquire=read): | ||||||
|  |             assert vals['read'] == 1 | ||||||
|  |             with lk.WriteTransaction(lock, acquire=read): | ||||||
|  |                 assert vals['read'] == 1 | ||||||
|  |  | ||||||
|  |     # read/write/read/write | ||||||
|  |     vals.clear() | ||||||
|  |     assert vals['read'] == 0 | ||||||
|  |     with lk.ReadTransaction(lock, acquire=read): | ||||||
|  |         assert vals['read'] == 1 | ||||||
|  |         with lk.WriteTransaction(lock, acquire=read): | ||||||
|  |             assert vals['read'] == 1 | ||||||
|  |             with lk.ReadTransaction(lock, acquire=read): | ||||||
|  |                 assert vals['read'] == 1 | ||||||
|  |                 with lk.WriteTransaction(lock, acquire=read): | ||||||
|  |                     assert vals['read'] == 1 | ||||||
|  |  | ||||||
|  |  | ||||||
| def test_lock_debug_output(lock_path): | def test_lock_debug_output(lock_path): | ||||||
|     host = socket.getfqdn() |     host = socket.getfqdn() | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Todd Gamblin
					Todd Gamblin