MB-39280: Flusher marks-clean item in the HashTable by Seqno

We used to check the CAS in place of Seqno in the PersistenceCallback,
which is wrong as we have legal scenarios where multip

MB-39280: Flusher marks-clean item in the HashTable by Seqno

We used to check the CAS in place of Seqno in the PersistenceCallback,
which is wrong as we have legal scenarios where multiple mutations with
different Seqno and same CAS (for the same key) may be queued in the
system.

Eg, an XDCR setup where multiple source clusters replicate to the same
destination cluster.
In that case, we may end up with marking clean (by CAS) a StoredValue
that has never been persisted. That makes the SV eligible for eviction.
Which in turn may lead to exposing a stale item from disk.

See the unit test linked to this fix for a detailed reproduction of the
described scenario.

Change-Id: I8873ea3b4078284c02c3f17dec7ae20df0ee122b
Reviewed-on: http://review.couchbase.org/c/kv_engine/+/129399
Well-Formed: Build Bot <build@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>

show more ...

Minimise includes of kv_bucket.h

Change-Id: I489b23efe69e44db51602a52d4e01f275959034b
Reviewed-on: http://review.couchbase.org/113438
Tested-by: Build Bot <build@couchbase.com>
R

Minimise includes of kv_bucket.h

Change-Id: I489b23efe69e44db51602a52d4e01f275959034b
Reviewed-on: http://review.couchbase.org/113438
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Richard de Mellow <richard.demellow@couchbase.com>

show more ...

MB-23086: Always use SynchronousEPEngine::build

Ensure the various tests which use SynchronousEPEngine
use the factory build method to give better control
around the setup/teardown o

MB-23086: Always use SynchronousEPEngine::build

Ensure the various tests which use SynchronousEPEngine
use the factory build method to give better control
around the setup/teardown of this object.

Change-Id: Iedc019f3bae151746775af62ba1a9de7b645621c
Reviewed-on: http://review.couchbase.org/107946
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

show more ...

MB-32762: Refactor: change ADD_STAT to std::function 'AddStatFn'

Allows greater flexibility in how ADD_STAT is implemented; not limited
to the only state being cookie.

This is u

MB-32762: Refactor: change ADD_STAT to std::function 'AddStatFn'

Allows greater flexibility in how ADD_STAT is implemented; not limited
to the only state being cookie.

This is used by subsequent patches to wrap ADD_STAT so its memory
usage is correctly tracked.

One downside of std::function compared to a raw function pointer is it
is more costly to copy. As such, also update all the uses of ADD_STAT
to pass as const reference so there's no additional runtime cost for
parameter passing. Given this results in touching virtually all uses
of ADD_STAT, rename it to match the current coding standards naming
convetion - AddStatFn.

Change-Id: I358833c7012652ac07f6efda73f4bf4a0d058294
Reviewed-on: http://review.couchbase.org/103893
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>

show more ...

MB-30552: Convert type of vBucket id's from uint16_t to Vbid [8/n]

(Stage 8 of n) This commit changes to the type of Vbid for
ep's tests, including mock.
Vbid allows greater control

MB-30552: Convert type of vBucket id's from uint16_t to Vbid [8/n]

(Stage 8 of n) This commit changes to the type of Vbid for
ep's tests, including mock.
Vbid allows greater control over what operations can be performed
on a vBucket id, as well as ensuring the output formatting is
consistent. To support the splitting of this into multiple stages,
Vbid can currently be implicitly converted back to uint16_t to
cover elements that have not yet been converted. During this,
there may be logs or prints with "vb:vb:", which should be fixed
within this set of changes.

Change-Id: Ie86933239982b0dd882746fcc5ea0e490b79b3e7
Reviewed-on: http://review.couchbase.org/99627
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

show more ...

MB-22010: Implement statistical counter eviction policy

Implements the eviction policy for the statistical counter. The policy
is describes as follows:

The algorithm first calc

MB-22010: Implement statistical counter eviction policy

Implements the eviction policy for the statistical counter. The policy
is describes as follows:

The algorithm first calculates the percentage of total memory that
we need to recover to get below the low water mark. This percentage
is used as an initial estimate of how many items to evict from the hash
tables.

The algorithm visits each vbucket in turn. For each vbucket we visit
every item in the hash table. For each item we:
- Add its frequency count to a histogram.
- Use the estimate percentage to retrieve a frequency threshold from the
histogram (Note: as an opitmisation we do not update the estimate
on every item that is visited).
- Get the frequency count of the current item and if it below or equal
to the threshold we evict it from the hash table.

