ep-engine.git
3 years agoMB-16181: Filters with deleted collections 70/78570/2 master
Jim Walker [Wed, 24 May 2017 16:10:50 +0000 (17:10 +0100)]
MB-16181: Filters with deleted collections

Upstream integration revealed an issue with the VB::Filter.
If we construct a filter with a collection "X" and "X" is actually
marked as deleted, "X" should not be part of the filter.

We fix this by replacing VB::Manifest::doesCollectionExist with
VB::Manifest::isCollectionOpen which performs the correct checks.

Tests updated to cover this case.

Change-Id: I38d4ba025805da7653bf3a84053d4280f4f547d7
Reviewed-on: http://review.couchbase.org/78570
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-23990: Fix intermittent hangs in unit test 08/78508/3
Premkumar Thangamani [Wed, 24 May 2017 02:28:14 +0000 (19:28 -0700)]
MB-23990: Fix intermittent hangs in unit test

In some scenarios, the io complete notification reached before we start
waiting for it via the condition variable. This results in the test
hanging forever. Now we track the no.of notifications and wait
accordingly

Change-Id: Id9d9404892e111d7d60760d0e9ce727b474c0997
Reviewed-on: http://review.couchbase.org/78508
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24293: Retry atmost 10 times if directory removal fails 54/78254/12
Sriram Ganesan [Wed, 17 May 2017 19:01:53 +0000 (12:01 -0700)]
MB-24293: Retry atmost 10 times if directory removal fails

The database directory is removed in order to simulate a commit
failure. The directory removal could fail for various reasons.
Retry atmost 10 times to remove the directory

Change-Id: I16e81f3572e0b7d58af3d5ee1f7849aec8cabf97
Reviewed-on: http://review.couchbase.org/78254
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoMB-24424: Pass correct parameters when making MutationResponse 02/78302/16
Daniel Owen [Thu, 18 May 2017 11:50:22 +0000 (12:50 +0100)]
MB-24424: Pass correct parameters when making MutationResponse

The isKeyOnly parameter was missing when passed into make_unique. This
patch fixes this issue.  In addition it moves the creation of a
PassiveStream into a separate function.  This enables the function that
creates a PassiveStream to be overridden with one that creates a
MockPassiveStream, which is used in testing.

Change-Id: I58e2e8ca06acba24573d1b4a53aeed85dbcecffa
Reviewed-on: http://review.couchbase.org/78302
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-16181: Add collections checks to more commands 90/78390/3
Jim Walker [Mon, 22 May 2017 09:30:20 +0000 (10:30 +0100)]
MB-16181: Add collections checks to more commands

Currently only get/set have collections validation in-place, this
needs extending to cover all "commands-paths" where a client is
operating on a key.

Change-Id: I976533788e3365fb4eb789bce6132812a4339e4c
Reviewed-on: http://review.couchbase.org/78390
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoRemove unnecessary #includes in DCP code 22/78322/4
Dave Rigby [Wed, 17 May 2017 11:58:57 +0000 (12:58 +0100)]
Remove unnecessary #includes in DCP code

Change-Id: Id6bb0258230281647f191bb25282e60a533c32de
Reviewed-on: http://review.couchbase.org/78322
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
3 years agoMB-24055: Change ht_size to 47; ht_resize_interval to 1s 92/77892/7
Dave Rigby [Tue, 9 May 2017 13:48:02 +0000 (14:48 +0100)]
MB-24055: Change ht_size to 47; ht_resize_interval to 1s

To determine a good tradeoff between initial memory usage and
rebalance speed, the following tests were performed:

    ht_size     ht_resize_interval (sec)  Rebalance time (min)  Initial RSS (MB)
    3079 (default)  60 (default)       4.5                   74.4
    769             1                       3.8                   52.2
    193             1                         4.3                   47
    47             1                         4.1                   45.5
    13             1                         4.4                   45

Based on these results a ht_size of 47 (with resize_interval==1) has
been picked - this gives rebalance time as good as current; but with a
39% reduction in initial memory usage.

