ep-engine.git
3 years agoMB-22602: Use CMake variables for target paths 81/78181/2
Dave Rigby [Tue, 16 May 2017 09:53:08 +0000 (10:53 +0100)]
MB-22602: Use CMake variables for target paths

Instead of manually specifying the path to various targets, use the
appropriate CMake variable / generator expression. This has the
advantage that if a target is moved to a different location, it will
still be correctly referred to.

In the case of ADD_TEST(), use the variant which specifies the NAME
option - this enables expansion of executable targets to their
absolute path.

Change-Id: I625b9908824969010b7535f2df5a69456d3d5ae4
Reviewed-on: http://review.couchbase.org/78181
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24244: Prevent duplicate items in an inMemoryBackfill 14/78014/9
James Harrison [Tue, 9 May 2017 11:07:40 +0000 (12:07 +0100)]
MB-24244: Prevent duplicate items in an inMemoryBackfill

Have rangeRead ignore stale items if their replacement also appears in
the requested range, for example

 A₁  B₂  C₃           Initial items
[A₁  B₂  C₃]          rangeRead 1-3
[A₁  B₂  C₃] B₄       Update B
 A₁  B₂  C₃  B₄       RR ends
[A₁  B₂  C₃  B₄]      new RR 1-4, B₂ should be ignored because B₄ is
also in the snapshot
[A₁  B₂  C₃  B₄] B₅   Another update, but B₄ must still be sent as B₅ is
not in the range
 A₁  B₂  C₃  B₄  B₅   RR ends

To achieve this, when marking items as stale, we store in it a pointer
to its replacement. To avoid bloating the OSV, we reuse the UniquePtr
which points to the items successor when in the HashTable. Once the item
is stale, it will never be used for that purpose again, leaving it free
for reuse.

This is a basic solution. It would be better to not have to take the
writeLock, but improvements of that form will be done in a separate
patch.

Change-Id: Id743da606e009d17f5e5af6f9344223a95aa4a38
Reviewed-on: http://review.couchbase.org/78014
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-16181: Interface update, allow DCP open to accept a value 38/78038/4
Jim Walker [Tue, 14 Mar 2017 12:05:25 +0000 (12:05 +0000)]
MB-16181: Interface update, allow DCP open to accept a value

DCP consumers will be able to open a DCP stream and specify a filter,
a list of collections they are interested in. This filter will be
passed as a JSON 'value' to DCP open.

Change-Id: Id33306f0d663b263840f70c44a16deff6db0a89e
Reviewed-on: http://review.couchbase.org/78038
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
3 years agoMB-24246: Combine 'writeLock' & 'highSeqnosLock' into one in BasicLinkedList 58/77958/7
Manu Dhundi [Fri, 12 May 2017 16:25:52 +0000 (09:25 -0700)]
MB-24246: Combine 'writeLock' & 'highSeqnosLock' into one in BasicLinkedList

Functionally 'writeLock' and 'highSeqnosLock' both result in the
serialization of the BasicLinkedList write and other list operations.
Hence this commit combines the 2 locks into one lock.

As a consequence this commit ensures that the writeLock is held on
the BasicLinkedList until the sequence number for the newly added,
updated and soft-deleted is generated. While this is strictly not
needed in new add, it is necessary in update and soft delete as
explained below.

(a) New Add :
   A1, B2, C3 and we are trying to add D4
   A1, B2, C3, D? added but no seqno yet.
   Now we don't really care if range read starts here as it can read
   only A1, B2, C3 in the snapshot.

   But to maintain uniformity (hence simpler code) with update
   and soft-delete we grab writeLock such that we append to list
   + update highSeqno as an atomic operation.

(b) Update (and Soft-delete):
   A1, B2, C3 and we are trying to update B to B4
   A1, B2, C3, B? added but no seqno yet and/or no B2_stale yet
   Now we cannot start the range read here because we do not wait
   for range read to finish to mark the item stale (note that we
   are planning to not send stale(duplicate) items in a snapshot).

   This was already in case (b) using highSeqnosLock, this commit
   uses just writeLock for the same.

The commit also adds documentation regarding the desired heirarchy of
the lock acquisition in BasicLinkedList.

Change-Id: I10a80f55a763d1496adec24fa12bc75d83ea1573
Reviewed-on: http://review.couchbase.org/77958
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoUse 'const' for the member 'Item* value' in class 'GetValue' 40/78040/3
Manu Dhundi [Thu, 11 May 2017 21:25:44 +0000 (14:25 -0700)]
Use 'const' for the member 'Item* value' in class 'GetValue'

Class GetValue contains a pointer to an item which is only used to
transfer out the item contained from one layer to another. It does not/
should not change the item it contains.

Hence it would be best to explicitly use const so that the item
contained is not modified by mistake in the future.

Further, it could be a future task to use std::unique_ptr<Item> in
GetValue to explicitly transfer the ownership of the item that
gets handed over from one layer to another.

Change-Id: Ic0a880f3070e299e8ff1628f3ac329aaa1a1b0cb
Reviewed-on: http://review.couchbase.org/78040
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoMB-24055: Change ht_size to same value as ns_server (3079) 90/77890/4
Dave Rigby [Tue, 9 May 2017 13:24:44 +0000 (14:24 +0100)]
MB-24055: Change ht_size to same value as ns_server (3079)

In preparation for removing the default value for ht_size from
ns_server where it currently is set (so KV-Engine owns the default),
change the value in configuration.json from 3 to 3079 - matching
ns_server's current default.

Note that this value is expected to be subsequently changed (reduced)
again as part of MB-24055 (to reduce memory overhead).

Change-Id: I1025709029dc2e6807e859c27f6ae6725efb7f53
Reviewed-on: http://review.couchbase.org/77890
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: James Harrison <james.harrison@couchbase.com>
3 years agoMB-24055: remove defaultNum{Buckets,Locks}, use config.json instead 86/77886/5
Dave Rigby [Tue, 9 May 2017 12:20:31 +0000 (13:20 +0100)]
MB-24055: remove defaultNum{Buckets,Locks}, use config.json instead

HashTable defines it's own set of default values for the hash table
initial size and number of locks. This appears to be a remnant of a
time before the central configuration.json we now know and love.

Having both is over-complex and confusing - it might /appear/ that the
default ht_size is 47 (from configuration.json), but in fact the
default is 3 - as hard-coded in hash_table.cc

Given we have config.json now, just use that as the holder of the
default sizes (and which can be overridden by ns_server if
desired). As such HashTable now requires an explicit initialSize and
nLocks in the constructor - which in normal usage will simply come
from the config.

Note that there should be no material change in the values used for
ht_locks or ht_size when run as a full Couchbase Server instance - if
a value is passed in by ns_server (via bucket config string) that will
be used; otherwise the previous values of defaultNum{Buckets,Locks} -
3 and 193 respectively - are used. In unit tests, any previously
overridden values have been updated here to the same values.

Change-Id: I8e3067014714a48acf1954310fdf13dce0b731d7
Reviewed-on: http://review.couchbase.org/77886
Reviewed-by: James Harrison <james.harrison@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoCTest: Increase maximum output size recorded to 1MB 93/77893/3
Dave Rigby [Tue, 9 May 2017 14:55:14 +0000 (15:55 +0100)]
CTest: Increase maximum output size recorded to 1MB

While tests run via Jenkins record the output of each test, the size
recorded is extremely limited - only 1KB for test which pass. This can
be problematic when (for example) one is trying to analyse which tests
in a suite were slow - for many of our test programs we run many
(sometimes hundreds) of tests, and hence 1024 characters quickly cuts
off interesting data.

As such, increase the limit to 1MB.

Change-Id: I5b2e5e966507951e01e088829120931a0e3bef11
Reviewed-on: http://review.couchbase.org/77893
Reviewed-by: James Harrison <james.harrison@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24055: change defaultNumLocks from 193 to 47 (logical no-op). 89/77889/3
Dave Rigby [Tue, 9 May 2017 13:03:42 +0000 (14:03 +0100)]
MB-24055: change defaultNumLocks from 193 to 47 (logical no-op).

Change the value of HashTable::defaultNumLocks from 193 to 47. Note
this is a no-op in a ns_server-controlled configutation as it already
explicilty sets the number of ht_locks to 47; therefore the only time
193 is used is in unit tests.

The reason for changing to 47 is to simplify testing - (1) we should
ideally test in a configuration as close to the "full" system, and (2)
we actually need to reduce the number of locks to <64 so we can run
successfully under ThreadSanitizer.

Change-Id: I1bcecc86cc90fbac215b8ee231179bee5d8cca32
Reviewed-on: http://review.couchbase.org/77889
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: James Harrison <james.harrison@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
3 years agoMB-24151: Implement general range read iterator in seqlist 40/77640/10
Manu Dhundi [Wed, 10 May 2017 00:36:40 +0000 (17:36 -0700)]
MB-24151: Implement general range read iterator in seqlist

This commit implements range read iterator in sequence list, the ordered
data strucuture in Ephemeral buckets.

We need range read iterators because we have more than one client that
needs to do a range read (backfill, tombstone purger). Further the reads
can also have more involved constraints like memory management in backfills.
By having an API of read iterator we can just do the read from the sequence
list and additional stuff like memory management can be done outside by the
clients. That is, iterator clients can make progress at their own pace.

