4 years agoMB-23591: Re-instate isBucketDeletion check for flusher 86/76186/2 v4.6.2 v4.6.2-MP1 v4.6.2-MP2 v4.6.2-MP3
Jim Walker [Mon, 3 Apr 2017 08:58:38 +0000 (09:58 +0100)]
MB-23591: Re-instate isBucketDeletion check for flusher

The VBDeleteTask maybe scheduled to run, and isBucketDeletion
flag indicates if so, thus the flusher still needs to check
it to ensure no writes occur to a file that is about to be

Change-Id: I746eb372f35c9b838dfab2ee7ac2b6c59606e94a
Reviewed-on: http://review.couchbase.org/76186
Well-Formed: Build Bot <build@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: David Haikney <david.haikney@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-23503: With an unpersisted snapshot, remove HT items till rollback point 28/75628/3
Manu Dhundi [Thu, 23 Mar 2017 18:52:06 +0000 (11:52 -0700)]
MB-23503: With an unpersisted snapshot, remove HT items till rollback point

When the rollback request intends to have a rollback to a point in
an unpersisted snapshot, we must remove all unpersisted checkpoint
items from both checkpoint and hash table till the last persisted
disk snapshot.

Prior to this commit, we removed the items from the checkpoint
correctly, but only removed hash table items till the requested
rollback point, not all the unpersisted items, when
requested_rollback_seqno > persisted_seqno.

Note: We currently always rollback to a point which is persisted on
disk. Hence we must drop all checkpoint items from checkpoint and
hash table, irrespective of whether they are part of a full snapshot
or partial snapshot.

Change-Id: I1c14d0df6ae5e5459e60ecb0fc4a72ecd14231d3
Reviewed-on: http://review.couchbase.org/75628
Well-Formed: Build Bot <build@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-23112 Fix deleteWithMeta and extended meta data input 29/74529/4
Jim Walker [Thu, 2 Mar 2017 16:28:35 +0000 (16:28 +0000)]
MB-23112 Fix deleteWithMeta and extended meta data input

An incorrect offset meant that we tried to parse an incorrect
part of the packet as ExtendedMetaData.

Test code updated to cover this case.

Change-Id: If0610fe73b378e5830b4f4b9978d2b7507b516b4
Reviewed-on: http://review.couchbase.org/74529
Reviewed-by: David Haikney <david.haikney@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Well-Formed: Build Bot <build@couchbase.com>

4 years agoMB-22180: Ensure all pendingBGFetches are terminated on VBbucket delete 11/73811/20
Daniel Owen [Mon, 19 Dec 2016 04:17:33 +0000 (04:17 +0000)]
MB-22180: Ensure all pendingBGFetches are terminated on VBbucket delete

On a VBucket delete we need to ensure that all pendingBGFetches are
terminated and any connections waiting for a BGFetch to complete are
notified with ENGINE_NOT_MY_VBUCKET.

Although we previously deleted the pendingBGFetches in the vbucket
destructor we did not send any notifications to waiting connections.

This patch moves the deletion of pendingBGFetches into the
notifyAllPendingConnsFailed function and in addition notifies all the
connections awaiting a BGFetch with an ENGINE_NOT_MY_VBUCKET.

Change-Id: I13a99fe01153a4ba8786aaf608b25ed31ace5a0c
Reviewed-on: http://review.couchbase.org/73811
Well-Formed: Build Bot <build@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-22178: Don't use opencheckpointid to determine if in backfill phase 13/73813/3
Daniel Owen [Thu, 8 Dec 2016 15:59:44 +0000 (15:59 +0000)]
MB-22178: Don't use opencheckpointid to determine if in backfill phase

The opencheckpointid of a bucket can be zero after a rollback.
If an opencheckpointid was zero it was assumed that the vbucket was in
backfilling state.  This caused the producer stream request to be stuck
waiting for backfilling to complete.

Ths patch uses the vb->isBackfillPhase() to determine if the vbucket is
in a backfill state as opposed to using an opencheckpointid of zero.

Change-Id: Ia977d6bf90e54fd1ceb8db4a9088b19d94d4bc8c
Reviewed-on: http://review.couchbase.org/70810
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
Reviewed-on: http://review.couchbase.org/73813
Well-Formed: Build Bot <build@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
4 years agoMB-22256: Fix intermittent failures in test_duplicate_items_disk 58/73758/3
Daniel Owen [Thu, 16 Feb 2017 17:39:47 +0000 (17:39 +0000)]
MB-22256: Fix intermittent failures in test_duplicate_items_disk

The ep_testsuite: test_duplicate_items_disk test occassionally fails
because waiting for the ep_vbucket_del stat to update (from 0 to 1)

The reason the stat occasionally does not increase is because the
the vbucket that is undergoing deletion is set back to active before
the deletion is complete.

A side-effect of setting a vbucket to active, is that if the vbucket
does not exist then it is recreated.

This patch moves the setting of the vbucket back to active to after we
have confirmed the deletion has completed (by ensuring the
ep_vbucket_del stat increases).  Thereby removing the race condition.

Change-Id: Ied680c64536862829974fd061a25d317f766bb46
Reviewed-on: http://review.couchbase.org/73758
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
4 years agoMB-22451: Ensure isBackfillTaskRunning is correctly set 10/73310/10
Daniel Owen [Fri, 3 Feb 2017 14:33:43 +0000 (14:33 +0000)]
MB-22451: Ensure isBackfillTaskRunning is correctly set

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

Change-Id: If8219a7f87b65af46d37a800eebf2257917cc555
Reviewed-on: http://review.couchbase.org/73310
Well-Formed: Build Bot <build@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-22235: Drift thresholds don't get updated from set_param 00/72100/3
Jim Walker [Mon, 9 Jan 2017 14:13:39 +0000 (14:13 +0000)]
MB-22235: Drift thresholds don't get updated from set_param

Need to plug the change value listener into the bucket config

Test updated to look at the vbucket's threshold values

Change-Id: Id515d71fe551474e93ff5113148ff8411ed739fb
Reviewed-on: http://review.couchbase.org/72100
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-21925: Fix queue_fill stat when persitenceCursor on queue_op::empty 30/70730/3 v4.6.0 v4.6.0-MP1
Dave Rigby [Wed, 7 Dec 2016 17:44:03 +0000 (17:44 +0000)]
MB-21925: Fix queue_fill stat when persitenceCursor on queue_op::empty

There is a bug related to how we account for items when we enqueue
mutations to be persisted (Checkpoint::queueDirty). The bug occurs
when we enqueue an item which already exists (i.e. it is to be
de-duplicated), and the persistence cursor has not permitted any items
for that VBucket - i.e. at the very beginning of the lifetime of the

With the advent of the MB-20852 (the set_vbucket_state changes), the
first item in a Bucket's checkpoint is now a
queue_op::set_vbucket_state - previously it would have been a "normal"
item. When items are added to a checkpoint, we just update the queuing
stats - essentially there are two possibilities:

1. The item is a new key (NEW_KEY), or it's a key which isn't new but
   has already been persisted (PERSIST_AGAIN) - we need to persist it -
   increment queue_fill

2. The item can be de-duplcated (EXISTING_ITEM) - don't increment

Essentially, there's a bug in this logic - we incorrectly mark the
incoming item as PERSIST_AGAIN, instead of EXISTING_ITEM. As a result
the queue_fill value is incorrectly inflated.

This is due to using a unsigned value for the mutation_id, which we
decrement by one if it's a meta_item. This should go to -1 (less than
any possible valid item id), but instead it goes to 2^63-1, *greater*
than any possible valid item (!)

Note: this is purely a stats issue, the actual item /is/ queued and
persisted correctly.

Change-Id: I8d40c417dc165e192b4f90e8cc16c6e87557d451
Reviewed-on: http://review.couchbase.org/70730
Tested-by: buildbot <build@couchbase.com>
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
4 years agoMB-21867: test_item_pager: Retry on ENOMEM when fetching items 15/70515/2
Dave Rigby [Thu, 1 Dec 2016 16:24:51 +0000 (16:24 +0000)]
MB-21867: test_item_pager: Retry on ENOMEM when fetching items

Change-Id: I9031ca1ff51aed609144ccf141897354138c2f88
Reviewed-on: http://review.couchbase.org/70515
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
4 years agoMB-21790: CheckpointTest.SeqnoAndHLCOrdering needs 1 checkpoint 50/70350/2
Jim Walker [Thu, 24 Nov 2016 16:45:38 +0000 (16:45 +0000)]
MB-21790: CheckpointTest.SeqnoAndHLCOrdering needs 1 checkpoint

Configure the test so that a single checkpoint is created, otherwise
the time-based closing code may split the data over more than 1
checkpoint causing the final data checks to fail.

Change-Id: I449a80d775b8d21afd9b46387a432dce7dcec2d1
Reviewed-on: http://review.couchbase.org/70350
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21769: Fix vbstate snap start/end off by one 40/70340/4
Jim Walker [Thu, 24 Nov 2016 10:41:25 +0000 (10:41 +0000)]
MB-21769: Fix vbstate snap start/end off by one

Recent changes to vbstate writing introduced an edge case regression.
If we force shutdown and had empty active vbuckets, after warmup
we will expose an incorrect failover table as we have an entry
for seq 1, yet the VB has high-seq of 0.

Change-Id: Iee67f71ce46c8eaf4f2cd822103354dcdecc04d8
Reviewed-on: http://review.couchbase.org/70340
Reviewed-by: Dave Rigby <daver@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB_21787: Remove log message from couch-kvstore for non-existent key 45/70345/2
Dave Rigby [Thu, 24 Nov 2016 14:54:55 +0000 (14:54 +0000)]
MB_21787: Remove log message from couch-kvstore for non-existent key

If an arithmetic operation is performed on a key which isn't resident,
on a full-eviction bucket then the following message is often printed
in the logs:

    WARNING (default) Failed to fetch data from database, vBucket=0
    key=Key_19 error=document not found [none]

This is unnecessarily verbose - this is not actually a warning in this
case and can quickly fill up the logs.

(Note: the reason this is shown is that prior to full-eviction, all
metadata would be resident and hence it would be unexpected to attempt
to read from disk a key which doesn't exist). However this is no
longer the case).

