MB-23795: Move StoredValue::stale to OSV; guard with SeqList::writeLock 15/76715/7
authorDave Rigby <daver@couchbase.com>
Wed, 12 Apr 2017 14:00:53 +0000 (15:00 +0100)
committerDave Rigby <daver@couchbase.com>
Thu, 13 Apr 2017 16:25:23 +0000 (16:25 +0000)
commit485a86ca13365110b428b4c4c36a19b5b53cefc5
tree73444ab0ae7930cdba04422f0fe47967a26824be
parent5c17c2ccb77bd244e7a0d144ee5ecd23fc54a341
MB-23795: Move StoredValue::stale to OSV; guard with SeqList::writeLock

The stale flag in (Ordered)StoredValue needs to be read by the
tombstone purger when iterating the sequenceList; however at this
point there is no HashTable lock available - the OSV has been released
from the hashTable and hence we have no way to determine it's bucket
(without reading the key to recalculate - and that's not possible
without first acquiring the lock!).

To allow us to safely access the stale flag, move it from being
protected by the HT locks to the SequenceList::writeLock, and move to
OSV so it doesn't share with the other bitfields in the same
byte. While on the face of it this might seen like a serious
performance degredation having to serialise on a single lock, it
actually doesn't appear to be /that/ bad:

1) The stale flag is only accessed rarely:

   a) Set to false when a StoredValue is initially created (and this
      doesn't need the lock as the item isn't in the SeqList yet)

   b) Set to true when an item becomes stale (result of concurrent
      update & rangeRead, or deleted item is aged out and marked Stale).

   c) Read during tombStone purging.

2) Even when we *do* need to access the stale flag, we have already
   acquired the writeLock in the hot path
   (updateStoredValue/markItemStale).

Note: This will increase contention during a tombstone purge, as it
will need to acquire and release the writeLock for every seqList
element; this is an area which should be looked at for future
performance improvement - for example can the stale flag be made
atomic, or can we add lightweight, per-OSV lock?

Note(2): This increases the size of OSV by 8 bytes, as moving Stale to
it pushes up the alignment requirements (it was previously aligned to
8 bytes), although we've only added 1 bit to it. Better solution would
be to pack this into the boost_list hook, // or introduce per-OSV
microlock to guard the whole object.

Change-Id: If814b966a10c64fada204998410dafc5ad255351
Reviewed-on: http://review.couchbase.org/76715
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
src/linked_list.cc
src/stored-value.cc
src/stored-value.h
tests/mock/mock_basic_ll.h
tests/module_tests/basic_ll_test.cc
tests/module_tests/stored_value_test.cc