The iterator is unidirectional (forward only) and cannot be invalidated
while in use. That means reading the items in the iterator results in a
valid point-in-time snapshot. But this adds a caveat that while an
iterator instance is active, certain invariants are to be met to get the
snapshot, and this happens at the expense of increased memory usage.

For now, only one iterator can be created at a time.

Change-Id: Idb320a148299255b74b7cf7e92cef35a20f483d4
Reviewed-on: http://review.couchbase.org/77640
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24159: Fix segfault in ephemeral backfill 80/77780/15
James Harrison [Wed, 3 May 2017 12:38:07 +0000 (13:38 +0100)]
MB-24159: Fix segfault in ephemeral backfill

DCPBackfillMemory::run would segfault if the underlying rangeRead
returned no items.

This was because the front and back of the UniqueItemPtr vector were
unconditionally dereferenced even if non-existent. This was to call
getBySeqno().

This patch replaces these calls with the startSeqno and endSeqno
specified when the DCPBackfillMemory task was created. This is
consistent with the behaviour of DCPBackfillDisk.

Change-Id: I952a78ef3d931bc0832cfffb9e392b394d412fb3
Reviewed-on: http://review.couchbase.org/77780
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24246: update highestDedupedSeqno when an existing value is changed 79/77779/12
James Harrison [Fri, 5 May 2017 13:22:21 +0000 (14:22 +0100)]
MB-24246: update highestDedupedSeqno when an existing value is changed

Without this, rangeReads are allowed to stop "too early" - potentially
missing out on items that have been updated, i.e.,

^ = HDDS (HighestDedupedSeqno)

 A₁   B₂   C₃            Initial items
[A₁   B₂   C₃]           rangeRead 1-3
[A₁   B₂   C₃]  B'₄       Update B
 A₁   B₂   C₃   B'₄       RR ends
[A₁   B₂   C₃]  B'₄       new rangeRead, still consistent
 A₁   B₂   C₃   B'₄       RR ends
 A₁        C₃   B'₄       purger removes stale B
!A₁        C₃!  B'₄       RR 1-3 could be requested, but would be
inconsistent. The HDDS serves to extend the end of a rangeRead to the
most recently deduped item, in this case B' replaced an older B.

Currently, this updates the HDDS too soon in the case of a stale item,
forcing the rangeRead to include the new version immediately even though
the stale item is still present and would still allow a valid range.
This is to be improved in a coming patch, in which the TombstonePurger
will update the HDDS when removing the stale item.

Change-Id: If6c57a86bab56ccc007b0fe17c9229218bb0c2c7
Reviewed-on: http://review.couchbase.org/77779
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-16181: Minimal update to the DCP engine methods 35/77835/4
Jim Walker [Mon, 8 May 2017 15:45:29 +0000 (16:45 +0100)]
MB-16181: Minimal update to the DCP engine methods

Update the DCP methods to pass/accept a new collection length
parameter.

This is a minimal update as this patch needs to be linked to the
memcached change to pass CV.

Change-Id: Ied8d0eea1b4d0c17004aa606f96a427d184c1495
Reviewed-on: http://review.couchbase.org/77835
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoReduce EphTombstoneTest/ConcurrentPurge runtime 94/77894/3
James Harrison [Tue, 9 May 2017 15:09:02 +0000 (16:09 +0100)]
Reduce EphTombstoneTest/ConcurrentPurge runtime

Reduced the number of documents used, and inserted a yield.

Change-Id: I54b787110e2195b52ec2576c1a4d49cfffc73355
Reviewed-on: http://review.couchbase.org/77894
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMerge branch 'watson' 81/77881/2
Dave Rigby [Tue, 9 May 2017 10:43:12 +0000 (11:43 +0100)]
Merge branch 'watson'

* watson:
  MB-24066/MB-22178: Set opencheckpointid to 1 after rollback
  MB-24066: Partial revert "MB-22178: Don't use opencheckpointid to determine if in backfill phase"

Change-Id: If7d42bd933abf24a737d2718a4056e4536df2fd8

3 years agoMerge branch 'watson' 80/77880/1
Dave Rigby [Tue, 9 May 2017 10:42:19 +0000 (11:42 +0100)]
Merge branch 'watson'

* watson:
  MB-23591: Re-instate isBucketDeletion check for flusher

Change-Id: I03b313a511ca9b8745213e89edb7d2efb0b964bc

3 years agoDoxygen: Include all source files in documentation 12/77712/3
Dave Rigby [Thu, 4 May 2017 10:25:24 +0000 (11:25 +0100)]
Doxygen: Include all source files in documentation

The Doxygen configuration explicilty listed specific subdirectories of
src/ to index. These were (a) outdated and (b) unnecessarily specific
- anything under src/ we want to index.

Update the Doxyfile to recusrively index all of src/

Change-Id: I5ab083c1ffc50d0ae44ae0fefdb0bbffa09fba1e
Reviewed-on: http://review.couchbase.org/77712
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
3 years agoMB-24055: Allow HashtableResizerTask interval to be customized 11/77711/3
Dave Rigby [Thu, 4 May 2017 10:21:55 +0000 (11:21 +0100)]
MB-24055: Allow HashtableResizerTask interval to be customized

Add a new entry to configuration.json: ht_resize_interval. This
controls how often the HashtableResizerTask should be scheduled to
check if the HashTables need resizing.

Change-Id: Icf312269079956a52bbe1ccdce358804839c4ff9
Reviewed-on: http://review.couchbase.org/77711
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
3 years agoDefragmenterTask: Don't wait task to complete before shutdown 86/76486/5
Dave Rigby [Fri, 7 Apr 2017 17:16:56 +0000 (18:16 +0100)]
DefragmenterTask: Don't wait task to complete before shutdown

Fix a bug in the constructor arguments which would mean that the
DefragmenterTask::completeBeforeShutdown was incorrectly defaulted (to
true).

Change-Id: I2e994d02f231ef5e19dddb9b1e8543329da8e4b7
Reviewed-on: http://review.couchbase.org/76486
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
3 years agoMB-24066/MB-22178: Set opencheckpointid to 1 after rollback 37/77837/3
Daniel Owen [Mon, 8 May 2017 17:00:09 +0000 (18:00 +0100)]
MB-24066/MB-22178: Set opencheckpointid to 1 after rollback

An opencheckpoint of 0 has the special meaning of being in a backfill
phase.

Therefore after performing a rollback we must ensure the
opencheckpointid is reset to 1.  This is the value used when the
vbucket is originally intialised.

Change-Id: I6b97faa7b502406961a4b48ad7affdbf6bc30512
Reviewed-on: http://review.couchbase.org/77837
Well-Formed: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24066: Partial revert "MB-22178: Don't use opencheckpointid to determine if in... 04/77604/6
Daniel Owen [Tue, 2 May 2017 12:46:50 +0000 (13:46 +0100)]
MB-24066: Partial revert "MB-22178: Don't use opencheckpointid to determine if in backfill phase"

Revert functional changes in
Change-Id: Ia977d6bf90e54fd1ceb8db4a9088b19d94d4bc8c,
which although addressed the rollback bug described in MB-22178, caused
the bug described in MB-24066.

The tests added in Ia977d6bf90e54fd1ceb8db4a9088b19d94d4bc8c remain but
the rollback test has been disabled as it attempts a streamRequest when
the vbucket has a opendcheckpointid of 0.

The test will be re-enabled and modified with a follow-up patch that
addresses the bug described in MB-22178.

Change-Id: Ifd11c77a10e4ebe571c50e5d518403b423c3dfa5
Reviewed-on: http://review.couchbase.org/77604
Well-Formed: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-23714: Move new task so it's in priority order 58/77458/5
Jim Walker [Thu, 27 Apr 2017 19:07:48 +0000 (20:07 +0100)]
MB-23714: Move new task so it's in priority order

A related commit added a new task, but not in the correct order.

Change-Id: Ic95b10eb8bb4563cbb5dfc9ce35e4fd808158755
Reviewed-on: http://review.couchbase.org/77458
Reviewed-by: Dave Rigby <daver@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24221: Don't fetch deleted values as part of get_if 46/77746/3
Sriram Ganesan [Thu, 4 May 2017 23:22:13 +0000 (16:22 -0700)]
MB-24221: Don't fetch deleted values as part of get_if

The get_if doesn't support deleted values. Hence, it shouldn't be
part of the options.

Change-Id: I12270a8e61ec7a04b3d166626eb534736e7e403e
Reviewed-on: http://review.couchbase.org/77746
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoFix cbstats tasks incorrect key 57/77657/2
James Harrison [Wed, 3 May 2017 09:51:44 +0000 (10:51 +0100)]
Fix cbstats tasks incorrect key

Change-Id: I4c2d3551a2b2a3f1ca213bf3cd9b067140f6e67a
Reviewed-on: http://review.couchbase.org/77657
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoMB-23211: Allow the expiry callback to carry full item 61/75861/37
Sriram Ganesan [Tue, 28 Mar 2017 09:36:22 +0000 (11:36 +0200)]
MB-23211: Allow the expiry callback to carry full item