Change-Id: I6ac4f2b1f8d43700ff6869da8a6670e21454b3a0
Reviewed-on: http://review.couchbase.org/70345
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-21777: max_cas_str not formatting on Linux 98/70298/2
Jim Walker [Wed, 23 Nov 2016 15:28:05 +0000 (15:28 +0000)]
MB-21777: max_cas_str not formatting on Linux

Seems an issue with snprintf where we use timeString as input and
output yields only the nanosecond fractions being printed.

Fix also reduces buffers down, they're still bigger than we need, but
100 is way too much.

Change-Id: Ief6755d15d4d77a10a6c14fb76321893a7dd0d9c
Reviewed-on: http://review.couchbase.org/70298
Reviewed-by: David Haikney <david.haikney@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoSplit test_curr_items test into 4 smaller tests 52/70152/3
Dave Rigby [Mon, 21 Nov 2016 13:20:49 +0000 (13:20 +0000)]
Split test_curr_items test into 4 smaller tests

test_curr_items performs a number of different checks on the
curr_items stat. To simplify diagnosing issues with it (there's been a
number of intermittent failures), break it into four sub-tests.

Change-Id: I2ecd4bcacf8ca4cb7add65ec72a71e2a17545a24
Reviewed-on: http://review.couchbase.org/70152
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoRemove duplicate full_eviction tests from ep_testsuite 51/70151/3
Dave Rigby [Mon, 21 Nov 2016 13:12:29 +0000 (13:12 +0000)]
Remove duplicate full_eviction tests from ep_testsuite

Now we run all ep testsuites in with full and value eviction modes,
there is no need to run some specific tests in full_eviction
mode. Remove them.

Change-Id: I58256f28173b4113a9818fb7b514aceda002d7ec
Reviewed-on: http://review.couchbase.org/70151
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21765: Missing lock from doWorkerStat 62/70262/3
Daniel Owen [Tue, 22 Nov 2016 16:35:47 +0000 (16:35 +0000)]
MB-21765: Missing lock from doWorkerStat

tMutex must be acquired before accessing the threadQ data structure.

07:01:46 WARNING: ThreadSanitizer: data race (pid=32512)
07:01:46 Write of size 8 at 0x7d1c00016eb8 by thread T9 (mutexes: write M27337):
07:01:46 #0 operator delete(void*) <null> (engine_testapp+0x000000464cbb)
07:01:46 #1 <null> <null> (libstdc++.so.6+0x0000000c1ac7)
07:01:46 #2 ExecutorThread::run() /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/executorthread.cc:127 (ep.so+0x0000000f7ff5)
07:01:46 #3 launch_executor_thread(void*) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/executorthread.cc:32 (ep.so+0x0000000f78a5)
07:01:46 #4 CouchbaseThread::run() /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/platform/src/cb_pthreads.cc:58 (libplatform_so.so.0.1.0+0x000000007b91)
07:01:46 #5 platform_thread_wrap(void*) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/platform/src/cb_pthreads.cc:71 (libplatform_so.so.0.1.0+0x000000005e92)
07:01:46 Previous read of size 8 at 0x7d1c00016eb8 by main thread (mutexes: write M20977, write M26658):
07:01:46 #0 strlen <null> (engine_testapp+0x0000004652af)
07:01:46 #1 showJobLog(char const*, char const*, std::vector<TaskLogEntry, std::allocator<TaskLogEntry> > const&, void const*, void (char const*, unsigned short, char const*, unsigned int, void const*)) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/statwriter.h:39 (ep.so+0x0000000f346b)
07:01:46 #2 ExecutorPool::doWorkerStat(EventuallyPersistentEngine*, void const*, void (char const*, unsigned short, char const*, unsigned int, void const*)) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/executorpool.cc:801 (ep.so+0x0000000f2ce2)
07:01:46 #3 EventuallyPersistentEngine::getStats(void const*, char const*, int, void (char const*, unsigned short, char const*, unsigned int, void const*)) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/ep_engine.cc:4383 (ep.so+0x0000000c30bf)

Change-Id: Iae7aa0fe9da15805671762516221f25da69fe5d8
Reviewed-on: http://review.couchbase.org/70262
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoFix error in Blob::operator== 68/70168/2
Dave Rigby [Mon, 21 Nov 2016 17:22:17 +0000 (17:22 +0000)]
Fix error in Blob::operator==

Fix typo in Blob::operaror== - incorrect comparision of
extMetaLen. Introduced by http://review.couchbase.org/69477

Bug identified by Coverity:

    CID 153111:  Incorrect expression  (CONSTANT_EXPRESSION_RESULT)
    "lhs.extMetaLen == lhs.extMetaLen" is always true regardless of
    the values of its operands because those operands are identical. This
    occurs as the logical second operand of "&&".

Change-Id: I87529e6a3801358bafe265869125c2eff2d40587
Reviewed-on: http://review.couchbase.org/70168
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agoMB-21738: Fix potential crash due to race deleting VBucket 63/70163/3
Dave Rigby [Mon, 21 Nov 2016 15:09:29 +0000 (15:09 +0000)]
MB-21738: Fix potential crash due to race deleting VBucket

There is a potential race condition in
VBucketMap::setPersistenceCheckpointId during VBucket deletion which
can result in dereferencing a deleted pointer, triggering a segfault.

The issue is that setPersistenceCheckpointId can dereference a RCPtr
which has just become null. The issue is on line 177 - we dutifully
check if is valid, but then re-fetch the VBucket - at which point it
may have been set to null by another thread (such as when a VBucket is

Fix is to just use the local `vb` to dereference.

Change-Id: I683cb0d0cfe37e710e98ba6bbf1ddd4cf3682a35
Reviewed-on: http://review.couchbase.org/70163
Reviewed-by: David Haikney <david.haikney@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21599: Prevent LWW XDCR from non LWW cluster 16/70016/12
Jim Walker [Thu, 17 Nov 2016 12:35:43 +0000 (12:35 +0000)]
MB-21599: Prevent LWW XDCR from non LWW cluster

 - This flag must be set if the bucket is lww
 - Error if the bucket is !lww

Add support for REGENERATE_CAS
 - This flag requires SKIP_CONFLICT_RESOLUTION
 - When the item is stored, its CAS is re-created.

Tidy up test code relating to *_with_meta and add more extensive
testing of options and nmeta.

Change-Id: Ifb149927d3f63357d30392352c9c81533cbf2ff1
Reviewed-on: http://review.couchbase.org/70016
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-21539: Address raciness / intermittent timeout in test_mb16357 85/70085/7
Dave Rigby [Fri, 18 Nov 2016 15:29:52 +0000 (15:29 +0000)]
MB-21539: Address raciness / intermittent timeout in test_mb16357

This is a multithreaded test which attempts to run the compactor
concurrently with a DCP stream when a vBucket flips to
replica. However, there is no interlocking of the threads' startup,
which can result in the compaction running and completing before the
DCP thread reaches line 810 (where it busy-waits for
ep_pending_compactions != 0). As a result the test can get stuck at
that line and timeout.

Address this by introducing synchronisation between the two threads -
ensure that the compactor thread is ready and about to compact before
advancing the DCP thread.

Change-Id: Icc5abb3080a981823bb2161d083f0d866364b76e
Reviewed-on: http://review.couchbase.org/70085
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-21716: Consistent drift_threshold parameters 83/70083/8
Jim Walker [Fri, 18 Nov 2016 12:53:51 +0000 (12:53 +0000)]
MB-21716: Consistent drift_threshold parameters

Change hlc_ahead to hlc_drift_ahead

Same for behind parameters.

Change-Id: I9331772c736767fecc82350087f581cdf13801a9
Reviewed-on: http://review.couchbase.org/70083
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21685: Use EWOULDBLOCK variant of delVBucket 82/70082/7
Jim Walker [Tue, 15 Nov 2016 16:04:23 +0000 (16:04 +0000)]
MB-21685: Use EWOULDBLOCK variant of delVBucket

The test can occasionally fail because a delete overlaps
the test code which has set the VB active and written data.
The data in the overlap gets deleted from disk.

MB-21687 covers the overlap issue for delVBucket without the async=0

Change-Id: I74666d087aab65456110d7815f701e4193fc71b9
Reviewed-on: http://review.couchbase.org/70082
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21724: Reduce iterations in CheckpointTest basic_chk_test 86/70086/6
Jim Walker [Fri, 18 Nov 2016 15:37:12 +0000 (15:37 +0000)]
MB-21724: Reduce iterations in CheckpointTest basic_chk_test

Reduce number of items and reduce threads and items further when
running under valgrind.

Also removed a sleep(1) and fixed the thread start/wait code
that relied on the sleep(1).

Change-Id: Ie6d71bf0972e30225343c12c51e36d5adadec794
Reviewed-on: http://review.couchbase.org/70086
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-21725: test_exp_persisted_set_del check for greater or equal 1 91/70091/2
Jim Walker [Fri, 18 Nov 2016 16:43:17 +0000 (16:43 +0000)]
MB-21725: test_exp_persisted_set_del check for greater or equal 1

The test stores key twice and waits for ep_total_persisted to equal 1.
The test fails because the wait_for stat function timesout, the
total persisted is 2.

If the flusher ran quickly we may actually have a value of 2.

Change-Id: I18d8e50e5a5bcc4839830adb30e104bb6796daa0
Reviewed-on: http://review.couchbase.org/70091
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21540: ep-engine: Remove abort() 76/69376/11
Dave Rigby [Tue, 1 Nov 2016 16:39:05 +0000 (16:39 +0000)]
MB-21540: ep-engine: Remove abort()

Final set of changes to remove abort() from ep_engine production code;
replacing with exceptions where a dynamic error is still required.

All non-test ep-engine code is now abort-free :)

Change-Id: I9702b6bbaf28267b696498067318e78af0988002
Reviewed-on: http://review.couchbase.org/69376
Reviewed-by: Daniel Owen <owend@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21568: Don't double-free newdb if rewind_db_header fails 80/70080/3
Dave Rigby [Fri, 18 Nov 2016 12:31:19 +0000 (12:31 +0000)]
MB-21568: Don't double-free newdb if rewind_db_header fails

