MB-29480, MB-29512: Fail backfills that go below purge-seqno

If a backfill is requested and it is not a backfill of everything
the start must be below the purge-seqno, otherwise a DCP cl

MB-29480, MB-29512: Fail backfills that go below purge-seqno

If a backfill is requested and it is not a backfill of everything
the start must be below the purge-seqno, otherwise a DCP client
may miss deletions which have been purged.

This is achieved by loading the purgeSeqno into the ScanContext
and getting backfill create to abort (setting the stream as dead)

The initScanContext method will have opened the data file (and kept
it open) so that the purge-seqno used in the final check won't
change again.

Change-Id: I7505529d46eb6f2b6006695baf7dd9f190237df9
Reviewed-on: http://review.couchbase.org/93690
Reviewed-by: Dave Rigby <daver@couchbase.com>
Reviewed-by: Tim Bradgate <tim.bradgate@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

show more ...

MB-28327: Support frequency counter over DCP

Provide a new DCP control message for stating whether the consumer
supports the new high fidelity most recently used eviction algorithm
(

MB-28327: Support frequency counter over DCP

Provide a new DCP control message for stating whether the consumer
supports the new high fidelity most recently used eviction algorithm
(hifi_mfu) (also known as the statistical counter eviction policy).

If the consumer (and producer) supports the hifi_mfu then for all
mutations pass their associated frequency counter over DCP. If the
producer uses the hifi_mfu, but the consumer is using the old 2-bit_lru
then we convert each frequency counter value into one of the 4 states
used by the 2-bit_lru algorithm.

Change-Id: Ib0629cbce837996c4598710aff73f1ac9fce7db3
Reviewed-on: http://review.couchbase.org/90437
Reviewed-by: Tim Bradgate <tim.bradgate@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

show more ...

MB-26455: Allow correct processing of 'old' keys and separator changes

The collection's separator can only be changed when 0 collections
exist. However nothing stops the separator from c

MB-26455: Allow correct processing of 'old' keys and separator changes

The collection's separator can only be changed when 0 collections
exist. However nothing stops the separator from changing if there are
deleted collections (and their keys) in play.

Prior to this commit each separator change resulted in a single system
event being mutated, that event had a static key. Thus a VB could have
the following sequence of keys/events.

1 fruit::1
2 fruit::2
<fruit collection is deleted>
<separator is changed from :: to - >
<fruit collection is recreated>
6 fruit-1
7 fruit-2
<fruit collection is deleted>
<separator is changed from - to # >
9 $collections_separator (the Item value contains the new separator)
10 $collections::fruit (fruit recreated)
11 fruit#1
12 fruit#2

In this sequence, some of the changes are lost historically because a
static key is used for the separator change. Between seqno 2 and 6 the
separator changed from :: to -, but the separator change system event
is currently at seqno 9 with # as its value.

With this setup a problem exists if we were to process the historical
data e.g. whilst backfilling for DCP or performing a compaction
collection erase. The problem is that when we go back to seqno 1 and
2, we have no knowledge of the separator for those items, all we have
is the current # separator. We cannot determine that fruit::1 is a
fruit collection key.

This commit addresses this problem by making each separator change
generate a unique key. The key itself will encode the new separator,
and because the key is unique it will reside at the correct point in
history for each separator change.

The unique key format will be:

$collections_separator:<seqno>:<new_separator>

With this change the above sequence now looks as:

1 fruit::1
2 fruit::2
<fruit collection is deleted>
4 $collections_separator:3:- (change separator to -)
<fruit collection is recreated>
6 fruit-1
7 fruit-2
<fruit collection is deleted>
9 $collections_separator:8:# (change separator to #)
10 $collections::fruit (fruit recreated)
11 fruit#1
12 fruit#2

Now the code which looks at the historical data (e.g. backfill) will
encounter these separator change keys before it encounters collection
keys using that separator. Backfill and collections-erase can just
maintain a current separator and can now correctly split keys to
discover the collection they belong to. The collections eraser and
KVStore scanning code now include a collections context which has data
and associated code for doing this tracking.

A final piece of the commit is the garbage collection of these unique
keys. i.e. if each separator change puts a unique key into the seqno
index, how can we clean these away when they're no longer needed (i.e.
all fruit:: keys purged)?

Whilst the eraser runs it tracks all 'separator change' keys, because
a separator change can only happen when 0 collections exist, it can
assume that all but the latest separator change key are redundant once
the erase has completed. This list of keys are simply deleted in the
normal way by pushing a deleted Item into the checkpoint once
compaction is complete.

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