It is possible that the percentage estimate of items to evict is not
sufficient to bring the total memory memory usage below the low water
mark. Therefore we have an evictionMultiplier that is used to increase
the estimate if additional pass of the algorithm are required.

The multipler is increased each time an additional pass of the algorithm
is required, therefore ensuring that we evict sufficient items to drop
below the low water mark.

Change-Id: I06c5249424ab6849a9c946058e67982b12744101
Reviewed-on: http://review.couchbase.org/88927
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

show more ...

MB-26470: Active stream to hold only a weak_ptr to producer conn

Currently active stream holds a shared_ptr to its producer conn
and the producer conn holds shared_ptr to all its streams

MB-26470: Active stream to hold only a weak_ptr to producer conn

Currently active stream holds a shared_ptr to its producer conn
and the producer conn holds shared_ptr to all its streams.
When a DcpConnMap is deleted without explicitly closing all its
connections, there is a cyclic reference between producer conn
and its streams.

Hence, this commit converts the shared_ptr (to producer) held by
the active stream to weak_ptr. It also adds a test case that
demonstrates the memory when run with address sanitizer.

Change-Id: Ie84e6559595e40aca31363062606c07fa083f6ec
Reviewed-on: http://review.couchbase.org/84652
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

show more ...

MB-25897: ObjectRegistry: decrement numItem when Items are deleted

ObjectRegistry::onDeleteItem was incorrectly incrementing numItem
instead of decrementing.

This is a regressio

MB-25897: ObjectRegistry: decrement numItem when Items are deleted

ObjectRegistry::onDeleteItem was incorrectly incrementing numItem
instead of decrementing.