The expiry callback needs to carry the whole item so that in the
case of full eviction, the system xattrs can be retained in the
body after deleting the rest of the body

Change-Id: Id3cb613217f4882a0f0400c01318bb2efc58b8aa
Reviewed-on: http://review.couchbase.org/75861
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoRemove unnecessary #include "tasks.h" 98/77498/2
Dave Rigby [Fri, 28 Apr 2017 17:28:36 +0000 (18:28 +0100)]
Remove unnecessary #include "tasks.h"

tasks.h is a relatively expensive header to include, as it contains
the definitions of various disjoint Task types, so in turn it includes
a large proportion of ep-engine headers.

However, it is unneessarily included in a large number of places. This
is probably at last in part due to the fact that globaltask.h didn't
used to be it's own header, and hence to get the GlobalTask definition
one needed to include tasks.h

Change-Id: I0971eab4808ce51d470fe061c13796d18b444234
Reviewed-on: http://review.couchbase.org/77498
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24089: Handle DCP backfill buffers smaller than item size correctly 04/77304/7
Manu Dhundi [Wed, 26 Apr 2017 00:53:16 +0000 (17:53 -0700)]
MB-24089: Handle DCP backfill buffers smaller than item size correctly

When items are drained from DCP readyQ, we free up the DCP backfill
buffer (that is, we decrement its memory usage). This commit addresses
2 bugs in that:
1. We never set the backfill buffer full status to false when the
   next read size is larger than max buffer size. This can result in
   backfill hangs with items larger than backfill buffer size.
2. We may never set the backfill buffer full status to false when
   buffer.bytesRead == (buffer.maxBytes * 3 / 4) == 0. In practice
   this may never happen as buffer.maxBytes is generally > 4 :).

This commit has fix for both 1 and 2. Also adds a functional test
and a unit test reproducing the bug 1.

Change-Id: Icf9512bbe6f21296374958b69cfbe851ec8873b3
Reviewed-on: http://review.couchbase.org/77304
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-23885: Add RBAC auth to cbcompact 81/76781/6
James Harrison [Thu, 13 Apr 2017 16:46:32 +0000 (17:46 +0100)]
MB-23885: Add RBAC auth to cbcompact

Change-Id: Icd2bf20a7cae844363418ee12d46c1bc56f74372
Reviewed-on: http://review.couchbase.org/76781
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Matt Carabine <matt.carabine@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoEnable xerror in python tooling 50/77450/2
James Harrison [Thu, 27 Apr 2017 16:12:23 +0000 (17:12 +0100)]
Enable xerror in python tooling

A number of errors are explicitly handled and give reasonable errors,
but the remaining exceptions will now print the associated message from
the error map too. Errors which would previously lead to being
disconnected as the client would not understand are now received and
raised appropriately.

Change-Id: I69205b105d066f2899f7152969e96065682b0708
Reviewed-on: http://review.couchbase.org/77450
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: James Harrison <james.harrison@couchbase.com>
3 years agoMB-24141: Use correct unit for slowTask recording 48/77448/2
Dave Rigby [Thu, 27 Apr 2017 15:18:53 +0000 (16:18 +0100)]
MB-24141: Use correct unit for slowTask recording

Change-Id: I388eb43fea01f8b79f5a122afa2c68757736fb81
Reviewed-on: http://review.couchbase.org/77448
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
3 years agoBasicLinkedList::operator<<: Count elements in seqlist 65/77265/2
Dave Rigby [Fri, 21 Apr 2017 16:07:46 +0000 (17:07 +0100)]
BasicLinkedList::operator<<: Count elements in seqlist

Change-Id: I3b66dee8122d54aa328941982700b6d07e5daa99
Reviewed-on: http://review.couchbase.org/77265
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
3 years agoMB-23734: Do memory mgmt across backfills in Ephemeral backfills 02/77402/5
Manu Dhundi [Wed, 26 Apr 2017 23:19:36 +0000 (16:19 -0700)]
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>
3 years agoMB-23714: Make VBucketPtr deletion schedule the delete of the VBucket 00/77100/14
Jim Walker [Thu, 20 Apr 2017 16:06:46 +0000 (17:06 +0100)]
MB-23714: Make VBucketPtr deletion schedule the delete of the VBucket

Using the std::shared_ptr Deleter this patch hooks into the deletion
of the VBucket so that a specific AUXIO (for persistent buckets) or a
NONIO (for ephemeral buckets) task performs the removal of VBucket
resources. This to ensure that no front-end thread runs the removal of
memory and more specifically the deletion of the disk data.

The aim of this commit is also to ensure there's no "race" between a
background task deleting the VB disk file when the VB has itself been
recreated. This is achieved now by having the creation of a VB to
increment the revision of the file and ensuring a delete can only
delete the older revision of the VB.

This has a few subtle changes, as only compaction or creation are
moving the revision forward, no code paths move the revision back.

1. A flush of the bucket which calls KVStore::reset would previously
return all vbuckets to a revision of 1. Again to ensure a delete task
cannot delete a "live" file, the flush doesn't reset the revision
number. So a flush of the bucket will result in deleted data, but
with x.couch.4 etc...

2. A delete of an individual vbucket has the same change. Previously
after a delete, the revision was reset to 1. This is no longer the
case and after the delete completes the revision is left unchanged.

Change-Id: I40d2f5fd658d9f8dd28a671028544831518a90d0
Reviewed-on: http://review.couchbase.org/77100
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years ago[Test Code]: Teardown StreamTest class correctly 44/77344/5
Manu Dhundi [Tue, 25 Apr 2017 21:31:41 +0000 (14:31 -0700)]
[Test Code]: Teardown StreamTest class correctly

During the teardown, only cleanup the objects that were created.

Change-Id: I065ee367013d9f9c8faafe484c13de34003fb8c5
Reviewed-on: http://review.couchbase.org/77344
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years ago[Test Code]: Run basic backfill unit test for persistent bucket as well 40/77340/4
Manu Dhundi [Wed, 26 Apr 2017 17:05:39 +0000 (10:05 -0700)]
[Test Code]: Run basic backfill unit test for persistent bucket as well

The test code was trying to run flush vbucket for persistent buckets
while a flusher task was already running and hence resulting in a
temp fail of flusher.

This has been fixed by not running the flusher by explicit call.
Rather the test waits for the flusher to complete.

Change-Id: Ic77cdb518aae388baa24187ee2cf588c6bb1a609
Reviewed-on: http://review.couchbase.org/77340
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoMB-24034 [Ephemeral]: Fix incorrect NumDeletedItems after un-delete 86/77186/3
Dave Rigby [Fri, 21 Apr 2017 15:50:31 +0000 (16:50 +0100)]
MB-24034 [Ephemeral]: Fix incorrect NumDeletedItems after un-delete

In an Ephemeral bucket, if an item is created, deleted, and then
re-created, the numDeletedItem count in the SeqList is incorrect - we
fail to decrement the deleted item count when it's re-created.

Change-Id: Iba9b77be4814ebd81f252c37e4c934c65965532f
Reviewed-on: http://review.couchbase.org/77186
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
3 years agoMB-23936: Use Threadlocal variables to accumulate stats 54/76854/10
Premkumar Thangamani [Tue, 25 Apr 2017 18:19:56 +0000 (11:19 -0700)]
MB-23936: Use Threadlocal variables to accumulate stats

Currently when we allocate/deallocate memory, we update the per bucket
variable `totalMemory`. Mutiple threads contend on this variable heavily
as mem allocation/deallocation happen often. The primary idea of
this commit is to maintain threadlocal mem counters for each bucket and
merge it to the `totalMemory` once the local counter reaches a threshold
either based on size or no.of times the local counter has been updated.

Performance Improvement (30%-35%):
--------------------------------
- Tests were run on a cluster of 2 nodes spec: 40 core/2.2 Ghz/64G memory
- On a high throughput read heavy test of 256B values we noticed a
  [30%] increase in total ops (3.5M ops/s)
- On a similar write heavy test, we noticed a [35%] increase in ops (1.9M ops/s)

Limitations:
-----------
- We create one thread local variable per bucket. Different OS'es seem
  to enforce different limits on the no.of tlv. Although we have a hard
  limit of 10 buckets, I'm noting this here for future reference.
  -> [NetBSD:256 Linux:1024 OSX:512 Windows:1088]

- Because we merge local mem stats after certain thresholds are reached, there
  might be some small delays in reflecting the precise memory usage of a
  bucket. On an active system, the delay should be minimal and not noticeable.

- Windows does not seem to provide an api for releasing the mem
  allocated for a thread-local on thread exit in a way like pthreads do.
  So there will be a small leak of about (3*long) on every bucket
  unload on each thread.

- Valgrind might say that the unit tests have leaks on the
  threadlocals. This is because, in the tests, bucket init happens on
  the main thread & when main thread exits the program it does not
  call pthread_exit, so the dtors are not called.

Change-Id: Id14ced2776a29afae18831b372140dd028136b32
Reviewed-on: http://review.couchbase.org/76854
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoMB-16181: Make ActiveStream track the current collections separator 02/75502/9
Jim Walker [Fri, 21 Apr 2017 18:31:42 +0000 (19:31 +0100)]
MB-16181: Make ActiveStream track the current collections separator