If couchstore_rewind_db_header() fails, then it will free the DB
before returning, so we need to ensure that the DbHolder doesn't cause
a double-free by also trying to free the Db.

Found during merge to master where we have improved couchstore testing

Change-Id: I49ba9e7e91eb73aaae90ef8aa8b41f56bbe056c9
Reviewed-on: http://review.couchbase.org/70080
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-19929: Fix sporadic failure in 'dcp cursor dropping' test 84/70084/2
Manu Dhundi [Fri, 17 Jun 2016 21:29:26 +0000 (14:29 -0700)]
MB-19929: Fix sporadic failure in 'dcp cursor dropping' test

In cursor dropping test the max_size and chk_max_items should be such
that 2 checkpoints are created. The test simulates a case where of
the 2 checkpoints, the DCP cursor from one of them is dropped.

Change-Id: Ieb5c6dfba43bea2f8256fa01104f9ddf83a9781f
Reviewed-on: http://review.couchbase.org/70084
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-21650: Prevent false sharing of frequently modified memory stats 51/69951/8
Dave Rigby [Wed, 16 Nov 2016 17:59:55 +0000 (17:59 +0000)]
MB-21650: Prevent false sharing of frequently modified memory stats

We record the memory usage, memory overhead and number of Items per
bucket, in the EPStats object. These stats are very frequenly updated
(on every memory allocation/deallocation, and every Item
creation/destruction), and they are updated from all threads. This can
cause false sharing between these values if they co-exist in the same
cache line.

A recent change (66882e8 - MB-20852 [17/N]: Serialize VB state
changes) added a new element to EPStats which resulted in the layout
of that class changing such that memory stat variables (memOverhead,
numItem, totalMemory) ended up on the same cache line. This resulted
in a signficant regression (1.9M op/s -> 1.4M ops) in the performance
of the 'kv_max_ops_reads_512_avg_ops_hera_kv' test, due to false
sharing between these variables.

To address this, ensure that each variable is placed on its own cache
line to prevent false sharing, by using the CachelinePadded template

Change-Id: I8316637a7a0c6fd05fd6f6ba24a1df44c43390c5
Reviewed-on: http://review.couchbase.org/69951
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21650: Don't copy key when servicing memcached get() request 23/70023/5
Dave Rigby [Wed, 16 Nov 2016 17:37:13 +0000 (09:37 -0800)]
MB-21650: Don't copy key when servicing memcached get() request

In the get request path (EvpGet() and onwards) we take a copy of the
key from the request packet into a std::string. This is then passed
around ep-engine (read-only) to retrieve the document to return to the

This incurs the cost of creating a std::string object per get request,
along with the related memory tracking overheads which are not
insigificant (see ObjectRegistry).

To reduce this cost, use a const_sized_buffer (essentially a pair of
{const char*, size_t}) for the key, relying on the fact that the
client's request is always available in the request packet owned by

On the 2-node 'hera' cluster this improves the op/s of the following
pillowfight benchmark, from 1.8M op/s total to 2.3M op/s:

    cbc-pillowfight --spec couchbase:// \
        --batch-size 1000 --num-items 1000000 --num-threads 50 \
         --min-size 512 --max-size 512 --set-pct 20

(client running on 3rd hera-client node).

Change-Id: I0371fb5ef9bdcdc6f92bb941926e8af80cf5e24f
Reviewed-on: http://review.couchbase.org/70023
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20079: Ensure updateWaketimeIfLessThan performs a compare_exchange 27/70027/4
Daniel Owen [Thu, 17 Nov 2016 17:00:25 +0000 (17:00 +0000)]
MB-20079: Ensure updateWaketimeIfLessThan performs a compare_exchange

The patch MB-20079: Use std::chrono::steady_clock (ProcessClock) with
changeID: I2fc9688abb782fe2c9e80efb6da840be3643d4a5 introduced a bug
where the waketime could be incorrectly updated, due to not performing
a compare_exchange_strong.

This patch reverts to using a compare_exchange_strong, ensuring that
waketime is only updated if it is less than the input time_point.

Change-Id: Ib68689f9a76c6a0455422c293ad62e9c4d04f3d7
Reviewed-on: http://review.couchbase.org/70027
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21568: Lock inversion issue in rollback 29/70029/3
Jim Walker [Thu, 17 Nov 2016 15:45:42 +0000 (15:45 +0000)]
MB-21568: Lock inversion issue in rollback

vbsetMutex must be obtained before vb::stateMutex.

the rollback path needs to keep both held until
complete so some refactoring to expose
_UNLOCKED variants of setVBucketState and resetVbucket
so there's no subsequent inversion risk

Change-Id: I16d869277ad5609b6b45042ea32b3f1037faeb72
Reviewed-on: http://review.couchbase.org/70029
Reviewed-by: Dave Rigby <daver@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21568: Use signed int64 when comparing against item bySeqno 24/70024/2
Jim Walker [Thu, 17 Nov 2016 16:49:19 +0000 (16:49 +0000)]
MB-21568: Use signed int64 when comparing against item bySeqno

addresses compiler warning

Change-Id: I63ae0f15baf6512679ca32491b707500309b09e9
Reviewed-on: http://review.couchbase.org/70024
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-21568: Reconcile hashtable with disk following rollback 25/69725/16
Dave Rigby [Mon, 7 Nov 2016 06:36:42 +0000 (22:36 -0800)]
MB-21568: Reconcile hashtable with disk following rollback

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

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

Change-Id: Iabe43f59ed40931c1c97b65147b7a414d4ff7cc5
Reviewed-on: http://review.couchbase.org/69725
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21692: Improved stability of "temp item deletion" test 41/69941/3
Jim Walker [Wed, 16 Nov 2016 10:53:47 +0000 (10:53 +0000)]
MB-21692: Improved stability of "temp item deletion" test

Ensure the expiry pager runs at the end of the test (before final
assertions) and after the meta bgFetches are complete.

Change-Id: I9182ae5a1ea84da6165ceca7cf04ab3cc1b2f1cd
Reviewed-on: http://review.couchbase.org/69941
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20079: Use std::chrono::steady_clock (ProcessClock) 99/69899/2
Daniel Owen [Tue, 15 Nov 2016 10:33:24 +0000 (10:33 +0000)]
MB-20079: Use std::chrono::steady_clock (ProcessClock)

Change task scheduling to use ProcessClock which is not
affected by changes to wall clock time.

Change-Id: I2fc9688abb782fe2c9e80efb6da840be3643d4a5
Reviewed-on: http://review.couchbase.org/69899
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-21540: tapconnection: Replace abort() with exceptions 75/69375/3
Dave Rigby [Tue, 1 Nov 2016 16:35:59 +0000 (16:35 +0000)]
MB-21540: tapconnection: Replace abort() with exceptions

Change-Id: I100af9bca783d3e3f282d315ffcb99915170eff5
Reviewed-on: http://review.couchbase.org/69375
Reviewed-by: David Haikney <david.haikney@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoItem: Add stream and equality operators 77/69477/4
Dave Rigby [Fri, 4 Nov 2016 09:23:06 +0000 (09:23 +0000)]
Item: Add stream and equality operators

Add operator<< and == to Item and the classes Item is built from
(Blob, ItemMetaData).

Change-Id: I40d5edafae777a65a2a80c7286bb77c91fc486cc
Reviewed-on: http://review.couchbase.org/69477
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-21587: Rollback: Don't use CAS when rolling back values in HashTable 78/69478/6
Dave Rigby [Fri, 4 Nov 2016 14:00:10 +0000 (14:00 +0000)]
MB-21587: Rollback: Don't use CAS when rolling back values in HashTable

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

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

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

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

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

Change-Id: I32577afd0f6f9c79122575f84a76acd00fb6f89a
Reviewed-on: http://review.couchbase.org/69478
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
4 years agoMB-21617: Change CAS resolution to nanoseconds 08/69708/7
Jim Walker [Tue, 8 Nov 2016 11:15:58 +0000 (11:15 +0000)]
MB-21617: Change CAS resolution to nanoseconds

This gives better compatibility with older datasets and a less
confusing experience if an old cluster did set_with_cas against
a new one.

The CAS is generated as a nanosecond value.

Drift is tracked though in microseconds to give a longer window before
we may overflow the counter. The config thresholds are also all ┬Ás
but converted to ns so we can correctly track drift.

A new max_cas_str is also added to assist the supportability of
LWW, so at a glance you can see what the max_cas means as a
date/time string.

Change-Id: I40fb89add968043aca01b1de103f62319d814a5c
Reviewed-on: http://review.couchbase.org/69708
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21540: ep_time.cc: Replace abort() with std::logic_error 74/69374/3
Dave Rigby [Tue, 1 Nov 2016 16:04:59 +0000 (16:04 +0000)]
MB-21540: ep_time.cc: Replace abort() with std::logic_error

Change-Id: I26ab8f0f92e879ba6fe49e974d79fa020da20a69
Reviewed-on: http://review.couchbase.org/69374
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21540: Convert ep_time.c -> ep_time.cc 73/69373/4
Dave Rigby [Tue, 1 Nov 2016 16:03:35 +0000 (16:03 +0000)]
MB-21540: Convert ep_time.c -> ep_time.cc

In preparation for replacing use of abort() with exceptions.

Change-Id: I8b95d9b5236c2f7735977cbebd1f182765adb2d4
Reviewed-on: http://review.couchbase.org/69373
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21540: DCP: Replace abort() with exceptions 72/69372/5
Dave Rigby [Tue, 1 Nov 2016 15:54:22 +0000 (15:54 +0000)]
MB-21540: DCP: Replace abort() with exceptions

Remove use of abort() when checking for "impossible" logical
situations in DCP. Where possible remove the abort entirely
(e.g. explicilty handle all cases in swtich statements), otherwise
replace with std::logic_error.

Change-Id: I9a45e283562b5e7361f8e9edd4316d9d253b4e05
Reviewed-on: http://review.couchbase.org/69372
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agoMB-21540: Refactor VBucketVisitor to remove need for abort 71/69371/5
Dave Rigby [Tue, 1 Nov 2016 15:42:12 +0000 (15:42 +0000)]
MB-21540: Refactor VBucketVisitor to remove need for abort