show more ...

MB-27489: Add further debug logging when backfill fails to read disk file

This commit logs the vbucket state when the backfill fails to read the
vbucket file and also bumps up the log le

MB-27489: Add further debug logging when backfill fails to read disk file

This commit logs the vbucket state when the backfill fails to read the
vbucket file and also bumps up the log level of a vbucket being deleted.

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

show more ...

MB-27495: Refactor all in-line methods from ep_engine.h into
ep_engine.cc

Remove dependency of kv_bucket.h from ep_engine.h
Cleanup includes of kv_bucket.h in ep_engine source

MB-27495: Refactor all in-line methods from ep_engine.h into
ep_engine.cc

Remove dependency of kv_bucket.h from ep_engine.h
Cleanup includes of kv_bucket.h in ep_engine source

Build time performance is as follows, running from a clean full
build, then touching kv_bucket.h and finally building ep_engine only:

ep_engine compile time pre-patch:
make -j8 336.90s user 20.23s system 651% cpu 54.835 total
ep_engine compile time post-patch:
make -j8 220.67s user 13.35s system 612% cpu 38.196 total

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

show more ...

MB-27489: Explicitly fail the stream upon backfill failure

When the DCP backfill fails due to a disk issue, we should
explictly fail the stream rather than ignoring the failure.

MB-27489: Explicitly fail the stream upon backfill failure

When the DCP backfill fails due to a disk issue, we should
explictly fail the stream rather than ignoring the failure.

Ignoring the failure resulted in a hang (like rebalance hang).
Now with this fix, the client can know that there is a failure
and hence maybe retry or handle the situation more gracefully.

The commit also adds a test for the same.

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

show more ...

MB-26021: Add StatusCallback<> subclass

Refactor the Callback<> template so the statusCode is moved into a
seperate subclass - StatusCallback.

This shrinks the size of Callback<

MB-26021: Add StatusCallback<> subclass

Refactor the Callback<> template so the statusCode is moved into a
seperate subclass - StatusCallback.

This shrinks the size of Callback<> (and any subclasses) which don't
need the status code by at least 4 bytes (can be more due to
alignment).

Specifically, this reduces PersistenceCallback from 48 to 32 bytes.

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

show more ...

MB-27036: Extend the scope of conn shared_ptr while logging in stream

When a stream object uses a the connection logger object for logging
it should ensure that the connection object is

MB-27036: Extend the scope of conn shared_ptr while logging in stream

When a stream object uses a the connection logger object for logging
it should ensure that the connection object is alive. For this,
the commit creates a stream function 'log()' which holds a shared_ptr
to the connection object during logging.

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

show more ...

MB-26705: DCP should make disk items cold

DCP already transmits some 'nru' data in the mutation structure,
however this value is not a real cache heat because the Item created
for a

MB-26705: DCP should make disk items cold

DCP already transmits some 'nru' data in the mutation structure,
however this value is not a real cache heat because the Item created
for a SET (which eventually ends up in DCP sending) has an nru always
defaulted to INITIAL_NRU_VALUE (value of 2) and never 'copies' the
real nru from the cache. So in this commit we still send the nru of 2
for items being sent from memory but if an item came into the
DCP readyQueue from backfill only we make the mutation cold (i.e.
MAX_NRU_VALUE).

The consumer side now reads the NRU value from the incoming mutation
and constructs Items with that NRU value. So previosly we just used
the default value (2) for *all* mutations.

The result of this patch is that in-memory values still send nru:2 and
still get created on the consumer with nru:2, but now items from disk
have an nru:3 (cold). In a DGM rebalance this allows the pager to get
busy evicting the cold disk items on the first eviction pass rather
than doing multiple passes, the outcome is we keep mem_used away from
the quota and reduce the TMP_FAIL errors a new node may produce.

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

show more ...

MB-26470: Use weak_ptr when non-owner objs hold ref to a stream obj

The DCP producer conn owns the stream obj and holds a ref counted
shared ptr to it. All other objs, say like backfill

MB-26470: Use weak_ptr when non-owner objs hold ref to a stream obj

The DCP producer conn owns the stream obj and holds a ref counted
shared ptr to it. All other objs, say like backfill obj, that need
the stream obj must only hold a weak_ptr to it inorder to avoid
cyclic reference issues.

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

show more ...

MB-25344: [5/n] Ignore logically deleted keys for GET