This patch updates the ActiveStream so that it stores a copy of the current
separator and tracks changes to as they are transmitted through the checkpoint
via the separator changed SystemEvent.

Change-Id: Ie3ea87d006b0bbab3e0edd8895a4756c7c5d9fe8
Reviewed-on: http://review.couchbase.org/75502
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24034: VBucketTest: addOne/setOne to return Add/MutationStatus 85/77185/4
Dave Rigby [Fri, 21 Apr 2017 15:36:11 +0000 (16:36 +0100)]
MB-24034: VBucketTest: addOne/setOne to return Add/MutationStatus

Change VBucketTest::addOne() & setOne() to return the result of the
operation, instead of passing in the expected value and having the
function itself (addOne/setOne) check it.

This was done for two reasons:

1. Sometimes addOne() is used in setup code, where an ASSERT_EQ() is
   more appropriate to check the result.

2. If the EXPECT does fail, then in the previous code the GTest error
   message would point at the implementation of addOne(), which
   generally isn't very helpful - normally you want to see the
   call-site of addOne.

Change-Id: Id478fdf2a96002503b75d6bd40edb62d869d46bc
Reviewed-on: http://review.couchbase.org/77185
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-23994: Update deletedTime when deleted SV is re-deleted 88/76488/11
Dave Rigby [Fri, 7 Apr 2017 14:11:39 +0000 (15:11 +0100)]
MB-23994: Update deletedTime when deleted SV is re-deleted

When re-deleting an already deleted item (for example when the deleted
item has a body and that body is changed), the DeletedTime was not
updated.  As such, it may be purged too early - the purge age should
be based on the last modification of the item.

Fix by ensuring the deletedTime is updated whenever the item is
(re)deleted, not just the first time.

Change-Id: I8d901cd82720597235f1400dcb2c88643ff7ed10
Reviewed-on: http://review.couchbase.org/76488
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
3 years agoMB-24055 [Ephemeral]: disable Bloom filter 69/77269/7
Dave Rigby [Mon, 24 Apr 2017 15:21:39 +0000 (16:21 +0100)]
MB-24055 [Ephemeral]: disable Bloom filter

The Bloom filter is still enabled for Ephemeral buckets, however this
is a waste of memory; as it serves no purpose for Ephemeral buckets.

Disable it - as well as saving any maintenance cost, it also reduces
the RSS of an empty Ephemeral bucket from:

    57800 KB
to:

    44632 KB

Change-Id: I430ec84dbb7ff795141500aa76329b7ae5f5cd17
Reviewed-on: http://review.couchbase.org/77269
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-23999: Perform CAS check only if item being stored is alive 17/77217/6
Sriram Ganesan [Fri, 21 Apr 2017 23:15:15 +0000 (16:15 -0700)]
MB-23999: Perform CAS check only if item being stored is alive

If the existing document is expired, then storing another
deleted document with a CAS results in a ENOENT instead of
returning EEXISTS. The CAS check on an expired document is only
applicable if the incoming document is not in Deleted state

Change-Id: Ib6b78dd50236770a6be27a5fe341e321ef4eaec2
Reviewed-on: http://review.couchbase.org/77217
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24055: Reduce HashTable::defaultNumBuckets from 1531 to 3 66/77266/5
Dave Rigby [Mon, 24 Apr 2017 15:08:13 +0000 (16:08 +0100)]
MB-24055: Reduce HashTable::defaultNumBuckets from 1531 to 3

The variable HashTable::defaultNumBuckets configures the number of
HashTable slots to use by default; assuming ns_server doesn't specify
a value in the bucket config. This was previously 1531. Given we
normally have 1024 vBuckets, this creates 1.51M slots in all the
HashTables (assuming no value from ns_server).

For small buckets this adds a lot of unnecessary overhead -
particulary comparing Ephemeral to memcache buckets, where their empty
size (single node) is:

    Bucket             RSS (KB)
    <none>             40096
    memcache           40260
    Ephemeral          87504

This first needs to be changed in ns_server (see linked patch) so it
no longer specifies the default (previously 3079); however once that
is done we still need to reduce the ep_engine default.

This patch reduces it down to the minimum of 3. Note that's still 3072
slots by default per bucket; and:

a) This doesn't limit how many Docuements can be stored, on Hash
   collision we simply use internal chaining.

b) The HashTableResizer task runs periodically to select the the
   'optimum' size, so we'll quickly adjust to larger sizes.

After this change, the empty size of an Ephemeral bucket is reduced by
30MB:

    Bucket             RSS (KB)
    Ephemeral(size 3)  57800

Change-Id: I94ce68cf3fbc2dfe70690fe8b18fc0dbada0848d
Reviewed-on: http://review.couchbase.org/77266
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-23711: [Ephemeral] Allow pageOut of Deleted-with-value document 28/76128/14
Dave Rigby [Fri, 31 Mar 2017 13:58:38 +0000 (14:58 +0100)]
MB-23711: [Ephemeral] Allow pageOut of Deleted-with-value document

A Deleted-with-value should be able to be paged out under Ephemeral
buckets (if we're low on space we should be able to remove the deleted
body), however this currently fails as it is not permitted to delete
an item which is already marked as deleted.

Given the semantics of Deleted documents are slightly different now we
have Deleted Bodies, we /should/ be able to delete something which has
a deleted value.

Update StoredValue and HashTable to correctly set the deleted flag in
this case, and correct the count of items in the HashTable.

Change-Id: I9bba6fb5779b82b16fa0a6b3bac7ccf468c4c47f
Reviewed-on: http://review.couchbase.org/76128
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoDCP Backfill: Use size_t instead of uint32_t to record mem usage 70/77270/2
Manu Dhundi [Mon, 24 Apr 2017 16:21:04 +0000 (09:21 -0700)]
DCP Backfill: Use size_t instead of uint32_t to record mem usage

We keep a record of DCP backfill memory usage to ensure that backfill
memory usage is bounded by a finite size (backfill buffer size).

We must consistently use size_t instead of uint32_t so that there is
no overflow error.

Change-Id: I01d0548b7d1cf3081b11bf96aec7868956c0bb6f
Reviewed-on: http://review.couchbase.org/77270
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-23987: Ignore repeatedly scheduled tasks 35/77035/9
James Harrison [Thu, 20 Apr 2017 09:49:38 +0000 (10:49 +0100)]
MB-23987: Ignore repeatedly scheduled tasks

A timeout was observed in a test when running against an ephemeral
bucket.

This arose during tear down from a deadlock between
ExecutorPool::_unregisterTaskable and ExecutorPool::_cancel.

Normally, this should not be possible, as _unregisterTaskable waits for
the task locator to be empty after cancelling each of the tasks,
indicating that all tasks have been taken up by a thead and cleaned up.

The problem developed from the ItemPager as used in ephemeral buckets.
KVBucket::enableItemPager cancels and schedules the same task object.
Cancelling a task simply marks it as dead; it must later be cleaned up
by a ExecutorThread.

ExecutorPool::_schedule was altered to reset dead tasks to running to
facilitate enableItemPager. Therefore, if a thread did not clean up the
task between the cancel and schedule, enableItemPager reduced down to
just a schedule. If the task was already scheduled, this would be a
duplicate schedule.

Duplicate scheduling does not adversely affect the taskLocator as it is
a map. However, the taskQueues /will/ have a second copy of the task
added.

Now, when unregisterTaskable finds the taskLocator to be empty, it is
still possible for a thread to take up the duplicate copy of the task.
The task is already marked dead, and so the thread immediately calls
_cancel to clean it up.

if _unregisterTaskable already holds the mutex, _cancel will block.
_unregisterTaskable will then join all the ExecutorThreads, deadlocking
when it reaches the thread blocked on _cancel.

Change-Id: Ia0dc7e8e8c3423e1098c71cf376653b23b4393e6
Reviewed-on: http://review.couchbase.org/77035
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years ago[Ephemeral]: MB-23734: Handle DCP backfill failures 15/77215/5
Manu Dhundi [Fri, 21 Apr 2017 22:56:59 +0000 (15:56 -0700)]
[Ephemeral]: MB-23734: Handle DCP backfill failures

When a DCP backfill task is run there could be failures. We must handle
the failures gracefully.

This commit handles any failures in DCP backfill in Ephemeral buckets.
Upon a failure we close the stream and the DCP client can retry at
a later time.

Change-Id: I2aeffb9baf7d5a8ac367b129470741af7806e710
Reviewed-on: http://review.couchbase.org/77215
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-23905: getWithMeta doesn't need to bgFetch for datatype 82/77182/2
Jim Walker [Fri, 21 Apr 2017 09:41:38 +0000 (10:41 +0100)]
MB-23905: getWithMeta doesn't need to bgFetch for datatype

Some more code left-over from when datatype was part of the value
is now removed. getMeta only needs to perform a meta-data fetch
and have no special case for datatype requests.

A test is added which recreates what happened to trigger the MB.
A getMeta was returning key_enoent instead of the datatype because
it was doing a full bgFetch against a deleted value.

Change-Id: I6715d789f6cb8503cd44b860fd78ae3224d9bc67
Reviewed-on: http://review.couchbase.org/77182
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23875: Remove old (now unused) gat/touch impl 32/77032/5
Trond Norbye [Wed, 19 Apr 2017 09:00:07 +0000 (11:00 +0200)]
MB-23875: Remove old (now unused) gat/touch impl