Change-Id: I329e18240bace9f4905e3eb0c4d59abd97dd8cd1
Reviewed-on: http://review.couchbase.org/77892
Reviewed-by: Jim Walker <jim@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24472 [Ephemeral]: Check for null vBucket in getNumPersistedDeletes 06/78406/2
Dave Rigby [Mon, 22 May 2017 14:36:08 +0000 (15:36 +0100)]
MB-24472 [Ephemeral]: Check for null vBucket in getNumPersistedDeletes

Check the vBucket obtained from the vbmap is non-null before
dereferencing. If it is null, throw std::runtime_error.

Change-Id: I181d8b7d696386bee5b18daa8b211cbc7238d87b
Reviewed-on: http://review.couchbase.org/78406
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
3 years agoMB-16181: Add Collections Filter classes and test 37/78137/13
Jim Walker [Tue, 14 Mar 2017 15:05:40 +0000 (15:05 +0000)]
MB-16181: Add Collections Filter classes and test

Two classes exist for filtering.

Collections::Filter
Collections::VB::Filter

The idea is that a DCP producer will establish a Collections::Filter
that lives for the lifetime of the DCP producer.

As the DCP producer creates streams, a Collections::VB::Filter is
assigned to the stream which contains the real set of collections to
filter (and also the actual "filter" function).

Change-Id: I2f35b1698ce977116486a2e6940437eee25faef1
Reviewed-on: http://review.couchbase.org/78137
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-16181: Build SystemEvent keys with the collections separator 36/78136/12
Jim Walker [Thu, 23 Mar 2017 16:55:32 +0000 (16:55 +0000)]
MB-16181: Build SystemEvent keys with the collections separator

The keys were fixed as $collections::<event> but are now changed
so that the :: is the collections separator.

This allows code to split the event key if they wish using the
same code they would split document keys.

Change-Id: I48575d295f8c058a79cf208fe3c9d3a9b3c9ed15
Reviewed-on: http://review.couchbase.org/78136
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-16181: Transfer the collection name over DCP 35/78135/12
Jim Walker [Tue, 21 Mar 2017 15:15:16 +0000 (15:15 +0000)]
MB-16181: Transfer the collection name over DCP

The DCP mutation/deletion callbacks now take a collection_len field,
the data in this field will be sent over DCP streams when a client
has signalled they want collection-aware DCP.

For example "dairy::cheese" will set a collection length of 5,
default collection documents, set a collection length of 0.

Change-Id: I303d9b18bc5d0fd0968708d84e461ee59577c003
Reviewed-on: http://review.couchbase.org/78135
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-16181: Add collections.set_manifest support 36/77436/16
Jim Walker [Fri, 10 Mar 2017 16:34:47 +0000 (16:34 +0000)]
MB-16181: Add collections.set_manifest support

Add a method which will accept the new manifest and apply it to
all active vbuckets.

The latest manifest is saved in memory and also used for when any VB
is set to active

Change-Id: Ic6a339bc5af279d105b679f528ff3675d1f16ac7
Reviewed-on: http://review.couchbase.org/77436
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years ago[Fix coverity warning]: Initialize all members of RangeIteratorLL 25/78425/2
Manu Dhundi [Mon, 22 May 2017 19:05:18 +0000 (12:05 -0700)]
[Fix coverity warning]: Initialize all members of RangeIteratorLL

We must always initialize all the non static members of the RangeIteratorLL
class in the constructor.

Below is the warning reported by coverity:
*** CID 169606:  Uninitialized members  (UNINIT_CTOR)
/ep-engine/src/linked_list.cc: 430 in BasicLinkedList::RangeIteratorLL::RangeIteratorLL(BasicLinkedList&)()
424         std::lock_guard<std::mutex> listWriteLg(list.getListWriteLock());
425         std::lock_guard<SpinLock> lh(list.rangeLock);
426         if (list.highSeqno < 1) {
427             /* No need of holding a lock for the snapshot as there are no items;
428                Also iterator range is at default (0, 0) */
429             readLockHolder.unlock();
   CID 169606:  Uninitialized members  (UNINIT_CTOR)
   Non-static class member "earlySnapShotEndSeqno" is not initialized in this constructor nor in any functions that it calls.
430             return;
431         }
432
433         /* Iterator to the beginning of linked list */
434         currIt = list.seqList.begin();