GET should fail if the requested key is logically deleted, even if
the request uses the GET_DELETED_VALUE flag. Keys deleted by

MB-25344: [5/n] Ignore logically deleted keys for GET

GET should fail if the requested key is logically deleted, even if
the request uses the GET_DELETED_VALUE flag. Keys deleted by
collection deletion should be considered deleted differently to when
the user deletes a key.

Commit updates getInternal to accept a collections read handle so that
it can check any StoredValue for being logically deleted.

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

show more ...

MB-26470: Use shared ptr for Stream and ActiveStream objs.

Currently we use SingleThreadedRCPtr for Stream and ActiveStream
objs that are shared across multiple modules. This can lead to

MB-26470: Use shared ptr for Stream and ActiveStream objs.

Currently we use SingleThreadedRCPtr for Stream and ActiveStream
objs that are shared across multiple modules. This can lead to
cyclic references and hence memory leak.

This commit changes all SingleThreadedRCPtr for Stream and ActiveStream
to std::shared_ptr and hence sets up the ground to use std::weak_ptr
that would finally get rid of cyclic references.

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

show more ...

MB-25342: Hide logically deleted keys when DCP backfilling

When DCP backfills, it must not forward onto the DCP stream any
logically deleted keys as a client must never see them.

MB-25342: Hide logically deleted keys when DCP backfilling

When DCP backfills, it must not forward onto the DCP stream any
logically deleted keys as a client must never see them.

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

show more ...

Refactor: CacheCallback to not directly access HashTable object Ver. 2

The hash table object should ideally be a protected member of the
VBucket class, as opposed to being public.

Refactor: CacheCallback to not directly access HashTable object Ver. 2

The hash table object should ideally be a protected member of the
VBucket class, as opposed to being public.

The patch moves towards this goal by introducing a getItem function to
the VBucket class, which ensures that the backfill function
CacheCallback::callback no longer requires direct access to the hash
table object.

Tests to ensure the refactor does not change functionality were added
in the following patch http://review.couchbase.org/#/c/78650/

Note: This patch performs the same refactoring to one that has already
been merged. However it was found to have a bug and therefore was
reverted. The bug was that it was thought that
EPVBucket::getInternalNonResident would only be called with
QUEUE_BG_FETCH set as an option however, this was found not to be the
case. CacheCallback::callback, which utlimately calls
EPVBucket::getInternalNonResident, does not set the option. In this
case we want EPVBucket::getInternalNonResident to return
ENGINE_EWOULDBLOCK.

For original refactor see:
- Change-Id: I2c71dd30f6b8c66de668137cac1bf358bbdf6588
For the revert of the refactor see:
- Change-Id: I5dc4e7dad4bb4a91ff94eb7509499ec8f62874b5

In addition to addressing the bug found in the original patch, this
version has an additional regression test to protect from the bug being
re-introduced.

Change-Id: I67f2693d8cbabb30be5b9cc72d550b8f68ee6316
Reviewed-on: http://review.couchbase.org/79961
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>

show more ...

MB-24972: Revert of CacheCallback to not directly access HashTable object

The change I2c71dd30f6b8c66de668137cac1bf358bbdf6588 appears to have
caused a regression where values of documen

MB-24972: Revert of CacheCallback to not directly access HashTable object

The change I2c71dd30f6b8c66de668137cac1bf358bbdf6588 appears to have
caused a regression where values of documents are lost during rebalance.

This patch reverts I2c71dd30f6b8c66de668137cac1bf358bbdf6588.
Some trivial code tidy-up remains.

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

show more ...

Refactor: CacheCallback to not directly access HashTable object

The hash table object should ideally be a protected member of the
VBucket class, as opposed to being public.

The

Refactor: CacheCallback to not directly access HashTable object

The hash table object should ideally be a protected member of the
VBucket class, as opposed to being public.

The patch moves towards this goal by introducing a getItem function to
the VBucket class, which ensures that the backfill function
CacheCallback::callback no longer requires direct access to the hash
table object.

Tests to ensure the refactor does not change functionality were added
in the following patch http://review.couchbase.org/#/c/78650/

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

show more ...

MB-24037 / MB-23605 Strip xattrs and value and record message size

Moves the stripping of xattrs and value (if want key only) from
memcached to ep-engine. Also ensures that the mutation

MB-24037 / MB-23605 Strip xattrs and value and record message size