This is a regression since 8bf14273
(http://review.couchbase.org/#/c/69951/8), introduced in 4.6.0.

Change to decrement; and add regression test.

Change-Id: Id5af53c66c434848523dce0faded2890a408c571
Reviewed-on: http://review.couchbase.org/83016
Well-Formed: Build Bot <build@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: Build Bot <build@couchbase.com>

show more ...

MB-20940: [18/n] Tap Removal: Remove unused code from Connmap

Change-Id: If43f8aab014f5e2949b6373d1e7066ad3b703353
Reviewed-on: http://review.couchbase.org/81163
Tested-by: Build Bot

MB-20940: [18/n] Tap Removal: Remove unused code from Connmap

Change-Id: If43f8aab014f5e2949b6373d1e7066ad3b703353
Reviewed-on: http://review.couchbase.org/81163
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>

show more ...

Refactor DcpConnMap to be a unique_ptr

Change-Id: Ideade056b31f6c7587eb808763730246c113e304
Reviewed-on: http://review.couchbase.org/80454
Reviewed-by: Sriram Ganesan <sriram@couchba

Refactor DcpConnMap to be a unique_ptr

Change-Id: Ideade056b31f6c7587eb808763730246c113e304
Reviewed-on: http://review.couchbase.org/80454
Reviewed-by: Sriram Ganesan <sriram@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

show more ...

MB-20940: Remove TAP tests

The first step in the removal of TAP is to remove the TAP specific
tests. No changes have been made to production code.

Change-Id: I6842d8119ac598fad

MB-20940: Remove TAP tests

The first step in the removal of TAP is to remove the TAP specific
tests. No changes have been made to production code.

Change-Id: I6842d8119ac598fad2322fb2594526f539568ca6
Reviewed-on: http://review.couchbase.org/80343
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>

show more ...

Move ep-engine to engines/ep

Add WithMeta unit tests

Create a test-suite using the SingleThreaded store test to drive
the with_meta commands.

The test-suite so far tries to exercise many of the error and su

Add WithMeta unit tests

Create a test-suite using the SingleThreaded store test to drive
the with_meta commands.

The test-suite so far tries to exercise many of the error and success
paths.

The intent is to give a starting point for more comprehensive and easy
to understand *with_meta tests as opposed to the extension of the
testapp tests.

Change-Id: I4bdfec9879de3a9671fed820f9b5aab0ac7d2c71
Reviewed-on: http://review.couchbase.org/76381
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>

show more ...

Unit tests: refactor to allow Ephemeral variant

Refactor the EPBucket unit tests to allow subsequent subclasses of
tests to be created which instantiate an Ephemeral bucket instead of

Unit tests: refactor to allow Ephemeral variant

Refactor the EPBucket unit tests to allow subsequent subclasses of
tests to be created which instantiate an Ephemeral bucket instead of
Persistent.

There are two main changes here:

1. Modify the way we create the KVBucket instance to dynamically
select based on the configuration, using the new makeBucket()
method. Previously this was done with a hard-coded type, which made
it hard to re-use the same code for Ephemeral buckets.

2. Modify the way we expose protected from EPBucket to tests, from
creating a derived 'MockEPBucket' to using friendship. While
friendship is generally less desirable (as gives access to private
members in addition to protected), it doesn't require us to
subclass the specific class we want to access; and that is
problematic when we want to access two sibling classes (EPBucket &
EphemeralBucket) from the same code. I experimented with multiple
inheritance but it quickly got hairy.

Change-Id: I7d942683c14ba5c160e85e45855ae6915133800b
Reviewed-on: http://review.couchbase.org/74994
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>

show more ...

Move Bucket construction to makeBucket()

To allow test code to construct the correct KVBucket subclass based on
the configuration, move the code to select and create the bucket from

Move Bucket construction to makeBucket()

To allow test code to construct the correct KVBucket subclass based on
the configuration, move the code to select and create the bucket from
EventuallyPersistentEngine::initialize to a seperate method -
makeBucket(). This will be used in subsequent Ephemeral bucket unit
tests.

As part of this, change EventuallyPersistentEngine::kvBucket from a
raw to a unique_ptr.

Change-Id: I527736f2f03a5eaa6effe6289deb15701f4c301b
Reviewed-on: http://review.couchbase.org/74993
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

show more ...

Mocks: Rename MockEPStore -> MockEPBucket

This more accurately reflects the class it's mocking (EPBucket) given
our recent renaming for Ephemeral.

Change-Id: I08845a5341f7929d7d

Mocks: Rename MockEPStore -> MockEPBucket

This more accurately reflects the class it's mocking (EPBucket) given
our recent renaming for Ephemeral.

Change-Id: I08845a5341f7929d7d932e08f7f2704c97a05f5b
Reviewed-on: http://review.couchbase.org/74991
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

show more ...

MB-22013: Refactor out mock synrchonous ep-engine

Refactors out the synchronous_ep-engine for use in other tests and
benchmarks.

Change-Id: Idf8966835b8cb69dd241b5fa0aa27afa2cee

MB-22013: Refactor out mock synrchonous ep-engine

Refactors out the synchronous_ep-engine for use in other tests and
benchmarks.

Change-Id: Idf8966835b8cb69dd241b5fa0aa27afa2cee43ed
Reviewed-on: http://review.couchbase.org/74015
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

show more ...

MB-22451: Ensure isBackfillTaskRunning is correctly set

In ActiveStream::completeBackfill if in STREAM_BACKFILLING state and
pendingBackfill is true then we will schedule another backfil

MB-22451: Ensure isBackfillTaskRunning is correctly set

In ActiveStream::completeBackfill if in STREAM_BACKFILLING state and
pendingBackfill is true then we will schedule another backfill. This
will cause isBackfillTaskRunning to be set to true. The flag should
remain true on exit of the completeBackfill function.

Change-Id: If8219a7f87b65af46d37a800eebf2257917cc555
Reviewed-on: http://review.couchbase.org/73310
Well-Formed: Build Bot <build@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>

show more ...

Add pre-expiry hook in order to strip off system xattrs

Implement a pre-expiry hook to retain the system extended
attributes in case a document containing xattrs is being expired.

Add pre-expiry hook in order to strip off system xattrs

Implement a pre-expiry hook to retain the system extended
attributes in case a document containing xattrs is being expired.

Change-Id: I1cb821d30ed4bb9131e28ce55944118c5147dd32
Reviewed-on: http://review.couchbase.org/72077
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>

show more ...

[Ephemeral] Create EPBucket concrete subclass

Create a new EPBucket (Eventually Persistent Bucket) class which is a
subclass of KVBucket. This will be used for any EP-specific
functi

[Ephemeral] Create EPBucket concrete subclass

Create a new EPBucket (Eventually Persistent Bucket) class which is a
subclass of KVBucket. This will be used for any EP-specific
functionality which is not applicable to the sibling EphemeralBucket
class.

Change-Id: Idf6366b8f3bdb5476a1914161962474bd06ba114
Reviewed-on: http://review.couchbase.org/71383
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>

show more ...

[4/4] Ephemeral class shuffle: Rename class EPBucket -> KVBucket

Finally rename the code which is the shared parent class to the
correct name.

Change-Id: I4d1368663c73c64665d81d

[4/4] Ephemeral class shuffle: Rename class EPBucket -> KVBucket

Finally rename the code which is the shared parent class to the
correct name.

Change-Id: I4d1368663c73c64665d81d32e6a5aa60e34411f3
Reviewed-on: http://review.couchbase.org/71105
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>

show more ...

[3/4] Ephemeral class shuffle: Rename ep.{cc,h} -> kv_bucket.{cc,h}

Changes no code, but move the file which is to become the new shared
parent class to the correct filename.

Ch

[3/4] Ephemeral class shuffle: Rename ep.{cc,h} -> kv_bucket.{cc,h}

Changes no code, but move the file which is to become the new shared
parent class to the correct filename.

Change-Id: I956d8eaa1343924e49cc3906e1f66581b553a1f0
Reviewed-on: http://review.couchbase.org/71104
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>

show more ...

MB-21916: Make use of StoredDocKey

Where std::string stored a key, now we use StoredDocKey
When we passed std::string& through interfaces we now
pass DocKey as much as possible, thus

MB-21916: Make use of StoredDocKey

Where std::string stored a key, now we use StoredDocKey
When we passed std::string& through interfaces we now
pass DocKey as much as possible, thus delaying the
heap alloc and memcpy to the places we really need
to store a key.

This patch does not store the namespace in StoredValue
This patch does not store the namespace in couchstore/fdb

Thus any keys created in say Collections namespace will
not work. This is correct as this patch does not make
any assertions about the support of non-DefaultCollection

Change-Id: Ibc7f59183e59f43d55fad5e582232e2891231179
Reviewed-on: http://review.couchbase.org/70698
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>

show more ...

MB-21568: Reconcile hashtable with disk following rollback

After rolling back the disk store to the requested seqno a scan of the
vbucket's checkpoint must occur. Any item in the checkpo

MB-21568: Reconcile hashtable with disk following rollback

After rolling back the disk store to the requested seqno a scan of the
vbucket's checkpoint must occur. Any item in the checkpoint with
a seqno > than the rollback must be dropped or rolled back.

+ A missing close is addressed by using a wrapper class that will
RAII close each file opened in the rollback code.

Change-Id: Iabe43f59ed40931c1c97b65147b7a414d4ff7cc5
Reviewed-on: http://review.couchbase.org/69725
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>

show more ...

MB-21587: Rollback: Don't use CAS when rolling back values in HashTable

When ep-engine performs a rollback, if a key needs to be reverted to a
previous value (it has been modified since

MB-21587: Rollback: Don't use CAS when rolling back values in HashTable

When ep-engine performs a rollback, if a key needs to be reverted to a
previous value (it has been modified since the rollbackSeqno), the
HashTable update fails due to being called incorrectly - it uses a set
with CAS (where the CAS used is the one for the previous item),
whereas the element in the HashTable will have a newer CAS (by
defintion as we are trying to roll it back).

The effect of this is that after rollback, old values for keys still
exist in memory (disk is correct) on the replica vBucket(s). In the
event of a subsequent failover (and promotion of the replica ->
active), incorrect values will be returned for such keys.

Fix by changing the HashTable::set to a non-CAS one.

As part of testing, made use of recently-added equality and output
operators for Item and related classes:

[ RUN ] RollbackTest.MB21587_RollbackAfterMutation
source/ep-engine/tests/module_tests/evp_store_rollback_test.cc:70: Failure
Value of: *result.getValue()
Actual: Item[0x10441d860] with key:a
value:Blob[0x104543180] with size:5 extMetaLen:1 age:0 data: <01 01 n e w>
metadata:ItemMetaData[0x10441d868] with cas:5407a59650006 revSeqno:2 flags:0 exptime:0
bySeqno:7 queuedTime:0 vbucketId:0 op:set nru:2
Expected: item_v1
Which is: Item[0x7fff5f181ce0] with key:a
value:Blob[0x104543160] with size:5 extMetaLen:1 age:0 data: <01 01 o l d>
metadata:ItemMetaData[0x7fff5f181ce8] with cas:5407a59650005 revSeqno:1 flags:0 exptime:0
bySeqno:6 queuedTime:0 vbucketId:0 op:set nru:2
Fetched item after rollback should match item_v1
[ FAILED ] RollbackTest.MB21587_RollbackAfterMutation (22 ms)

Change-Id: I32577afd0f6f9c79122575f84a76acd00fb6f89a
Reviewed-on: http://review.couchbase.org/69478
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>

show more ...