Change-Id: Ib85c910eb3a298e284951d3733abfa494d21f2ff
Reviewed-on: http://review.couchbase.org/77032
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23875: Implement the engine API for GAT 31/77031/6
Trond Norbye [Thu, 13 Apr 2017 13:48:00 +0000 (15:48 +0200)]
MB-23875: Implement the engine API for GAT

Change-Id: If8b40447f72089413e7a89b5acdebde3734c179e
Reviewed-on: http://review.couchbase.org/77031
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-23994: Move StoredValue::setValue() to .cc 83/76783/6
Dave Rigby [Thu, 13 Apr 2017 17:13:43 +0000 (18:13 +0100)]
MB-23994: Move StoredValue::setValue() to .cc

Refactoring in preparation for the actual code fix in next patch.

Change-Id: Id320ce729a1529f42c0ae3a6fbdedb69a84ad9d4
Reviewed-on: http://review.couchbase.org/76783
Reviewed-by: James Harrison <james.harrison@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoEPEngine:initialize: Log the config string from ns_server 53/76553/4
Dave Rigby [Tue, 4 Apr 2017 15:42:58 +0000 (16:42 +0100)]
EPEngine:initialize: Log the config string from ns_server

Aids in debugging / testing.

Change-Id: I871629639c54f8efd202634ad804b9bfd607495f
Reviewed-on: http://review.couchbase.org/76553
Reviewed-by: James Harrison <james.harrison@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23535 [Ephemeral]: Return 0 for OBSERVE_SEQNO::persisted_seqno 15/77115/5
Dave Rigby [Thu, 20 Apr 2017 10:54:35 +0000 (11:54 +0100)]
MB-23535 [Ephemeral]: Return 0 for OBSERVE_SEQNO::persisted_seqno

Ephemeral buckets (which have no persistence) should not return a
last_persisted_seqno in the OBSERVE_SEQNO response - that would be
inaccurate (and misleading) to applications which are attempting to
verify that an item has been persisted to disk. Instead zero should be
returned.

However, *wihin* the cluster we still need to report the highest
sequence number a vBucket is up to for things like DCP takeover, and
currently both of these use-cases use the same method -
getPersistenceSeqno().

To support both use-cases, add a new getPublicPersistenceSeqno()
method which is used when reporting persistence to clients. For EP
buckets this is identical to the original getPersistenceSeqno()
method, but for Ephemeral it always returns zero.

Update OBSERVE_SEQNO tests as appropriate so they check for the
correct sequence number based on the bucket type.

Change-Id: I2db32bd0747e45a749bf964d2152bdfe79d1e3d2
Reviewed-on: http://review.couchbase.org/77115
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoRemove duplicate ItemPager construction 10/77110/3
James Harrison [Thu, 20 Apr 2017 09:26:19 +0000 (10:26 +0100)]
Remove duplicate ItemPager construction

The itemPager is already constructed in the initialize method of the
superclass, KVBucket.

Change-Id: Ifeff3c7905d36ecc45f1c9ac2a6f7204eb1a5fa1
Reviewed-on: http://review.couchbase.org/77110
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoStreamline configuration.h 75/76475/13
James Harrison [Fri, 7 Apr 2017 13:08:18 +0000 (14:08 +0100)]
Streamline configuration.h

Move more out of configuration.h into configuration_impl.h as
configuration.h is large and included in a number of places.

Change-Id: I099ab79253372675f70d677c13e73a2d9c59d74f
Reviewed-on: http://review.couchbase.org/76475
Reviewed-by: David Haikney <david.haikney@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23535: Update CMD_OBSERVE tests for Ephemeral buckets 66/76966/3
Dave Rigby [Tue, 18 Apr 2017 17:14:45 +0000 (18:14 +0100)]
MB-23535: Update CMD_OBSERVE tests for Ephemeral buckets

CMD_OBSERVE is still valid for Ephemeral buckets, with two caveats:

1. It will never return a value of OBS_STATE_PERSISTED for the
   `persisted` field - instead it will return OBS_STATE_NOT_PERSISTED
   for non-deleted, valid items.

2. Items which have been deleted but not yet purged with return
   OBS_STATE_LOGICAL_DEL instead of OBS_STATE_NOT_FOUND.

As such, re-enable the existing CMD_OBSERVE tests which were
previously disabled for Ephemeral buckets, and update based on the two
above differences.

Change-Id: I3a44531abf7cfd0bfad8fbd89a76f4cad94133fb
Reviewed-on: http://review.couchbase.org/76966
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoDEBUG: Add HashTable::operator<<, expand StoredValue::operator<< 87/76187/16
Dave Rigby [Fri, 31 Mar 2017 17:41:19 +0000 (18:41 +0100)]
DEBUG: Add HashTable::operator<<, expand StoredValue::operator<<

Also fix some formatting on the existing output. Example from dump():

  HashTable[0x104cd4010] with numInMemory:3 numDeleted:2 values:
    OSV @0x104cd1e30 ... WDN.. temp:    seq:5 rev:2 key:"ns:0 1" exp:0 vallen:0
    OSV @0x104cd1de0 ... WDN.. temp:    seq:4 rev:2 key:"ns:0 0" exp:0 vallen:0
    OSV @0x104cd1e80 ... W.N.. temp:    seq:3 rev:1 key:"ns:0 2" exp:0 vallen:2 val:"2"

Change-Id: I8794f0565bf8280d6019443329b5c04730722cb6
Reviewed-on: http://review.couchbase.org/76187
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agodeleteWithMeta to manage XATTR documents 78/76478/11
Jim Walker [Fri, 7 Apr 2017 15:57:42 +0000 (16:57 +0100)]
deleteWithMeta to manage XATTR documents

When deleting a document with XATTRs, system XATTRs are retained.

Change-Id: Icde8ac48466359ee57b946b6aea39b66466990ad
Reviewed-on: http://review.couchbase.org/76478
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23263: DefragmenterVisitor: Check blob refcount 58/76958/10
olivermd [Wed, 19 Apr 2017 08:55:34 +0000 (09:55 +0100)]
MB-23263: DefragmenterVisitor: Check blob refcount

This patch adds the facility to check the refcount value of a
SingleThreadedRCPTR and uses this to ensure that the refcount of the
blob is less than 2 before the defragmenter reallocates it. This is to
ensure that we do not end up just creating a copy of the blob thus
increasing memory usage which would be the opposite of what you would
expect from the defragmenter.

It's worth noting that it appears as though something could create
another reference to the blob without holding the hashtable lock. This
could lead to a soft data race on the refcount. This means that we
cannot give a hard guarantee that the defragmenter doesn't duplicate the
blob, just that it will in most cases. The case where it won't will be
where the other thing creating the reference doesn't hold the hash
bucket lock and happens to create a reference after refcount is read but
before the blob is reallocated, which seems rare.

Change-Id: I3a8b8812ac039445451952384cb5b40ce8b433cb
Reviewed-on: http://review.couchbase.org/76958
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23263: Refactor defragmentor memory test 63/76963/5
olivermd [Tue, 18 Apr 2017 16:45:20 +0000 (17:45 +0100)]
MB-23263: Refactor defragmentor memory test

This patch moves the processes of inserting documents in to a vbucket
and fragmented said vbucket in to seperate helper methods as these will
be used in future patches.

Change-Id: I41cc0936e39a026621ff0fc06f3b398fed71b7ce
Reviewed-on: http://review.couchbase.org/76963
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23976: Return updated CAS from TOUCH command 61/76961/4
Dave Rigby [Tue, 18 Apr 2017 16:53:16 +0000 (17:53 +0100)]
MB-23976: Return updated CAS from TOUCH command

The TOUCH command was incorrectly returning the CAS *before* the
document was updated, instead of the CAS value after the update.

Change-Id: I9f6b1dd08c67f09d3191c4d3061d63c96d894af7
Reviewed-on: http://review.couchbase.org/76961
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23714: Use a weak_ptr in DcpBackfillMemory task 58/76558/9
Jim Walker [Mon, 10 Apr 2017 15:05:24 +0000 (16:05 +0100)]
MB-23714: Use a weak_ptr in DcpBackfillMemory task

Ensures there can be no cyclic dependency with VB pointers in the
complex DCP slab of objects and tasks.

Change-Id: I89d902ff0aa0c69dcd598653ae40cda34ea582f9
Reviewed-on: http://review.couchbase.org/76558
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23714: Make VBucketPtr a std::shared_ptr 57/76557/10
Jim Walker [Wed, 12 Apr 2017 13:39:18 +0000 (14:39 +0100)]
MB-23714: Make VBucketPtr a std::shared_ptr

Change the VBucketPtr definition so that VBucket poiners are managed
using std::shared_ptr.

To enable some functions to turn this* into a VBucketPtr, VBucket
inherits std::enable_shared_from_this. The only current user of this
is EphemeralVBucket where it constructs the DCPBackfillMemory with a
shared pointer to itself. Arguably EphemeralVBucket could be the class
to inherit std::enable_shared_from_this, but giving the base-class the
shared_from_this method seems more flexible for future enhancement.