Change-Id: Ie3e26d685159e24054ad94d8f04bc455c8e3c4f2
Reviewed-on: http://review.couchbase.org/78425
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoMB-24294: Stop couch-kvstore changing the file revision 24/78024/8
Jim Walker [Thu, 11 May 2017 15:38:35 +0000 (16:38 +0100)]
MB-24294: Stop couch-kvstore changing the file revision

There's a lot of code around the open paths which may open a different
file-revision to the one requested, which is the cause of MB-24294.

In summary we asked for 0.couch.6, which doesn't yet exist and the
checkNewRevNum function (now removed) would then look for 0.couch.*
files and depending on the async delete task's progress may find
0.couch.5 and direct saveDocs into that VB, which is about to be
deleted...

The main fix here is to remove checkNewRevNum, if we have a
dbFileRevMap value of n, we should open/create 0.couch.n and never
anything else.

This lead onto finding an issue where the RO instance of CouchKVStore
was relying on populating it's "copy" of the revision map via failing
to open 0.couch.0 and then looking for 0.couch.* and updating the map
from what it finds. So this is fixed by having a single dbFileRevMap
which is now referenced by the RW/RO pair.

Change-Id: I03dbb0c0a3e635397a12d85ea6092aa1312354b7
Reviewed-on: http://review.couchbase.org/78024
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24376: Stop rangeRead on invalid Seqno 53/78353/4
James Harrison [Fri, 19 May 2017 11:23:53 +0000 (12:23 +0100)]
MB-24376: Stop rangeRead on invalid Seqno

An exception is thrown if the start of the readRange is updated with a
value x < 0 || x > endSeqno, as this should not be done.

When updating an item which is currently covered by a rangeRead, the
following states occur in the seqList:

 A₁ B₂ C₃        Initial items
[A₁ B₂ C₃]       rangeRead to highSeqno

{ Write Lock

[A₁ B₂ C₃] A₋₁   ** Update; A₁ marked stale, new OSV appended.
                 NB: this is before queueDirty updates seqNo

[A₁ B₂ C₃] A₄    Seqno updated in queueDirty

}

RangeReads do not hold the write lock, and stop only upon either
reaching the end of the seqList, or reading an item with
seqno > endSeqno.
There is a brief window at the state marked ** in which a rangeRead
may try to update the start of the readRange with -1:

- rangeRead (endSeqno 3) reads C₃ normally
- proceeds to next item in the seqList
- checks if seqno > endSeqno - !(-1 > 3), don't stop yet
- update the start of the readRange with -1

This patch ends the readRange if the current item has a seqno < 0.

Change-Id: Ia1e53e4c40705b673c1f01e1b15fecbed5dff2fc
Reviewed-on: http://review.couchbase.org/78353
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoMB-24426: Use virtual destructors for some of the DCP classes 34/78334/3
Manu Dhundi [Thu, 18 May 2017 21:35:42 +0000 (14:35 -0700)]
MB-24426: Use virtual destructors for some of the DCP classes

DCP classes like 'DCPProducer', 'DCPConsumer', 'Consumer',
'ActiveStream' and 'PassiveStream' have subclasses derived from them.
It is appropriate to use virtual destructors to avoid any memory
leaks during object deletion.

Change-Id: I8b8457095b7c9914ca2c3d4b18a2251833e6c770
Reviewed-on: http://review.couchbase.org/78334
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoMB-24151: Implement memory managed backfills using Range Iterators 55/78055/9
Manu Dhundi [Wed, 17 May 2017 23:21:28 +0000 (16:21 -0700)]
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>
3 years agoMB-24430: Set the value correctly for an expired item in memory 20/78220/3
Sriram Ganesan [Wed, 17 May 2017 00:19:20 +0000 (17:19 -0700)]
MB-24430: Set the value correctly for an expired item in memory

If an item is found to be expired in the hash table, then its value
needs to be set correctly. For example, in case the item contains
xattrs, the system xattrs will be retained in the value and hence
needs to be set correctly

Change-Id: I99a54ca82feb837f530b18eb3f245a9d92f12369
Reviewed-on: http://review.couchbase.org/78220
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoMB-16181: Add DCP system-event engine hooks 35/77435/10
Jim Walker [Fri, 10 Mar 2017 10:19:50 +0000 (10:19 +0000)]
MB-16181: Add DCP system-event engine hooks