Moves the stripping of xattrs and value (if want key only) from
memcached to ep-engine. Also ensures that the mutationResponse message
size, which will be sent over the wire, is updated correctly.

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

show more ...

MB-23377: Alter Item* to unique_ptr in GetValue

This patch adds a mechanism to transfer ownership of the Item out of the
GetValue which a number of call sites need to make use of.

MB-23377: Alter Item* to unique_ptr in GetValue

This patch adds a mechanism to transfer ownership of the Item out of the
GetValue which a number of call sites need to make use of.

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

show more ...

Move ep-engine to engines/ep

MB-24151: Implement memory managed backfills using Range Iterators

This commit implements memory management in the backfills of
Ephemeral buckets. It borrows the idea from disk backfills

MB-24151: Implement memory managed backfills using Range Iterators

This commit implements memory management in the backfills of
Ephemeral buckets. It borrows the idea from disk backfills where
the backfill is suspended upon high memory usage, that is items
are not put onto the readyQ of the stream.

At the core is a Backfill state machine which is driven by
the BackfillManagerTask. It keeps track of any suspended backill
and also resumes from the suspended point. It also creates a range
iterators on the in-memory sequence list and using this iterator
it reads (backfills) items one by one.

This commit also adds certain features to the sequence list iterators
to get snapshot numbers, estimate num items in backfill etc.

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

show more ...

MB-23734: Do memory mgmt across backfills in Ephemeral backfills

In Ephemeral buckets we currently do not have backfill memory mgmt.
Mainly because upon increased memory usage by backfil

MB-23734: Do memory mgmt across backfills in Ephemeral backfills

In Ephemeral buckets we currently do not have backfill memory mgmt.
Mainly because upon increased memory usage by backfill items we cannot
easily pause the backfill midway because pausing a backfill will
increase the duplicate items in the ephemeral sequential data structure.

This commit adds memory mgmt across backfills (each vbucket is an
individual backfill). Upon full usage of the backfill buffer we stop
running other backfills until the backfill buffer is empty again.

However once a backfill starts it runs till completion even if its
memory usage goes beyond the buffer size.

Benefit: We will not run new backfills once backfill buffer is full.

Known limitation: We don't stop the currently running backfill even
if the backfill buffer is full.

We plan to address this limitation soon.

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

show more ...

MB-22738: Move stripping of value from DCP stream back into ep-engine

A revert of the http://review.couchbase.org/#/c/72398/
with the addition that determining whether whether to retriev

MB-22738: Move stripping of value from DCP stream back into ep-engine

A revert of the http://review.couchbase.org/#/c/72398/
with the addition that determining whether whether to retrieve only
is made on the connection level, (as opposed to the stream level).

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

show more ...

Refactor: Replace RCPtr<VBucket> with VBucketPtr

Declare a named type for shared pointers to VBuckets.

Change-Id: I93e121f86199617c1651c5896efc7df7cd99ea83
Reviewed-on: http://r

Refactor: Replace RCPtr<VBucket> with VBucketPtr

Declare a named type for shared pointers to VBuckets.

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

show more ...

MB-23637: Extra logging for dcp-vbtakeover stat & refactor stream type

The dcp-vbtakeover stat is critical to the success of rebalance.
Therefore it is useful for debugging future rebala

MB-23637: Extra logging for dcp-vbtakeover stat & refactor stream type

The dcp-vbtakeover stat is critical to the success of rebalance.
Therefore it is useful for debugging future rebalance failures to log
the unexpected paths and error paths.

We want to print the stream type in the logging output. Therefore the
stream type has been refactored into a enum class and a to_string
method provided to print a textual representation of the type.

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

show more ...

Change StoredValue::toItem to return via unique_ptr<>

To prevent memory leaks of Items created by StoredValue::toItem(),
change from returning a raw pointer to unique_ptr<Item>. Update t

Change StoredValue::toItem to return via unique_ptr<>

To prevent memory leaks of Items created by StoredValue::toItem(),
change from returning a raw pointer to unique_ptr<Item>. Update the
callers of toItem() as appropriate.

Note I've curtailed the propogation of the Item* -> unique_ptr<Item>
change to the first "non-intrusive" point, otherwise the scope of this
patch would be significantly larger, however ideally we should
continue the propogation to make all owners of Item objects use owning
pointers.

Change-Id: I3d7062b8a06c9f522bb80409bac4561eaac60d3a
Reviewed-on: http://review.couchbase.org/75216
Reviewed-by: Oliver Downard <oliver.downard@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

show more ...