Change-Id: Id2f39ece3983509b5c6742107de56b1dcba2d844
Reviewed-on: http://review.couchbase.org/76557
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoRestore cbstats and cbepctl no auth default bucket access 80/76780/3
James Harrison [Thu, 13 Apr 2017 16:33:33 +0000 (17:33 +0100)]
Restore cbstats and cbepctl no auth default bucket access

This was briefly not possible

Change-Id: Ie84f3bd99b59fcf12fe2fbc47440ef6a1ab4b858
Reviewed-on: http://review.couchbase.org/76780
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoGeneralise cli_auth_utils 79/76779/3
James Harrison [Thu, 13 Apr 2017 15:19:33 +0000 (16:19 +0100)]
Generalise cli_auth_utils

Some minor modifications to allow arbitrary kwargs to be passed through
the cmd decorator - permits use in cbcompact

Change-Id: Ib506b29d1aa8b8cabc28ddad91024294632c0a63
Reviewed-on: http://review.couchbase.org/76779
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23906: Delete-with-value: Fix count of deleted items 20/76120/15
Dave Rigby [Fri, 31 Mar 2017 10:46:13 +0000 (11:46 +0100)]
MB-23906: Delete-with-value: Fix count of deleted items

Prior to Spock, the valid state transitions for StoredValues (relating
to deletion) were:

    /---\
    |   |
    |   V
    Alive --------> Deleted
          <--------

With the advent of Deleted Bodies for system XATTRs, Deleted SVs can
optionally have a value (Blob) associated with them, which can be
changed (or removed) without going back to Alive:

    /---\              /---\
    |   |              |   |
    \   V              \   V
    Alive -------->   Deleted   ----------> Deleted
      ^   <--------  with-value <---------- no-value
      |                                         |
      |                                         |
      \-----------------------------------------/

Given that Deleted-with-Value and Alive both are moved-to using
updateStoredValue(), we need to handle increasing numDeletedItems in
additional code paths - specifically whenever the value changes.

Fix this in HashTable::unlocked_updateStoredValue, and add tests for
these cases to the Delete-with-value tests.

Change-Id: Id9224ed0e66d0298c0f73621fedba3af1fe62e86
Reviewed-on: http://review.couchbase.org/76120
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23906: Implement delete-with-value with store() instead of delete() 19/76119/17
Dave Rigby [Wed, 29 Mar 2017 15:15:03 +0000 (16:15 +0100)]
MB-23906: Implement delete-with-value with store() instead of delete()

During investigation of issues with 'fully' deleting documents
(i.e. discarding their bodies) which are in state Delete-with-value,
it was found that there's two bugs which interact to allow some
transitions between Deleted states, but not others. As a result
Deletes are not always handled correctly.

Prior to Spock, the valid state transitions for StoredValues (relating
to deletion) were:

              /---\
              |   |
              |   V
              Alive -----------------\
                ^                    |
                |                    V
                \---------------- Deleted
                                 (no value)

With the advent of Deleted Bodies for system XATTRs, Deleted SVs can
optionally have a value (Blob) associated with them, which can be
changed (or removed) without going back to Alive:

              /---\
              |   |
              |   V
              Alive -----------------\
                ^                    |
                |                    V
                \---------------- Deleted <------------\
                           (with-value or no-value)    |
                                     |                 |
                                     \-----------------/

While the above transitions should be supported, StoredValue was not
fully updated to allow transitioning between the two Deleted variants.
Specifically, if an item had the `deleted` flag set, then calling
StoreValue::del() while in the Deleted-with-Value state became a no-op
- the value was not removed:

    void del(HashTable &ht) {
        if (isDeleted()) {
            return;
        }
        ...
        resetValue();  // <-- Value removed here

This means the Deleted-with-value -> Deleted-no-value transition is
broken - the deleted value was not removed.

If we want to fix this, and handle 'deleting-a-deleted-value' in
HashTable, then we need to allow the HashTable to re-delete an item
(essentially remove the early exit in StoredValue:del(). However in
doing so, it breaks *setting* the deleted value, due to a second bug
in VBucket::processSoftDelete():-

  VBucket::processSoftDelete() (which is the function which handles
  both Delete-with-value and Delete-no-value) assumes the deletion
  should have no value (it calls softDeleteStoredValue() with
  onlyMarkDeleted==false), and hence the item should have its body
  discarded. However due to the aforementioned 'bug' (early-exit) in
  StoredValue::del(), the body was kept (and Delete-with-Value ->
  Delete-with-Value transition currently works).

To resolve this, it seems simpler and cleaner to implement
Delete-with-Value as a Mutation - i.e. call down the EvpStore path
instead of EvpItemDelete. As EvpStore already passes an Item object
all the way down the call-chain to updateStoredValue(), we can simply
use the existing Item object to convey the the `del` op instead of a
`set`.

Change-Id: I3b9223da101b38e3e25aef237d0e52ee55bae623
Reviewed-on: http://review.couchbase.org/76119
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
4 years agoMove OSV::stale back to StoredValue 74/76774/5
Dave Rigby [Thu, 13 Apr 2017 13:01:56 +0000 (14:01 +0100)]
Move OSV::stale back to StoredValue

As part of MB-23795 it was necessary to move the OrderedStoredValue
{stale} flag out of the packed bitfields in StoredValue so it didn't
share its byte with any other data not guarded by writeLock. This was
to prevent any data races, as C++/x86_64 only exposes byte-level
addressing and if it remained in the bitfield we'd see races when
other elements in the bitfield (e.g. deleted) were changed.

While fixing the correctness issue, this had the consequence of
bloating the size of OrderedStoredValue by 8 bytes; as OSV was
previously an exact multiple of 8 bytes in size, and adding one more
byte for the bitfield forced the object size to increase by 8 bytes to
maintain ABI alignment rules.

However, we actually /do/ have a spare byte in StoredValue after the
bitfields. If we move the stale flag back to SV, but in its own byte
then we recover the size bloat of OSV - we are back to the original SV
and OSV sizes before MB-23795.

Change-Id: I15d299dcdd0107915c8b50c717305e2ecb9960a4
Reviewed-on: http://review.couchbase.org/76774
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
4 years agoFix cbstats timings 78/76778/3
James Harrison [Thu, 13 Apr 2017 15:15:45 +0000 (16:15 +0100)]
Fix cbstats timings

histograms was unexpectedly decorated with @cmd - despite not being a
top level cmd called by clitool.

This lead to an erroneous auth failure, no matter what.

Change-Id: I367d8869db8b151376916e726694fa8758b636d9
Reviewed-on: http://review.couchbase.org/76778
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23863: Make DCP backfill read deleted documents 09/76709/4
Jim Walker [Tue, 18 Apr 2017 09:12:31 +0000 (10:12 +0100)]
MB-23863: Make DCP backfill read deleted documents

recordDbDump is invoked for KVStore::scan (backfill) and was coded to
skip opening of the document if the docinfo says deleted.

This commit then removes the error case where
couchstore_open_doc_with_docinfo returns -5
(COUCHSTORE_ERROR_DOC_NOT_FOUND) as that just means the document has
no-value and we still need to continue to creating the Item.

Change-Id: I6e2e563ef68f9bc4404c5e59480f8c6fb2dd36e4
Reviewed-on: http://review.couchbase.org/76709
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoAdd WithMeta unit tests 81/76381/14
Jim Walker [Wed, 5 Apr 2017 15:20:00 +0000 (16:20 +0100)]
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>
4 years agoRCPtr: Add dynamic_pointer_cast 52/76552/2
Dave Rigby [Wed, 5 Apr 2017 16:46:08 +0000 (17:46 +0100)]
RCPtr: Add dynamic_pointer_cast

Allows dynamic cast from one RCPtr instantiation to another. Based on
same function for std::shared_ptr.

Change-Id: Idc8723ea90dfd2843a3c5b602f5e42fad7a36613
Reviewed-on: http://review.couchbase.org/76552
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: James Harrison <james.harrison@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
4 years agoFix typo in MockBasicLinkedList::getWriteLock 77/76777/4
Dave Rigby [Thu, 13 Apr 2017 16:14:32 +0000 (16:14 +0000)]
Fix typo in MockBasicLinkedList::getWriteLock

Change-Id: I7f7ceda0195549b89e3b317224c2eeebf80153b4
Reviewed-on: http://review.couchbase.org/76777
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23767: Notify vb high priority requests in a separate task 59/76659/12
Manu Dhundi [Thu, 13 Apr 2017 16:31:19 +0000 (09:31 -0700)]
MB-23767: Notify vb high priority requests in a separate task

Vbucket high priority requests are made by ns server during rebalance
wherein it expects a notification when the vbucket receives upto a
particular seqno.

We must send this notification in a separate task rather than in
front end thread to avoid deadlocks due to lock inversion.
That is, a front end thread generally makes call down the stack and
acquires locks in the order top to down (in our software stack).
But a notification path is up the software stack and hence must be
done as a separate task.

This commit creates a daemon task per Ephemeral Bucket to notify
vb high priority reuqests. When the task runs it notifies all the
connections to be notified as goes to sleep. When a new connection
is to be notified the task is woken again.

The current deadlock that was hit is captured in MB-23767.

Change-Id: Id114bedb5cd4de8b493ae6885734c3d440c8bea3
Reviewed-on: http://review.couchbase.org/76659
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23719: Add dynamic change of ephemeral_metadata_purge_age 41/76541/7
Dave Rigby [Mon, 10 Apr 2017 08:53:34 +0000 (09:53 +0100)]
MB-23719: Add dynamic change of ephemeral_metadata_purge_age

Add ephemeral_metadata_purge_age to setFlushParam() so its value can
be changed dynamically via SET_PARAM.

Also add a requirement on bucket_type==ephemeral, as it's not valid
for other bucket types.

Change-Id: I0fecb5ced449c672dd95a4b196bdab9f6aaa7347
Reviewed-on: http://review.couchbase.org/76541
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23795: Ephemeral Tombstone purging Task 85/76485/16
Dave Rigby [Fri, 7 Apr 2017 17:20:52 +0000 (18:20 +0100)]
MB-23795: Ephemeral Tombstone purging Task

Expands on the previous patch to implement a Task which performs
Tombstone purging. This Task is scheduled periodically (see
ep_ephemeral_metadata_purge_interval), and when run it visits all
vBuckets and purges all applicable OSVs.

Task can be reconfigured dynamically via epctl parameters:

- ephemeral_metadata_purge_age: Age in seconds after which deleted
  items are purged.

- ephemeral_metadata_purge_interval: How often should Tombstone
  purging task run to check for items to be purged.

Example output:

    NOTICE (eph) Ephemeral Tombstone Purger starting with purge age:60
    NOTICE (eph) Ephemeral Tombstone Purger completed. Purged 39000 items. Took 104ms. Sleeping for 60 seconds.

Change-Id: I126c74f2e82c0a31a2843240303548a24af2e90f
Reviewed-on: http://review.couchbase.org/76485
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23795: Ephemeral Tombstone purging (VBucket-level) 08/76408/25
Dave Rigby [Thu, 6 Apr 2017 16:14:42 +0000 (17:14 +0100)]
MB-23795: Ephemeral Tombstone purging (VBucket-level)

Add Ephemeral Tombstone purging for Ephemeral VBuckets. This patch
adds two purge operations - one for the HashTable, and one for the
sequenceList:

a) The HashTable visitor (EphemeralVBucket::HTTombstonePurger) visits
   all items in the HashTable, and identifies any deletes which are
   old enough to be purged (age > ephemeral_metadata_purge_age). Such
   items are marked as stale, and transferred from the HashTable to
   the SequenceList.

b) The SequenceList purger (BasicLinkedList::purgeTombstones()) then
   iterates over the sequencelist, and hard-deletes any items marked
   stale - both Alive and Deleted items. It is at this stage that the
   OSVs are actually deleted.

   Note the SequenceList purging is somewhat delicate, to ensure
   correctness while not blocking front-end writes. See the inline
   comments in BasicLinkedList::purgeTombstones() for further details.