Plug methods into the engine callbacks so that we can now have
system events pushed in from memcached.

Change-Id: I064b86542b2ab98f80e33097afe8a60242fd7147
Reviewed-on: http://review.couchbase.org/77435
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24294: Clean-up couch-kvstore openDB with reset=true 07/78007/7
Jim Walker [Thu, 11 May 2017 09:02:06 +0000 (10:02 +0100)]
MB-24294: Clean-up couch-kvstore openDB with reset=true

The openDB reset flag is only used by the CouchKVStore::reset method
when it calls set-state.

This is no longer required because of the changes in MB-23714 (which
also modified CouchKVStore::reset). There is no need to move the DB
file revision back to 1, it should be monotonically incrementing.

Change-Id: I50ba9873041157923ed99f34623f93ef67527641
Reviewed-on: http://review.couchbase.org/78007
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoDecouple StoredValue and HashTable class 70/78170/7
Sriram Ganesan [Mon, 15 May 2017 22:26:46 +0000 (15:26 -0700)]
Decouple StoredValue and HashTable class

A StoredValue object doesn't necessarily have to be part of
a HashTable. Hence, decouple the two

Change-Id: I6b88a3e010494989d3c1ad938c329a230c79d91d
Reviewed-on: http://review.couchbase.org/78170
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoImprove test duration under Valgrind 83/78083/3
James Harrison [Fri, 12 May 2017 15:33:42 +0000 (16:33 +0100)]
Improve test duration under Valgrind

Compared to the parent commit, this change reduces the run time of tests
under Valgrind from 1051s (17m31) to 295s (4m55) - roughly a 72%
reduction.

This reduction comes from the teardown phase of a number of tests.
When destroying an ExecutorPool, all the taskables must be unregistered.

Prior to this change, when unregistering the last taskable from an
ExecutorPool, all threads were woken and prevented from sleeping (by
pretending there is work, numReadyTasks[idx]++) /before/ they were
marked as dead. During this time the woken threads all busy-wait for
work.

Usually this wouldn't be too problematic because the main thread would
mark them dead soon after. However, under Valgrind only one thread can
execute at a time, and it can take significant time before the main
thread is able to mark all of the ExecutorThreads as dead.

This change simply marks them dead before waking them. They check their
status, find it is "dead", and exit.

Change-Id: Ic88ccec46ac26d511a18a1370d20117af173703b
Reviewed-on: http://review.couchbase.org/78083
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoMB-16181: Make server deny creation of reserved collections 74/78074/8
Jim Walker [Fri, 12 May 2017 09:12:20 +0000 (10:12 +0100)]
MB-16181: Make server deny creation of reserved collections

The design has always stated that _ and $ would be reserved for
system use. Make sure the server enforces this by checking the
names on incoming manifests.

1. Any _ prefixed collection will cause the Manifest construction to
 throw invalid_argument.

2. Any $ prefixed collection which is not $default will cause the
 Manifest construction to throw invalid_argument.

Change-Id: I1e5daf2ae87cba2a8dbcdda4c9bb0be66e40ffae
Reviewed-on: http://review.couchbase.org/78074
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
3 years agoMB-24094: Add Item to SequenceList on TTL update 32/78032/6
Premkumar Thangamani [Thu, 11 May 2017 17:58:48 +0000 (10:58 -0700)]
MB-24094: Add Item to SequenceList on TTL update

In the GAT Path, when the item is expired, we update the expiry
time. In the case of ephemeral buckets, that item should be updated on
the sequence list.

Change-Id: I2b83456e53cb2d2e4d762d939a716c39c9a725bd
Reviewed-on: http://review.couchbase.org/78032
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
3 years agoMB-24054: Fix ref to temp string in testsuite_dcp 34/78134/4
olivermd [Mon, 15 May 2017 13:27:58 +0000 (14:27 +0100)]
MB-24054: Fix ref to temp string in testsuite_dcp

This patch ensures we pass a std::string to the initializer list rather
than doing a pointless call to c_str() which just confuses MSVC2015.

Change-Id: Idb29db461a1d2572660622d1166c680b599e2b57
Reviewed-on: http://review.couchbase.org/78134
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>
3 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>