VBucketVisitor is used to visit all VBuckets in a Bucket, and is
currently implemented as a derived class of HashTableVisitor - as the
intent is that concrete classes will iterate at two levels - first
over each VBucket, and then for all StoredValues within the HashTable
of that VBucket.

However, not all derived classes actually visit the individual
StoredValues - many just operate at the VBucket level. As such, these
classes do not expect the visit(StoredValue*) method to be called, and
have abort()s on these code paths.

This is undesirable - it should be possible to statically (i.e. at
compile-time) ensure that such functions are never called.

To address this (and remove the abort() calls), modify VBucketVisitor
so it doesn't inherit from HashTableVisitor - essentially it just
deals with visiting at the VBucket level, which *is* true for all
derived classes. For those derived classes which /do/ wish to perform
recursive visiting of StoredValue, the derived class itself can
inherit from HashTableVisitor.

Additionally, as VBucketVisitor only deals with VBucket visiting,
simpify the visiting logic to a "classic" Visitor pattern, making
visitBucket return void (instead of bool), and let the visitBucket()
method itself handle what do with the VBucket is it passed. This means
that the caller (e.g. EventuallyPersistentStore::visit()) no longer
chooses if the HashTable::visit method is called - the derived class
can call that itself if it chooses.

Change-Id: I1dc8724db9e89daff261bccfe22dc6e2db885b22
Reviewed-on: http://review.couchbase.org/69371
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
4 years agoMB-20725: Reset drift stats 22/69422/3
Jim Walker [Wed, 2 Nov 2016 16:00:56 +0000 (16:00 +0000)]
MB-20725: Reset drift stats

When resetting stats, include the drift counters.

Change-Id: I5c0b78a9d885ed6970f219c2089bf34df3da2425
Reviewed-on: http://review.couchbase.org/69422
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20760: Fix data race on VBucket::chkFlushTimeout 34/69234/6 v4.6.0-DP
Dave Rigby [Wed, 26 Oct 2016 10:00:21 +0000 (11:00 +0100)]
MB-20760: Fix data race on VBucket::chkFlushTimeout

As identified by ThreadSantizer:

WARNING: ThreadSanitizer: data race (pid=23560)
  Write of size 8 at 0x7ff76a2e7e50 by thread T52 (mutexes: write M64345, write M85935):
    #0 VBucket::adjustCheckpointFlushTimeout(unsigned long) ep-engine/src/vbucket.cc:393 (ep.so+0x00000016c806)
    #1 VBucket::notifyOnPersistence(EventuallyPersistentEngine&, unsigned long, bool) ep-engine/src/vbucket.cc:332 (ep.so+0x00000016c806)
    #2 EventuallyPersistentStore::flushVBucket(unsigned short) ep-engine/src/ep.cc:3475 (ep.so+0x0000000be736)
    #3 Flusher::flushVB() ep-engine/src/flusher.cc:284 (ep.so+0x0000001100d1)
    #4 Flusher::step(GlobalTask*) ep-engine/src/flusher.cc:183 (ep.so+0x0000001110b7)
    #5 FlusherTask::run() ep-engine/src/tasks.cc:138 (ep.so+0x000000156642)
    #6 ExecutorThread::run() ep-engine/src/executorthread.cc:115 (ep.so+0x000000108d96)
    #7 launch_executor_thread ep-engine/src/executorthread.cc:33 (ep.so+0x000000109675)
    #8 CouchbaseThread::run() platform/src/cb_pthreads.cc:54 (libplatform.so.0.1.0+0x0000000057a5)
    #9 platform_thread_wrap platform/src/cb_pthreads.cc:66 (libplatform.so.0.1.0+0x0000000057a5)

  Previous read of size 8 at 0x7ff76a2e7e50 by thread T9 (mutexes: write M295):
    #0 VBucket::getCheckpointFlushTimeout() ep-engine/src/vbucket.cc:406 (ep.so+0x0000001683e6)
    #1 EventuallyPersistentEngine::doEngineStats(void const*, void (*)(char const*, unsigned short, char const*, unsigned int, void const*)) ep-engine/src/ep_engine.cc:3628 (ep.so+0x0000000e9678)
    #2 EventuallyPersistentEngine::getStats(void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*)) ep-engine/src/ep_engine.cc:4680 (ep.so+0x0000000ea930)
    #3 EvpGetStats ep-engine/src/ep_engine.cc:235 (ep.so+0x0000000eb9fe)
    #4 stat_executor memcached/daemon/mcbp_executors.cc:3367 (memcached+0x00000046c94b)
    #5 process_bin_packet memcached/daemon/mcbp_executors.cc:4650 (memcached+0x00000046481d)
    #6 mcbp_complete_nread(McbpConnection*) memcached/daemon/mcbp_executors.cc:4759 (memcached+0x00000046481d)
    #7 conn_nread(McbpConnection*) memcached/daemon/statemachine_mcbp.cc:314 (memcached+0x000000472678)
    #8 McbpStateMachine::execute(McbpConnection&) memcached/daemon/statemachine_mcbp.h:43 (memcached+0x000000447054)
    #9 McbpConnection::runStateMachinery() memcached/daemon/connection_mcbp.cc:1003 (memcached+0x000000447054)
    #10 McbpConnection::runEventLoop(short) memcached/daemon/connection_mcbp.cc:1274 (memcached+0x0000004470dd)
    #11 run_event_loop memcached/daemon/connections.cc:147 (memcached+0x00000044b9e9)
    #12 event_handler(int, short, void*) memcached/daemon/memcached.cc:851 (memcached+0x00000041466c)
    #13 event_persist_closure /home/couchbase/serverjenkins/workspace/cbdeps-platform-build/deps/packages/build/libevent/libevent-prefix/src/libevent/event.c:1319 (libevent_core-2.0.so.5+0x00000000b6b7)
    #14 event_process_active_single_queue /home/couchbase/serverjenkins/workspace/cbdeps-platform-build/deps/packages/build/libevent/libevent-prefix/src/libevent/event.c:1363 (libevent_core-2.0.so.5+0x00000000b6b7)
    #15 event_process_active /home/couchbase/serverjenkins/workspace/cbdeps-platform-build/deps/packages/build/libevent/libevent-prefix/src/libevent/event.c:1438 (libevent_core-2.0.so.5+0x00000000b6b7)
    #16 event_base_loop /home/couchbase/serverjenkins/workspace/cbdeps-platform-build/deps/packages/build/libevent/libevent-prefix/src/libevent/event.c:1639 (libevent_core-2.0.so.5+0x00000000b6b7)
    #17 CouchbaseThread::run() platform/src/cb_pthreads.cc:54 (libplatform.so.0.1.0+0x0000000057a5)
    #18 platform_thread_wrap platform/src/cb_pthreads.cc:66 (libplatform.so.0.1.0+0x0000000057a5)

  Location is global 'VBucket::chkFlushTimeout' of size 8 at 0x7ff76a2e7e50 (ep.so+0x000000431e50)

Change-Id: I2d6f928b8a5552cf08a91c6134ad5134810966c2
Reviewed-on: http://review.couchbase.org/69234
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21539: Use correct expiry value for test 'MB-16357' 65/69365/3
Dave Rigby [Tue, 1 Nov 2016 14:22:54 +0000 (14:22 +0000)]
MB-21539: Use correct expiry value for test 'MB-16357'

During the refactor done in b4c858e, the expiration value of 1s was
lost for this test. Restore it.

Change-Id: I0b1bf35548cffcd6867a29dcbb9a843b0f06b4c8
Reviewed-on: http://review.couchbase.org/69365
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21534: test_access_scanner - handle clock wraparound 64/69364/3
Dave Rigby [Tue, 1 Nov 2016 11:40:06 +0000 (11:40 +0000)]
MB-21534: test_access_scanner - handle clock wraparound

test_access_scanner attempts to set the access log scanner runtime to
two hours in the future; however it doesn't check for wraparound

Change-Id: I2851331a31773fe32221f3cf266f770f1ebefbaf
Reviewed-on: http://review.couchbase.org/69364
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21320: Notify memcached to visit readyQ and get any ready items 71/69171/7
Manu Dhundi [Mon, 31 Oct 2016 16:53:18 +0000 (09:53 -0700)]
MB-21320: Notify memcached to visit readyQ and get any ready items

We should notify memcached to visit readyQ and get any items that were
pushed there during stream creation. Also, we must notify the memcached
about cursor dropping so that it can visit ep-engine and stream any
pending items and do a subsequent stream state change.

This is not a functional fix. It improves performance however.
It is not absolutely necessary to notify immediately as conn manager
will notify eventually.

Change-Id: Id06fc450a20f6d0258fa7c687520dff5f4899a28
Reviewed-on: http://review.couchbase.org/69171
Reviewed-by: Dave Rigby <daver@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20822: Print correct value for vb_%d:num_erroneous_entries_erased 32/69132/3
Dave Rigby [Mon, 24 Oct 2016 09:21:30 +0000 (10:21 +0100)]
MB-20822: Print correct value for vb_%d:num_erroneous_entries_erased

'num_erroneous_entries_erased' was incorrectly printing the total size
of the failover table, and not the number of elements which have been

Change-Id: I7bbd75ca4db962cba257a1883ce3635b475287c7
Reviewed-on: http://review.couchbase.org/69132
Reviewed-by: Jim Walker <jim@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21190: update log message for set max_cas 94/68994/5
Jim Walker [Thu, 20 Oct 2016 08:03:40 +0000 (09:03 +0100)]
MB-21190: update log message for set max_cas

There's a future desire to print vbucket's as "vb:"

Change-Id: I5ac58daa01a1dd2ed5a631558bf735c9a81ed821
Reviewed-on: http://review.couchbase.org/68994
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20725: Update drift accounting stats 28/69328/5
Jim Walker [Mon, 31 Oct 2016 12:55:23 +0000 (12:55 +0000)]
MB-20725: Update drift accounting stats

The bucket now reports a total of drift for all VBs which will
allow better tracking of drift (e.g. UI graphing).

Change-Id: I5600df323b32f3f1d11feb5be2cf8ddac5988be9
Reviewed-on: http://review.couchbase.org/69328
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20759: Fix false-positive race on DcpConnMap::numActiveSnoozingBackfills 35/69235/4
Dave Rigby [Wed, 26 Oct 2016 10:12:08 +0000 (11:12 +0100)]
MB-20759: Fix false-positive race on DcpConnMap::numActiveSnoozingBackfills