Note this isn't yet driven by any tasks - only by the unit
tests. Subsequent patches will connect into tasks.

Change-Id: I937ed23317826c84cbdd0bb0b3749a99ff446497
Reviewed-on: http://review.couchbase.org/76408
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23795: Move StoredValue::stale to OSV; guard with SeqList::writeLock 15/76715/7
Dave Rigby [Wed, 12 Apr 2017 14:00:53 +0000 (15:00 +0100)]
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>
4 years agoMB-23530: Prevent replace with CAS on deleted item 61/76261/9
James Harrison [Tue, 4 Apr 2017 16:32:15 +0000 (17:32 +0100)]
MB-23530: Prevent replace with CAS on deleted item

Against an ephemeral bucket or full eviction persistent bucket, a
replace operation could erroneously succeed if the correct CAS value was
given. A replace with a CAS resolves internally to OPERATION_CAS, which
is treated in the same manner as OPERATION_SET.

Prior to this change, we checked if an item was previously deleted if
the CAS did _not_ match, but even if the CAS is /correct/ a replace with
a CAS should fail if the old item is deleted.

The set should only be rejected if the new item is /not/ deleted, to
avoid breaking the delete implementation described in
http://review.couchbase.org/#/c/76119.

Change-Id: Id1eeaeb5326b7a143c7213f07c774f02623bff8d
Reviewed-on: http://review.couchbase.org/76261
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMinor cbstats tasks improvements 34/76634/4
James Harrison [Tue, 11 Apr 2017 14:55:13 +0000 (15:55 +0100)]
Minor cbstats tasks improvements

Altered sort order to by default sort durations in descending order.

Small preparations for adding a "-r" flag to reverse.

Change-Id: I6a32d064c8bf38f677b512137bed0b0724fc8e67
Reviewed-on: http://review.couchbase.org/76634
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-22738: Move stripping of value from DCP stream back into ep-engine 18/76418/31
Daniel Owen [Thu, 6 Apr 2017 21:12:28 +0000 (22:12 +0100)]
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>
4 years agoMB-23530: Add test for CAS set after delete 26/76326/7
James Harrison [Wed, 5 Apr 2017 14:37:36 +0000 (15:37 +0100)]
MB-23530: Add test for CAS set after delete

It is erroneously possible to set an item with CAS after deleting it on
an ephemeral bucket or a full eviction persistent bucket. This breaks
the expected behaviour.

This test should identify this failure and as such can not be enabled
for these types until after the issue is resolved.

Change-Id: I5b382d10a2fa6f955645d8305282a494b5f8ba6b
Reviewed-on: http://review.couchbase.org/76326
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-23267: Prevent changing inapplicable options from cbepctl 99/76399/12
James Harrison [Thu, 6 Apr 2017 13:30:22 +0000 (14:30 +0100)]
MB-23267: Prevent changing inapplicable options from cbepctl

Check requirements in setFlushParam and setTapParam for the following
configuration parameters:

 "tap_keepalive"
 "access_scanner_enabled"
 "alog_sleep_time"
 "alog_task_time"
 "ephemeral_full_policy"

this will prevent them being set if their requirements are not met.

Change-Id: Ie70d062e5333393e12771d325d22438f5e865bdf
Reviewed-on: http://review.couchbase.org/76399
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMake set*Param methods members to allow use in tests 02/76702/3
James Harrison [Wed, 12 Apr 2017 10:55:04 +0000 (11:55 +0100)]
Make set*Param methods members to allow use in tests

Moved set*Param methods to be members of EventuallyPersistentEngine to
simplify testing the logic therein. They were previously static methods
and were not callable directly from tests.

Change-Id: I263ed94149a0142de5838556eb34799ff02c9049
Reviewed-on: http://review.couchbase.org/76702
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-22695: Add stats for datatypes 17/76617/3
olivermd [Mon, 6 Mar 2017 15:37:22 +0000 (15:37 +0000)]
MB-22695: Add stats for datatypes

This commit adds counts for datatypes of documents whose Storedvalue is
resident in memory.

Adds the datatype stats to the 'all' engine stat call. This means that
they will be added to the ETS tables in ns_server.

Introduces a regression test for MB-23892.

This patch was originally e4606e8 and was reverted by 6501c72 due to
MB-23892.

Change-Id: Ic0961af1a3c18362004369db0ff0fa7a9eeba22c
Reviewed-on: http://review.couchbase.org/76617
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-23267: Hide irrelavant config options 62/75962/14
James Harrison [Wed, 29 Mar 2017 09:42:38 +0000 (10:42 +0100)]
MB-23267: Hide irrelavant config options

Hides from stats and prevents changing through cbepctl of:
 * item_eviction_policy on ephemeral buckets
 * ep_alog_* on ephemeral buckets
 * ephemeral_full_policy on persistent buckets
 * ep_tap_* if tap is disabled

Change-Id: I70ed89b033b845ac6d02f965fd5dda9ce884185c
Reviewed-on: http://review.couchbase.org/75962
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoIntroduce testing exception for rescheduled dead tasks 51/76551/7
James Harrison [Fri, 7 Apr 2017 14:21:32 +0000 (15:21 +0100)]
Introduce testing exception for rescheduled dead tasks

In http://review.couchbase.org/#/c/76394/ the ability to reschedule a
cancelled (and now in state TASK_DEAD) GlobalTask was fixed.

it does not appear that any tasks other than the ItemPager for ephemeral
buckets are rescheduled once dead, but to avoid changing existing
behaviour this introduces an exception if any task other than the
ItemPager is rescheduled, to bring them to our attention.

NB: This patch will be reverted to remove this exception for Spock.
(MB-23797)

Change-Id: If44b7cf8a71c3dc4d85fba98d65c4f608d449460
Reviewed-on: http://review.couchbase.org/76551
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23750: Fix toggling ephemeral ejection policy 94/76394/11
James Harrison [Thu, 6 Apr 2017 12:23:38 +0000 (13:23 +0100)]
MB-23750: Fix toggling ephemeral ejection policy

If going

auto_delete -> fail_new_data -> auto_delete

the itemPager task was rescheduled, but as it had previously been
cancelled the state remained TASK_DEAD, and so ExecutorThread::run
behaves as if the task has just been cancelled and needs cleaning up.
By resetting scheduled tasks back to TASK_RUNNING if they are dead, this
can be avoided.

Change-Id: Id007a15fdaeb80a79828da0cade031a424e653cf
Reviewed-on: http://review.couchbase.org/76394
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMove defragmenter benchmarks to benchmarks 76/76476/5
olivermd [Fri, 7 Apr 2017 14:01:11 +0000 (15:01 +0100)]
Move defragmenter benchmarks to benchmarks

This moves the defragmenter benchmarks to ep_engine_benchmarks because
they can take ~18 seconds under ASAN. It also makes sense to group all
the benchmarks together.

To enable this move, this commit also puts the defragmenter test in to a
header file so that it can continued to be used by the defragmenter
tests which are not benchmarks.

Change-Id: I69f8da02c07e6469b9c80fbe06507c80d866ac7f
Reviewed-on: http://review.couchbase.org/76476
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoRefactor: Replace RCPtr<VBucket> with VBucketPtr 56/76556/3
Jim Walker [Mon, 10 Apr 2017 14:03:34 +0000 (15:03 +0100)]
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>
4 years agoMB-23829: Revert "MB-22695: Add stats for datatypes" 75/76575/3
Dave Rigby [Mon, 10 Apr 2017 17:45:26 +0000 (17:45 +0000)]
MB-23829: Revert "MB-22695: Add stats for datatypes"

Reverting to due null pointer dereference when replacing a non-resident item:

       297      if (v.getDatatype() != itm.getDataType()) {
    -> 298          --datatypeCounts[v.getValue()->getDataType()];
       299          ++datatypeCounts[itm.getDataType()];
       300      }
    (lldb) p v
    (StoredValue) $0 = {
      value = {
        value = 0x0000000000000000
      }

This reverts commit e4606e8f50797e40d3a9f7931c1e45a070f82002.

Change-Id: I5998e2eaadedf897192d0fb8aeb184ac85c4bf8f
Reviewed-on: http://review.couchbase.org/76575
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23796: Wait for flusher to avoid stat race 48/76548/2
olivermd [Mon, 10 Apr 2017 11:52:44 +0000 (12:52 +0100)]
MB-23796: Wait for flusher to avoid stat race

Under full eviction, the stat used for curr items is different to that
used under value eviction. Aditionally, under full eviction, the flusher
updates the relevant stat. This can lead to a race in the test between
the flusher and the test code. For example to test code to get the stats
may run before the flusher can, meaning that the previous operation is
not reflected in the stats, leading to the test failing a check.

Change-Id: I3ae6617fec0e0d076438bd839feaaea7633e23fd
Reviewed-on: http://review.couchbase.org/76548
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-23556: Add response stats to cbstats 55/76555/4
olivermd [Mon, 10 Apr 2017 13:33:41 +0000 (14:33 +0100)]
MB-23556: Add response stats to cbstats

Example output:

$ ./management/cbstats localhost:12000 responses
 SUCCESS: 14410

$ ./ep-engine/management/cbstats localhost:12000 responses all
 AUTH_CONTINUE:                   0
 AUTH_ERROR:                      0
 AUTH_STALE:                      0
 DELTA_BADVAL:                    0
 E2BIG:                           0
 EACCESS:                         0
 EBUSY:                           0
 EINTERNAL:                       0
 EINVAL:                          0
 ENOMEM:                          0
 ERANGE:                          0
 ETMPFAIL:                        0
 KEY_EEXISTS:                     0
 KEY_ENOENT:                      0
 LOCKED:                          0
 NOT_INITIALIZED:                 0
 NOT_MY_VBUCKET:                  0
 NOT_STORED:                      0
 NOT_SUPPORTED:                   0
 NO_BUCKET:                       0
 ROLLBACK:                        0
 SUBDOC_DELTA_EINVAL:             0
 SUBDOC_DOC_NOTJSON:              0
 SUBDOC_INVALID_COMBO:            0
 SUBDOC_MULTI_PATH_FAILURE:       0
 SUBDOC_NUM_ERANGE:               0
 SUBDOC_PATH_E2BIG:               0
 SUBDOC_PATH_E2DEEP:              0
 SUBDOC_PATH_EEXISTS:             0
 SUBDOC_PATH_EINVAL:              0
 SUBDOC_PATH_ENOENT:              0
 SUBDOC_PATH_MISMATCH:            0
 SUBDOC_SUCCESS_DELETED:          0
 SUBDOC_VALUE_CANTINSERT:         0
 SUBDOC_VALUE_ETOODEEP:           0
 SUBDOC_XATTR_INVALID_FLAG_COMBO: 0
 SUBDOC_XATTR_INVALID_KEY_COMBO:  0
 SUBDOC_XATTR_UNKNOWN_MACRO:      0
 SUCCESS:                         24909
 UNKNOWN_COLLECTION:              0
 UNKNOWN_COMMAND:                 0
 XATTR_EINVAL:                    0

Change-Id: Id75d6f19d4302a62c4c4d13274ba09a6bf8a0743
Reviewed-on: http://review.couchbase.org/76555
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-23618: Revert 25ce75aa1 bgFetch changes 40/76540/2
Jim Walker [Mon, 10 Apr 2017 08:52:17 +0000 (09:52 +0100)]
MB-23618: Revert 25ce75aa1 bgFetch changes

Revert the bgFetch changes from 25ce75aa1 as they're not required
anymore. The original commit added conditional "isMeta" parameter
changes to withMeta functions because the datatype of the document
use to be part of the body. Now that this is part of the meta data
the changes are no longer required.

This has the side-effect of fixing MB-23618, so there was also
something wrong with the bgFetch changes.

Change-Id: If3743b766b5da1b51c00e3b5ee84707f2ba4262e
Reviewed-on: http://review.couchbase.org/76540
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoReorganise EPStore tests 25/76325/5
James Harrison [Wed, 5 Apr 2017 13:48:08 +0000 (14:48 +0100)]
Reorganise EPStore tests

Separating out tests which are applicable to all of:

 EP full eviction
 EP value eviction
 Ephemeral

from those only applicable to the two former.

Change-Id: If1580b5d20b60778dd0a7d88260c80a172251b07
Reviewed-on: http://review.couchbase.org/76325
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-23267: Add ability to hide inapplicable config options 61/75961/10
James Harrison [Tue, 28 Mar 2017 12:46:12 +0000 (13:46 +0100)]
MB-23267: Add ability to hide inapplicable config options

This is expressed by declaring the requirements for a particular config
option in configuration.json e.g.,

"ephemeral_full_policy": {
    ...
    "requirements": {
        "bucket_type": "ephemeral"
    }
}

This example prevents "ephemeral_full_policy" from being listed in stats
if "bucket_type" is not "ephemeral".

Change-Id: I4c85132612f55a4edb7c5497c9744ef63efbd206
Reviewed-on: http://review.couchbase.org/75961
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-23719: Add ephemeral_metadata_purge_age setting 07/76407/6
Dave Rigby [Thu, 6 Apr 2017 16:39:51 +0000 (17:39 +0100)]
MB-23719: Add ephemeral_metadata_purge_age setting

Add the new config option "ephemeral_metadata_purge_age" to
ep-engine's configuration, along with listeners so changes in it's value
are propogated to the bucket.

Note the default value of 60s, and -1 for disabled exists mostly for
component testing - don't expect that to be exposed to
users. Similarly the unit of seconds is used to aid in testing - I'm
aware the UI is limited to a minimum of 1 hour.

This setting will be dynamic - i.e. can be changed via SET_PARAM
without restarting the bucket.

As per the name, this option is only applicable to Ephemeral buckets -
it will be ignored by Persistent Buckets.

  Implementation Note: The actual Tombstone Purger Tasks do not yet
  exist, so there's currenlty placeholder code for enable/disabling
  the task.

Change-Id: I78726a1bce8c870c70c916cae6f174ea86ef97bb
Reviewed-on: http://review.couchbase.org/76407
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
4 years agoStoredValue: record deleted time for OSVs 06/76406/6
Dave Rigby [Thu, 6 Apr 2017 16:18:35 +0000 (17:18 +0100)]
StoredValue: record deleted time for OSVs

For OrderedStoredValues, record the time when they are deleted, so
deleted OSVs can later be purged (completely deleted) when they reach
a certain age.

To avoid adding another 4bytes to OSV, the lock_expiry member is
re-used to also be used for the deleted time (deleted items cannot be
locked and hence this is otherwise unused once items enter that
state).

Change-Id: I1a6b9c5e8be4d3df1dca5c1e91be6219889686da
Reviewed-on: http://review.couchbase.org/76406
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
4 years agoMB-23712: Check for invalid cas before setting value for deleted body 52/76252/7
Dave Rigby [Tue, 4 Apr 2017 13:31:52 +0000 (14:31 +0100)]
MB-23712: Check for invalid cas before setting value for deleted body

Before setting the value for a deleted item, check to see if the
incoming cas matches the existing cas

Change-Id: If61e47f0c29ede41778f0f3d817082a83ec8f851
Reviewed-on: http://review.couchbase.org/76252
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Sriram Ganesan <sriram@couchbase.com>
4 years agoEPEngine::destroy: explain why stats are snapshotted 66/76466/3
Dave Rigby [Fri, 7 Apr 2017 08:21:44 +0000 (09:21 +0100)]
EPEngine::destroy: explain why stats are snapshotted

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