The existing code was safe, however ThreadSanitizer doesn't know about
our own Spinlocks. Given this shouldn't be a hot path, switch to
normal std:mutex.

ThreadSanitizer report:

WARNING: ThreadSanitizer: data race (pid=23569)
  Read of size 2 at 0x7d840000eca2 by thread T8 (mutexes: write M294, read M27095, write M66205, write M102676, write M95235):
    #0 DcpConnMap::canAddBackfillToActiveQ() ep-engine/src/connmap.cc:1308 (ep.so+0x000000045ac5)
    #1 BackfillManager::schedule() ep-engine/src/dcp/backfill-manager.cc:142 (ep.so+0x00000005b0eb)
    #2 DcpProducer::scheduleBackfillManager() ep-engine/src/dcp/producer.cc:702 (ep.so+0x000000078fe3)
    #3 ActiveStream::scheduleBackfill_UNLOCKED(bool) ep-engine/src/dcp/stream.cc:1016 (ep.so+0x00000008f280)
    #4 ActiveStream::transitionState(stream_state_t) ep-engine/src/dcp/stream.cc:1145 (ep.so+0x000000090589)
    #5 ActiveStream::setActive() ep-engine/src/dcp/stream.h:204 (ep.so+0x00000009958e)
    #6 DcpProducer::streamRequest() ep-engine/src/dcp/producer.cc:327 (ep.so+0x00000007f85d)
    #7 EvpDcpStreamReq ep-engine/src/ep_engine.cc:1573 (ep.so+0x0000000cea78)
    #8 dcp_stream_req_executor memcached/daemon/mcbp_executors.cc:2272 (memcached+0x00000045925c)
    #9 process_bin_packet memcached/daemon/mcbp_executors.cc:4650 (memcached+0x00000046481d)
    #10 mcbp_complete_nread(McbpConnection*) memcached/daemon/mcbp_executors.cc:4759 (memcached+0x00000046481d)
    #11 conn_nread(McbpConnection*) memcached/daemon/statemachine_mcbp.cc:314 (memcached+0x000000472678)
    #12 McbpStateMachine::execute(McbpConnection&) memcached/daemon/statemachine_mcbp.h:43 (memcached+0x000000447054)
    #13 McbpConnection::runStateMachinery() memcached/daemon/connection_mcbp.cc:1003 (memcached+0x000000447054)
    #14 McbpConnection::runEventLoop(short) memcached/daemon/connection_mcbp.cc:1274 (memcached+0x0000004470dd)
    #15 run_event_loop memcached/daemon/connections.cc:147 (memcached+0x00000044b9e9)
    #16 event_handler(int, short, void*) memcached/daemon/memcached.cc:851 (memcached+0x00000041466c)
    #17 event_persist_closure src/libevent/event.c:1319 (libevent_core-2.0.so.5+0x00000000b6b7)
    #18 event_process_active_single_queue src/libevent/event.c:1363 (libevent_core-2.0.so.5+0x00000000b6b7)
    #19 event_process_active src/libevent/event.c:1438 (libevent_core-2.0.so.5+0x00000000b6b7)
    #20 event_base_loop src/libevent/event.c:1639 (libevent_core-2.0.so.5+0x00000000b6b7)
    #21 CouchbaseThread::run() platform/src/cb_pthreads.cc:54 (libplatform.so.0.1.0+0x0000000057a5)
    #22 platform_thread_wrap platform/src/cb_pthreads.cc:66 (libplatform.so.0.1.0+0x0000000057a5)

  Previous write of size 2 at 0x7d840000eca2 by thread T55:
    #0 DcpConnMap::decrNumActiveSnoozingBackfills() ep-engine/src/connmap.cc:1319 (ep.so+0x000000045b7b)
    #1 BackfillManager::backfill() ep-engine/src/dcp/backfill-manager.cc:273 (ep.so+0x00000005a783)
    #2 BackfillManagerTask::run() ep-engine/src/dcp/backfill-manager.cc:62 (ep.so+0x00000005ac1c)
    #3 ExecutorThread::run() ep-engine/src/executorthread.cc:115 (ep.so+0x000000108d96)
    #4 launch_executor_thread ep-engine/src/executorthread.cc:33 (ep.so+0x000000109675)
    #5 CouchbaseThread::run() platform/src/cb_pthreads.cc:54 (libplatform.so.0.1.0+0x0000000057a5)
    #6 platform_thread_wrap platform/src/cb_pthreads.cc:66 (libplatform.so.0.1.0+0x0000000057a5)

Change-Id: I88df57b268c1e615b7c5d2b7caf5f038a53692ba
Reviewed-on: http://review.couchbase.org/69235
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-19672: Fix intermittent failure of 'test producer stream request (partial)' 33/69233/5
Dave Rigby [Wed, 26 Oct 2016 09:48:33 +0000 (10:48 +0100)]
MB-19672: Fix intermittent failure of 'test producer stream request (partial)'

This test was making incorrect assumptions about how / when
checkpoints would be created - it wasn't taking the checkpoint period
(how often we forcefully create new checkpoints) into account in it's
assuptions of checkpoint layout. As such it would occasionally fail if
it took longer than expected to run.

Fix this by essentially disabling chk_period (setting it to an
arbitrarily large value). At the same time make the tests expectations
explicit and check them where possible.

Change-Id: I9944f19d41a0a33064c9c43c8673c7ef9c4a3ab9
Reviewed-on: http://review.couchbase.org/69233
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21109: Fix intermittent failure in 'temp item deletion' 37/69237/4
Dave Rigby [Wed, 26 Oct 2016 12:05:03 +0000 (13:05 +0100)]
MB-21109: Fix intermittent failure in 'temp item deletion'

Test suffers from intermittent failures due to it not accounting for
background fetch of deleted items. The issue is that the read of
curr_temp_items races with the background fetcher - if the background
fetcher has not completed then it will have a value of 1 (expected),
however if the BG fetcher completes before the stat is read then 0
will be returned (as the temp item has been cleaned up).

To address this, disable the reader threads before triggering the
background fetch. This requires that we manually handle the
EWOULDBLOCK that get_meta returns (otherwise we'll hang forever
waiting for the BG fetch task to run).

Change-Id: If1bca36a6517909259b4e3767fd58a7ff99944c3
Reviewed-on: http://review.couchbase.org/69237
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [18/N]: Remove now dead VBucket persist Tasks 51/69151/9
Dave Rigby [Mon, 24 Oct 2016 11:46:01 +0000 (12:46 +0100)]
MB-20852 [18/N]: Remove now dead VBucket persist Tasks

Delete all tasks (apart from Flusher) which used to persist VBucket
state to disk - the Flusher Task now handles all VBucket state

Also remove statistics which are no longer valid / relevent -
snapshotVbucketHisto / persistVBStateHisto - we no longer snapshot
separately from flushing.

Change-Id: Ia1b3f30f0f792a8ca739d377443f909138dcaa86
Reviewed-on: http://review.couchbase.org/69151
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [17/N]: Serialize VB state changes 50/69150/11
Dave Rigby [Wed, 21 Sep 2016 15:03:47 +0000 (16:03 +0100)]
MB-20852 [17/N]: Serialize VB state changes


MB-20852 exposed an issue with how VBucket state was persisted to disk
- specifically that state can be persisted to disk out of order, and
intermediate state changes could be dropped (not persisted at all).

These problems are caused by the asynchronous tasks which are
responsible for persisting state to disk -
VBStatePersistTask{Low,High}. There are essentially two interrelated

1. We allow multiple tasks which persist VBState to exist concurrently
   (VBStatePersistTask{Low,High}, Flusher,
   VBSnapshotTask{Low,High}. Furthermore, multiple instances of the
   same task (e.g. VBStatePersistTask) can also exist concurrenlty.

2. We do not maintain a queue of states to persist, instead we just
   mark that a given vbid's state is pending.

(1) Can occur when a VBPersistTask has started to run on a BG writer
    thread (and has cleared the schedule_vbstate_persist flag), but
    then another scheduleVBStatePersist() call is made. As the
    schedule_vbstate_persist flag is clear, this second task is
    allowed to be created. There is then no guarantee which task will
    complete first (they could be scheduled to different OS writer
    threads, the first thread could be descheduled and the second one
    then runs and completes first).

    We could attempt to solve this by changing when
    schedule_vbstate_persist is cleared (say move it later in the
    persistVBState() function), but then the inverse problem is
    exposed - we may fail to schedule a second (different) state to be
    persisted if the current task is just finishing up (and will exit
    without persisting the now-outstanding work).

(2) Presents a subtle problem relating to when the state of a VBucket
    is materialized. As we only record the vbid to persist (and not
    the state), by the time the VBucketPersistTask runs the actual
    state we /wanted/ to write may have moved forward. Even worse, the
    state could have "gone backwards" (as shown in the MB) if the
    state of the VBucket is read before the vbucket is deleted,
    meaning the task has a 'stale' view of the VBucket object (due to
    us using ref-counted pointers for VBucket objects).

    Additionally, not persisting all intermediate states makes
    debugging harder. We don't actually change VBucket state /that/
    often, and so having all the intermediate states a VBucket went
    through on disk is extremely valuable in debugging.


Instead of having multiple different tasks which can persist state
(and attempting to manage when they are created / when they run / what
state they persist), we instead use a single task (the Flusher task)
to persist state for a given vBucket. Note the Flusher *already*
persists vbucket state if necessary during commit (see
EventuallyPersistentStore::flushVBucket), so this path just adopts the
Flusher as the canonical Task to perform vbstate persistence.

To ensure that state is persisted even when there are no outstanding
'normal' items in the vbucket's checkpoint queue, we use the new
queue_op::set_vbucket_state meta-item to signify that a persist is
pending (and to essentially make the pending items queue non-empty so
the flusher will run).

After this patch all the other Tasks which used to persist vbucket
state are redundent - a subsequent patch will remove them.

Change-Id: Ic44360a1dd14fb882ebfa6f28f4ccfe6127d17a8
Reviewed-on: http://review.couchbase.org/69150
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [16/N]: Add queue_op::set_vbucket_state meta-item 49/69149/7
Dave Rigby [Mon, 24 Oct 2016 11:01:15 +0000 (12:01 +0100)]
MB-20852 [16/N]: Add queue_op::set_vbucket_state meta-item

Add a new meta-item to queue_op enum - set_vbucket_state. This will be
used to mark that a VBucket should have it's state persisted.

Change-Id: I108befd70933962d262529ee230382e47c64e31a
Reviewed-on: http://review.couchbase.org/69149
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [15/N]: Accurately track meta items within checkpoints 20/69020/8
Dave Rigby [Tue, 11 Oct 2016 16:14:03 +0000 (17:14 +0100)]
MB-20852 [15/N]: Accurately track meta items within checkpoints

Instead of assuming that a Checkpoint only contains 1 (Open) or 2
(Closed) meta-items, maintain a count of items within each Checkpoint,
and track how many meta-items a CheckpointCursor has read.

This allows us to support an arbitrary number of meta-items within a
Checkpoint, and in any sequence. This feature will be used to add
support for set_vbstate meta-items in a subsquent patches.

Change-Id: I8fb3040cbe64e316aae1f693afee65001b2b4b17
Reviewed-on: http://review.couchbase.org/69020
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [14/N]: Improve debug/logging in CheckpointManager 21/69021/8
Dave Rigby [Thu, 20 Oct 2016 13:58:01 +0000 (14:58 +0100)]
MB-20852 [14/N]: Improve debug/logging in CheckpointManager

Include information on the CheckpointCursors associated with a
CheckpointManager by adding a operator<< for CheckpointCursor. Add a
dump() method to CheckpointManager to assist in debugging their
contents (e.g. from gdb).

Also add some additional CheckpointManager/CouchKVStore logging.

Change-Id: I8c3de5b5ec0e8e297db8530dee87ac0edd869a91
Reviewed-on: http://review.couchbase.org/69021
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [13/N]: Checkpoint: Add getNumMetaItems() method 18/69018/8
Dave Rigby [Thu, 13 Oct 2016 16:08:53 +0000 (17:08 +0100)]
MB-20852 [13/N]: Checkpoint: Add getNumMetaItems() method

Add a new method to Checkpoint which returns the number of metaItems
in the checkpoint.

Initially this just returns a fixed value of 1 if the checkpoint is
open, or 2 if closed, as this matches the current checkpoint meta item

However subsequent patches will modify this to track the count of how
many meta items actually are in the checkpoint and hence allow us to
determine the meta item count when n queue_op::set_vbstate is added.

Change-Id: I411f2e97e16f9b11ac19a1b7165e4767d09f37d1
Reviewed-on: http://review.couchbase.org/69018
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [12/N]: Add VBucket::getVBucketState method, use vector for VBuckets in Map 89/67889/12
Dave Rigby [Wed, 21 Sep 2016 14:38:47 +0000 (15:38 +0100)]
MB-20852 [12/N]: Add VBucket::getVBucketState method, use vector for VBuckets in Map

Add a method which will encode the current state of the VBucket into a
vbucket_state struct.

Move from a raw array new to std::vector for VBuckets.

Change-Id: I4755568f977b6e97e7ed9e3bdc64a76d842b6ebd
Reviewed-on: http://review.couchbase.org/67889
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [11/N]: Move persistenceCheckpoint id to VBucket 77/67877/14
Dave Rigby [Tue, 13 Sep 2016 16:22:38 +0000 (17:22 +0100)]
MB-20852 [11/N]: Move persistenceCheckpoint id to VBucket

Currently the VBucketMap owns the persistence checkpoint IDs of all
VBuckets. Given these are properties of the VBuckets themselves (for
example one cannot form a vbucket_state record without one) it seems
more logical to have the VBucket class itself own this property.

Change-Id: I50260da628f543365081e736fdd55056a6004cd9
Reviewed-on: http://review.couchbase.org/67877
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [10/N]: Return by value from VBucket::getPersistedSnapshot 75/68275/8
Dave Rigby [Mon, 3 Oct 2016 09:23:39 +0000 (10:23 +0100)]
MB-20852 [10/N]: Return by value from VBucket::getPersistedSnapshot

Simplifies usage & makes it explicit that all members of the range are

Change-Id: I43159f7dc4836b4d30c98d0e07bf570f3f34cc1e
Reviewed-on: http://review.couchbase.org/68275
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agoMB-20852 [9/N]: Explicilty handle all queue_op uses 61/68161/8
Dave Rigby [Thu, 29 Sep 2016 14:05:14 +0000 (15:05 +0100)]
MB-20852 [9/N]: Explicilty handle all queue_op uses

Remove instances of switch statements where we don't check all
enumerations in queue_op, and instead handle them all. Makes it easier
/ safer to add any new queue_op types (and ensure they are handled

Change-Id: I9406c6d327d572268afa4d1830d690ee8ec153a1
Reviewed-on: http://review.couchbase.org/68161
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [8/N]: Improve documentation of putCursorsInCollapsedChk 17/69017/5
Dave Rigby [Tue, 11 Oct 2016 14:12:03 +0000 (15:12 +0100)]
MB-20852 [8/N]: Improve documentation of putCursorsInCollapsedChk

Change-Id: Ibe29d7507aea0f49593454416e9e58e4994ffa4e
Reviewed-on: http://review.couchbase.org/69017
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [7/N]: CheckpointManager::queueDirty: Pass vb by reference 16/69016/5
Dave Rigby [Wed, 19 Oct 2016 10:26:33 +0000 (11:26 +0100)]
MB-20852 [7/N]: CheckpointManager::queueDirty: Pass vb by reference

The VBucket argument to queueDirty is passed via a (reference to) a
ref-counted pointer. Nothing in this function modifies the arguments'
ref count, or passes it another owner; moreover it is also never null.

Therefore change to pass as a reference.

See also:

Change-Id: Iccfeb42922da558b6e9ab430b96829002e85af4a
Reviewed-on: http://review.couchbase.org/69016
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [6/N]: Simplify {start,stop}Flusher & flushVBucket, move to C++11 90/67890/8
Dave Rigby [Thu, 29 Sep 2016 14:24:38 +0000 (15:24 +0100)]
MB-20852 [6/N]: Simplify {start,stop}Flusher & flushVBucket, move to C++11

Move to more C++11 style, and add some comments as to what certain
members mean.

Change-Id: Ibdb900cfe63a9b1352fa1020f9dc58839e3e446d
Reviewed-on: http://review.couchbase.org/67890
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-20852 [5/N]: Checkpoint: C++11-ification 15/69015/6
Dave Rigby [Tue, 11 Oct 2016 12:37:10 +0000 (13:37 +0100)]
MB-20852 [5/N]: Checkpoint: C++11-ification

Convert more of the checkpoint code to C++11 - mostly using auto for
long type names.

Change-Id: I16138f6b16f72e065ecc2c82d973caae7786f846
Reviewed-on: http://review.couchbase.org/69015
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agoMB-20852 [4/N]: Use named struct when moving cursors between checkpoints 14/69014/7
Dave Rigby [Tue, 11 Oct 2016 12:33:38 +0000 (13:33 +0100)]
MB-20852 [4/N]: Use named struct when moving cursors between checkpoints

Instead of an anonymous std::pair (which is hard to follow what the
two fields are), use a named struct `CursorPosition` and related
CursorIdToPositionMap map when recording cursor positions to move
between checkpoints.

Change-Id: I39d14dbdae34cd20fe880e23b95d01c5fbfdac91
Reviewed-on: http://review.couchbase.org/69014
Reviewed-by: Jim Walker <jim@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [3/N]: checkpoint_test enhancements 13/69013/6
Dave Rigby [Mon, 10 Oct 2016 13:44:44 +0000 (14:44 +0100)]
MB-20852 [3/N]: checkpoint_test enhancements

Tighten up existing checkpoint tests by adding additional EXPECT

Add a new CursorMovementReplicaMerge test which tests merging of
cursors on a replica VBucket.

Change-Id: Ia46812c2cd61eeea821a4b2ea46f645e1c66cf7e
Reviewed-on: http://review.couchbase.org/69013
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agoMB-20852 [2/N]: Convert queue_operation to scoped enum 60/68160/12
Dave Rigby [Thu, 29 Sep 2016 13:52:16 +0000 (14:52 +0100)]
MB-20852 [2/N]: Convert queue_operation to scoped enum

In preparation for adding new queue_op for setVBucketState, convert to
a typesafe C++11 scoped enum. Improve the documentation around
queue_op, and related classes Checkpoint, CheckpointCursor.

Also improve the output streaming (operator<<) for Checkpoint class.

Change-Id: I8f29b8e9e8915a68e31550b78bf3131b3737e2d7
Reviewed-on: http://review.couchbase.org/68160
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852 [1/N]: Update tests to facilitate set_vbucket_state changes 10/69010/10
Dave Rigby [Wed, 19 Oct 2016 11:58:11 +0000 (12:58 +0100)]
MB-20852 [1/N]: Update tests to facilitate set_vbucket_state changes

Use checkXX() macros instead of cb_assert() in a number of test which
check for number of items enqueued.

Also adjust the tests to find the initial value of ep_total_enqueued
before storing items, and use this when checking for expected
values. This facilitates the tests working once the changes for
MB-20852 add the set_vbucket_state meta-item to disk queues.

Change-Id: I0108cc5b635b5376d5852b513a6ed47e67f1e62b
Reviewed-on: http://review.couchbase.org/69010
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agoAdd error checking to test_access_scanner 12/69012/6
Dave Rigby [Mon, 17 Oct 2016 13:32:26 +0000 (14:32 +0100)]
Add error checking to test_access_scanner

test_access_scanner doesn't check if store() has actually succeeded
when populating data. Fix this.

Change-Id: Ibd4f97b3b47c15b81a7671d5ed32c15aa4796a40
Reviewed-on: http://review.couchbase.org/69012
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agowait_for_flusher_to_settle: Use wait_for_stat_to_be() 11/69011/5
Dave Rigby [Fri, 14 Oct 2016 16:04:34 +0000 (17:04 +0100)]
wait_for_flusher_to_settle: Use wait_for_stat_to_be()

wait_for_stat_to_be() records the last value (in current) which makes
it easier to debug issues when values arn't as expected. Additionally
this simplifies the code in wait_for_flusher_to_settle.

Change-Id: I601ab4e8b605c937a86b802746cad691ac659308
Reviewed-on: http://review.couchbase.org/69011
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agoMB-21448: replace-with-CAS should return NOT_FOUND for deleted items 80/69080/4
Dave Rigby [Fri, 21 Oct 2016 14:36:38 +0000 (15:36 +0100)]
MB-21448: replace-with-CAS should return NOT_FOUND for deleted items

Regression test for MB-21448 - if an attempt is made to perform a CAS
operation on a logically deleted item we should return NOT_FOUND (aka

The reason for this "intermittent" behaviour is that we are
essentially racing with the writer thread. The sequence of events for
deleting an item is as follows:

1. User calls DELETE, which locates the key in the HashTable, and then
   sets the deleted flag on that StoredValue (but crucially leaves the
   StoredValue in the HashTable). A deletion operation is then
   enqueued in the CheckpointManager (to write the deletion to disk).

2. When the flusher runs for this vBucket, it calls
   CouchKVStore::commit which after committing invokes
   PersistenceCallback::callback to inform ep-engine that the item has
   been persisted to disk (and we can now remove from the HashTable).

3. The callback locates the item in the HashTable and finally
   physically removes the StoredValue.

The issue here is the client is essentially racing with (3) above. If
the client replace-with-CAS operation is processed before (3), then
the StoredValue is still in the HashTable, and hence unlocked_set
returns NOT_FOUND (aka KEY_ENOENT):

    } else if (cas && cas != v->getCas()) {  // line 1078
        if (v->isTempDeletedItem() || v->isTempNonExistentItem()) {
            return NOT_FOUND;

If on the other hand the client replace-with-CAS operation is
processed after (3), then the StoredValue no longer exists in the
HashTable, and we instead hit this case where v is NULL:

    if (v) {
    } else if (cas != 0) {  // line 1102
        rv = NOT_FOUND;

Solution is to make the unlocked_set behave as if an item doesn't
exist (i.e. post (3) state), even if it's currently present but

Change-Id: If353a310e5096dc838c6db9f673d85515d090842
Reviewed-on: http://review.couchbase.org/69080
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agoMB-21475: Account for memory alloc/dealloc in stopTaskGroup 84/69184/2
Dave Rigby [Tue, 25 Oct 2016 12:23:13 +0000 (13:23 +0100)]
MB-21475: Account for memory alloc/dealloc in stopTaskGroup

While stopping a task group, any memory allocations/deallocations made
should be accounted to the bucket in question.  Hence no
`onSwitchThread(NULL)` call.

(Note: This is very similar to MB-20054).

Change-Id: Ib5f88fe8e4061ac827a28e267717df32131aba23
Reviewed-on: http://review.couchbase.org/69184
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agoMB-21178: In 'test_dcp_producer_stream_backfill_no_value' ENOMEM = ETMPFAIL 14/69214/2
Manu Dhundi [Tue, 25 Oct 2016 20:51:49 +0000 (13:51 -0700)]
MB-21178: In 'test_dcp_producer_stream_backfill_no_value' ENOMEM = ETMPFAIL

Change-Id: I38d59b0618c435dd984b5af68b4c8b9c57345133
Reviewed-on: http://review.couchbase.org/69214
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-20746: Delete bloom filter only if temporary filter is created 00/69100/9
Sriram Ganesan [Sat, 22 Oct 2016 01:09:35 +0000 (18:09 -0700)]
MB-20746: Delete bloom filter only if temporary filter is created

During compaction, a temporary filter is only created in the
bloom filter callback function. There is a possibility that this
function will not get invoked if there is no data present during
compaction. So, before swapping the filter, ensure that a temporary
filter is created before deleting the original filter.

Change-Id: I4fcf11d32674be8c28dbda02724b40d3d37e335a
Reviewed-on: http://review.couchbase.org/69100
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Sriram Ganesan <sriram@couchbase.com>
4 years agoMB-21320: Update handleSlowStream log to match the pattern of other DCP logs 73/69173/2
Manu Dhundi [Tue, 25 Oct 2016 01:20:38 +0000 (18:20 -0700)]
MB-21320: Update handleSlowStream log to match the pattern of other DCP logs

Change-Id: I2e9f1aecf66f7ee598d3c12d4be8ab875327bcf8
Reviewed-on: http://review.couchbase.org/69173
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-21320: Remove unused 'schedule' flag in DcpProducer::notifyStreamReady 72/69172/2
Manu Dhundi [Tue, 25 Oct 2016 01:13:55 +0000 (18:13 -0700)]
MB-21320: Remove unused 'schedule' flag in DcpProducer::notifyStreamReady

Change-Id: Ia562ec7baac012d401cc420982431df9920e92d3
Reviewed-on: http://review.couchbase.org/69172
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoLoggedAtomic: Expand set of operations supported, serialize logging 09/69009/4
Dave Rigby [Wed, 19 Oct 2016 10:20:40 +0000 (11:20 +0100)]
LoggedAtomic: Expand set of operations supported, serialize logging

Add support for increment & decrement (fetch_{add,sub}), assignment
and conversion to T.

Also add a mutex to serialize access to stderr, to prevent corruption
in log messages.

Change-Id: I0a617f702a22d287fb87bd1001ee0300b6551b9e
Reviewed-on: http://review.couchbase.org/69009
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
Reviewed-by: Will Gardner <willg@rdner.io>
4 years agoMB-20247: Check callback return value for failure 37/68937/3
Jim Walker [Wed, 19 Oct 2016 16:04:18 +0000 (17:04 +0100)]
MB-20247: Check callback return value for failure

The batchWarmupCallback applies many items to the vbucket yet
is not coded to handle failures, e.g. ENOMEM.

These errors are now checked for and the warmup stops when an error
is found.

Testing of this is difficult as it was always quite theoretical:

1. The real failure case here is very hard to hit, I think if you
warmup close to DGM whilst racing deletes/evict from the frontend
you could encounter this situation.

2. warmup has very little unit-testing that can be adapted.

However I've built an instrumented ep-engine that would force the
error condition after n callbacks and I've ran that version under
valgrind and ASAN.

Change-Id: I05d35e10e577a3b5c2c7d21807996ab7b8455cc1
Reviewed-on: http://review.couchbase.org/68937
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-21143: Don't store conflict resolution mode per document 30/68930/4
Jim Walker [Wed, 19 Oct 2016 14:08:10 +0000 (15:08 +0100)]
MB-21143: Don't store conflict resolution mode per document

Some more deadcode that at the moment returns an unitialised
value as there's no conflict mode to return.

Change-Id: I7c0230ff4167d81dabb6dcbcaa5b69e461f7a850
Reviewed-on: http://review.couchbase.org/68930
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years ago[BP] MB-20585: Fix memory leak 36/68936/3
Jim Walker [Wed, 19 Oct 2016 15:28:39 +0000 (16:28 +0100)]
[BP] MB-20585: Fix memory leak

Only one of the MB-20585 fixes needs backporting and
a fix to two tests that came from watson->master merge

Orginal MB commit to master is a288f6c6
Test fix came from 0f532bce

Change-Id: I200706997df2d24a7629282e119b0ab8af2c4872
Reviewed-on: http://review.couchbase.org/68936
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-21428: Make sure all tasks in fake executor pool finish execution in teardown 68/68968/2
Manu Dhundi [Wed, 19 Oct 2016 23:11:09 +0000 (16:11 -0700)]
MB-21428: Make sure all tasks in fake executor pool finish execution in teardown

It is not sufficient to just run all tasks in future and ready queues while
shutdown, we also need to make sure they finish before delete
EventuallyPersistentEngine instance.

Change-Id: I658a2176b6f4a99beacbc8dd2e010b93c4480eaa
Reviewed-on: http://review.couchbase.org/68968
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-21300: [Address Sanitizer] Prevent underflow during hash table resize() 87/68887/2
Manu Dhundi [Tue, 18 Oct 2016 20:51:10 +0000 (13:51 -0700)]
MB-21300: [Address Sanitizer] Prevent underflow during hash table resize()


If i == 0 in line 144 of HashTable::resize(), reading prime_size_table[i - 1]
is spurious.

Change-Id: I7f3135efff7a0993fb939707b912d064514e9a45
Reviewed-on: http://review.couchbase.org/68887
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoReduce log level from WARNING to NOTICE on some events 57/68857/2
Trond Norbye [Fri, 14 Oct 2016 09:26:19 +0000 (11:26 +0200)]
Reduce log level from WARNING to NOTICE on some events

Change-Id: Ice0d3391778a0c4a82739b1a206cb4fe17bc23dc
Reviewed-on: http://review.couchbase.org/68857
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-20247: Check scan return code 01/68801/2
Jim Walker [Mon, 17 Oct 2016 10:21:40 +0000 (11:21 +0100)]
MB-20247: Check scan return code

kvstore->scan can fail, so check the return code to prevent trying
to continue loading whilst failing.

Extension of changes made for MB-16910

Change-Id: Ia890d685d7acb144eca374ca0964e3d65100f89e
Reviewed-on: http://review.couchbase.org/68801
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21320: Improve debug when handling slow streams 98/68798/3
Dave Rigby [Thu, 13 Oct 2016 11:16:49 +0000 (12:16 +0100)]
MB-21320: Improve debug when handling slow streams

Change-Id: Ia24e955fa975ea538680d5b80150823382a988b9
Reviewed-on: http://review.couchbase.org/68798
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
4 years agoMB-21325: Fix race in BloomFilter::getNumOfKeysInFilter 03/68803/2
Dave Rigby [Mon, 17 Oct 2016 10:36:50 +0000 (11:36 +0100)]
MB-21325: Fix race in BloomFilter::getNumOfKeysInFilter

Address race condition identified by ThreadSanitizer on

WARNING: ThreadSanitizer: data race (pid=341)
  Write of size 8 at 0x7d140000df80 by thread T11 (mutexes: write M20582, write M20817, write M19369):
    #0 BloomFilter::addKey() ep-engine/src/bloomfilter.cc:119 (ep.so+0x00000002453d)
    #1 VBucket::addToFilter() ep-engine/src/vbucket.cc:482 (ep.so+0x000000149704)
    #2 PersistenceCallback::callback() ep-engine/src/ep.cc:3211 (ep.so+0x0000000b27c1)
    #3 non-virtual thunk to PersistenceCallback::callback(int&) ep-engine/src/ep.cc:3167 (ep.so+0x0000000b36c2)
    #4 CouchKVStore::commitCallback() ep-engine/src/couch-kvstore/couch-kvstore.cc:1898 (ep.so+0x00000017aa51)
    #5 _ZN12CouchKVStore17commit2couchstoreEP8CallbackIJ10KVStatsCtxEE ep-engine/src/couch-kvstore/couch-kvstore.cc:1712 (ep.so+0x00000017208b)
    #6 _ZN12CouchKVStore6commitEP8CallbackIJ10KVStatsCtxEE ep-engine/src/couch-kvstore/couch-kvstore.cc:954 (ep.so+0x000000171c92)
    #7 EventuallyPersistentStore::commit(unsigned short) ep-engine/src/ep.cc:3451 (ep.so+0x0000000a29e6)
    #8 EventuallyPersistentStore::flushVBucket(unsigned short) ep-engine/src/ep.cc:3399 (ep.so+0x0000000a1935)
    #9 Flusher::flushVB() ep-engine/src/flusher.cc:293 (ep.so+0x0000001038ae)
    #10 Flusher::step(GlobalTask*) ep-engine/src/flusher.cc:183 (ep.so+0x000000101f7e)
    #11 FlusherTask::run() ep-engine/src/tasks.cc:138 (ep.so+0x00000013ac42)
    #12 ExecutorThread::run() ep-engine/src/executorthread.cc:115 (ep.so+0x0000000fd53f)
    #13 launch_executor_thread(void*) ep-engine/src/executorthread.cc:33 (ep.so+0x0000000fd095)
    #14 platform_thread_wrap(void*) platform/src/cb_pthreads.cc:55 (libplatform_so.so.0.1.0+0x000000005deb)

  Previous read of size 8 at 0x7d140000df80 by main thread (mutexes: write M15538, write M1828880534441356728):
    #0 BloomFilter::getNumOfKeysInFilter() ep-engine/src/bloomfilter.cc:142 (ep.so+0x0000000246a0)
    #1 VBucket::addStats() ep-engine/src/vbucket.cc:597 (ep.so+0x00000014a527)
    #2 EventuallyPersistentEngine::doVBucketStats() ep-engine/src/ep_engine.cc:3837 (ep.so+0x0000000c69f6)
    #3 EventuallyPersistentEngine::doVBucketStats() ep-engine/src/ep_engine.cc:3861 (ep.so+0x0000000c664a)
    #4 EventuallyPersistentEngine::getStats() ep-engine/src/ep_engine.cc:4774 (ep.so+0x0000000cbbb3)
    #5 EvpGetStats() ep-engine/src/ep_engine.cc:231 (ep.so+0x0000000baafe)
    #6 mock_get_stats() memcached/programs/engine_testapp/engine_testapp.cc:215 (engine_testapp+0x0000004ce40d)
    #7 std::string get_stat<std::string>() ep-engine/tests/ep_test_apis.cc:1140 (ep_testsuite_xdcr.so+0x000000020c34)
    #8 unsigned long get_stat<unsigned long>() ep-engine/tests/ep_test_apis.cc:1179 (ep_testsuite_xdcr.so+0x00000002089b)
    #9 get_ull_stat() ep-engine/tests/ep_test_apis.cc:1174 (ep_testsuite_xdcr.so+0x00000001e73e)
    #10 test_del_with_meta_and_check_drift_stats() ep-engine/tests/ep_testsuite_xdcr.cc:1711 (ep_testsuite_xdcr.so+0x0000000143f1)
    #11 execute_test() memcached/programs/engine_testapp/engine_testapp.cc:1118 (engine_testapp+0x0000004cc997)
    #12 __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226 (libc.so.6+0x00000002176c)

Change-Id: I4e9a589e9286e72646df515db891c57b143d1fdd
Reviewed-on: http://review.couchbase.org/68803
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
4 years agoMB-21190: cbepctl support to change max_cas 36/68736/2
Jim Walker [Fri, 14 Oct 2016 10:30:29 +0000 (11:30 +0100)]
MB-21190: cbepctl support to change max_cas

The initial version of this commit blocks *all* set commands
as it required they all pass a vbucket-id even when not

This update means that changing max_cas is done via

 cbepctl set_vbucket_param

That command requires the vbucket-id.

This commit changes (reverts) to not requiring the extra CLI argument.

 cbepctl set

Change-Id: I04b8b607ccfbc56bca46fe55ca0360f0fe7874ef
Reviewed-on: http://review.couchbase.org/68736
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-19746: Add xml output to ep_perfsuite 97/68097/5
Matt Carabine [Fri, 17 Jun 2016 11:16:11 +0000 (12:16 +0100)]
MB-19746: Add xml output to ep_perfsuite

- The new performance commit validation, CBNT, requires input data
  in XML format which mirrors that of the GTest XML output
- This commit adds this functionality to ep_perfsuite, providing
  the ability to output the results in textual format or xml
- It can be invoked by using the new `-f` flag, followed by either
  `xml` or `text`, if this flag is not specified then the default
  behaviour of textual output is assumed

Change-Id: I153a816493181acb091e8d4433f5be543742231f
Reviewed-on: http://review.couchbase.org/68097
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agodefragmenter_test: Run for fewer iterations under Valgrind 68/68568/3
Dave Rigby [Wed, 3 Aug 2016 13:06:19 +0000 (13:06 +0000)]
defragmenter_test: Run for fewer iterations under Valgrind

Currently when this test is run under Valgrind it takes ~150s to
complete; because we create a large (500,000) number of items to
operate on for the benchmark tests. There's little value in
benchmarking under Valgrind, however it is still useful to run the
tests under Valgrind to validate functional correctness.

Therefore reduce the item count to 10 when run under Valgrind. This
reduces the runtime to <10s

Change-Id: Idda46dff5963c29db6890b0c8841c80b1401c010
Reviewed-on: http://review.couchbase.org/68568
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21190: cbepctl support to change max_cas 17/68517/8
Jim Walker [Fri, 7 Oct 2016 10:37:48 +0000 (12:37 +0200)]
MB-21190: cbepctl support to change max_cas

If the max_cas of a vbucket is forced forward by a
peer with a 'bad' clock, there are limited recovery
options. Allowing the max_cas to be "reset" could
be useful to recover from such a problem.

Change-Id: I9235520283ee1cd0d5b49820190a9eed3daac3c2
Reviewed-on: http://review.couchbase.org/68517
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21154: set_param support for drift thresholds 25/68325/11
Jim Walker [Tue, 4 Oct 2016 14:56:20 +0000 (15:56 +0100)]
MB-21154: set_param support for drift thresholds

Enable the set_param command to change the ahead and behind drift
thresholds, enabling changes without a bucket restart.

cbepctl is updated so that it can send drift threshold changes,
but they're not publicised.

> cbepctl ... set vbucket_param hlc_drift_ahead_threshold_us 1

Change-Id: I4973d0c36bfa03ff33e50924b7c10434675d90da
Reviewed-on: http://review.couchbase.org/68325
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21150: A single total for drift ahead exceptions 72/68272/11
Jim Walker [Mon, 3 Oct 2016 13:33:25 +0000 (14:33 +0100)]
MB-21150: A single total for drift ahead exceptions

Report a stat that captures every time we've set a vbucket's HLC
from a peer who is ahead of the threshold of the node.

Change-Id: I7b7d73b5044afd2a4cfcc2d8ed43e9018b478f19
Reviewed-on: http://review.couchbase.org/68272
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20725: LWW monitoring 58/68158/13
Jim Walker [Thu, 29 Sep 2016 14:58:18 +0000 (15:58 +0100)]
MB-20725: LWW monitoring

Add a number of new stats to the bucket stats and vbucket-details.

Add drift +/- threshold config parameters.

engine stats:

1. The maximum absolute cummulative-drift of the active vbuckets. See
    vbucket-details total_abs_drift

2. The number of updates applied to ep_active_hlc_drift

3. The maximum absolute drift of the replica vbuckets

4. The number of counts applied to ep_replica_hlc_drift

5. The total number of times a setMaxCas was from a peer who is
   ahead of the ahead threshold (hlc_ahead_threshold_us).

6. The total number of times a setMaxCas was from a peer who is
   behind of the behind threshold (hlc_behind_threshold_us).

7. The total number of times a setMaxCas was from a peer who is
   ahead of the ahead threshold (hlc_ahead_threshold_us) for
   replica VBs.

8. The total number of times a setMaxCas was from a peer who is
   behind of the behind threshold (hlc_behind_threshold_us) for
   replica VBs.

vbucket-details stats:

1. The current max_hlc (reported as max_cas for compatibility with
   other modules)

2. The vbucket's absolute cummulative drift

3. How many updates have been applied to total_abs_drift

4. How many times the ahead threshold has been exceeded.

5. How many times the behind threshold has been exceeded.

6. How many logical clock ticks this vbucket's HLC has returned

Change-Id: I063782d4451b97f58a3c89208506bd8bd08b705e
Reviewed-on: http://review.couchbase.org/68158
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20819: Use std::chrono::system_clock for HLC 38/68038/13
Jim Walker [Thu, 29 Sep 2016 14:56:24 +0000 (15:56 +0100)]
MB-20819: Use std::chrono::system_clock for HLC

Use a clock that follows the system clock for generation of HLC

Change-Id: Ibb928c3ea4b83fbcf0862e8c610b1c49154eb0ed
Reviewed-on: http://review.couchbase.org/68038
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20798: Allow CAS and seqno to be generated consistently 70/67670/16
Jim Walker [Tue, 27 Sep 2016 11:26:44 +0000 (12:26 +0100)]
MB-20798: Allow CAS and seqno to be generated consistently

Add a new option to queueDirty so that it can generate
and assign the CAS to the StoredValue.

This allows us to create a seqno and CAS under the same
lock, thus seqno and CAS will be incrementing when
a checkpoint is serially observed.

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