ep-engine.git
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
commit.

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()

http://cv.jenkins.couchbase.com/job/ep-engine-addresssanitizer-master/385

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
BloomFilter::keyCounter.

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
required.

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
        ep_active_hlc_drift

2. The number of updates applied to ep_active_hlc_drift
        ep_active_hlc_drift_count

3. The maximum absolute drift of the replica vbuckets
        ep_replica_hlc_drift

4. The number of counts applied to ep_replica_hlc_drift
        ep_replica_hlc_drift_count

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

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

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.
        ep_replica_ahead_exceptions

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.
        ep_replica_behind_exceptions

vbucket-details stats:

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

2. The vbucket's absolute cummulative drift
        vb_n:total_abs_drift

3. How many updates have been applied to total_abs_drift
        vb_n:total_abs_drift_count

4. How many times the ahead threshold has been exceeded.
        vb_n:drift_ahead_threshold_exceeded

5. How many times the behind threshold has been exceeded.
        vb_n:drift_behind_threshold_exceeded

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

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
time-stamps.

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>
4 years agoMB-20798: Update queueDirty options in prep for a new option 82/67582/16
Jim Walker [Tue, 13 Sep 2016 19:51:32 +0000 (20:51 +0100)]
MB-20798: Update queueDirty options in prep for a new option

Switch the bool 'genBySeqno' to a more descriptive enum, call
site will now be clearer regarding the parameter.

Change-Id: I2d6707df0360f02f7455b480383f5ca9d6e64261
Reviewed-on: http://review.couchbase.org/67582
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21143: Remove adjusted-time/drift and associated code 99/67999/10
Jim Walker [Mon, 26 Sep 2016 13:36:01 +0000 (14:36 +0100)]
MB-21143: Remove adjusted-time/drift and associated code

As part of simplfying the supported LWW code, remove the
adjusted-time API and associated code.

Change-Id: I4d1cb092d4fce3155d1cd1e0134333655bcb3a61
Reviewed-on: http://review.couchbase.org/67999
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21143: Don't store conflict resolution mode per document 55/67955/9
Jim Walker [Fri, 23 Sep 2016 13:45:40 +0000 (14:45 +0100)]
MB-21143: Don't store conflict resolution mode per document

Disk and memory have storage on a per document basis for the
conflict resolution mode.

LWW is now enabled globally so this meta data is wasted.

Change-Id: I7b54a96f453b4182e0976e6c18cb48ac964e5177
Reviewed-on: http://review.couchbase.org/67955
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21144: Simplify how LWW is enabled 54/67954/9
Jim Walker [Fri, 23 Sep 2016 16:01:27 +0000 (17:01 +0100)]
MB-21144: Simplify how LWW is enabled

Use existing conflict_resolution_type config flag to enable
LWW on a global bucket basis. Now ignoring the per document
LWW flag.

Change-Id: I2a19fd5633ec6cf28deead3cb5a371e242131fc6
Reviewed-on: http://review.couchbase.org/67954
Reviewed-by: Dave Rigby <daver@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMove more unit tests into the ep_unit_tests binary 12/68212/7
Dave Rigby [Fri, 12 Aug 2016 14:53:16 +0000 (15:53 +0100)]
Move more unit tests into the ep_unit_tests binary

For GTest-style tests we have created a single test binary
(ep-engine_ep_unit_tests) to link all the tests into. This has the
advantage of not having to compile different variants of our source
files for multiple different test binaries (which is partly a
limitation / 'feature' of CMake's dependancy calculation).

However not all tests are in this binary. This patch moves an
additional 2 test suites - checkpoint & defragmenter - into the single
binary. This speeds up our build time, and also removes a bunch of
duplicated boilerplate test setup code.

Change-Id: I7a9b6f8166fe2dcb739bdf124b43d1de6abc1e42
Reviewed-on: http://review.couchbase.org/68212
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
4 years agoRemove hooksApi global; reduce coupling with MemoryTracker 10/68210/6
Dave Rigby [Fri, 12 Aug 2016 13:45:20 +0000 (14:45 +0100)]
Remove hooksApi global; reduce coupling with MemoryTracker

MemoryTracker is somewhat tightly coupled with ep_engine.cc as it uses
the getHooksApi() function to obtain the memory allocator hooks.

Firstly this makes it hard to test - compile one file and you have to
include the other, and it's also difficult to provide a different
hooks api - either for injecting a mock one for testing, or simply to
use the 'normal' hooks API but without pulling in ep-engine.

Secondly, there is unnecessary indirection in NewHook / DeleteHook -
which are called on every new/delete so performance is relevant
there. By giving the MemoryTracker it's own copy of the alloc_hooks
(instead of calling getHooksApi() on every call) we can reduce the
amount of work in NewHook / DeleteHook by approx. 50% (measured in
terms of x64 instructions).

Change-Id: Ia0f8ebb0a5263567dc08b32fe6ff9b7ea9eefa92
Reviewed-on: http://review.couchbase.org/68210
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20822: When loading persisted failovers, remove any erroneous entries 02/67702/8
Manu Dhundi [Tue, 4 Oct 2016 20:31:33 +0000 (13:31 -0700)]
MB-20822: When loading persisted failovers, remove any erroneous entries

Due to bugs in older releases (and also possibly in future releases),
we may end up storing wrong failover table on disk. Hence during
warmup while loading failover table from disk we must prune out any
wrong entries.

Also, in the past we have seen erroneous entries with vb_uuid == 0.
So we make sure that vb_uuid is not generated as a valid value and
prune out any entry that has vb_uuid == 0

Change-Id: I630cb7fb1ea9a711432be64f36924d04fcd5e361
Reviewed-on: http://review.couchbase.org/67702
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-21193: Fix ConnectionTest failure when test runs for >21s 73/68273/3
Dave Rigby [Mon, 3 Oct 2016 16:41:14 +0000 (17:41 +0100)]
MB-21193: Fix ConnectionTest failure when test runs for >21s

When CV jobs take longer than normal (e.g. when running under
ThreadSanitizer) the ConnectionTest can fail:

    [ RUN      ] ConnectionTest.test_maybesendnoop_buffer_full
    /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-watson/ep-engine/tests/module_tests/dcp_test.cc:308: Failure
    Value of: ret
      Actual: 255
    Expected: ENGINE_E2BIG
    Which is: 8
    maybeSendNoop not returning ENGINE_E2BIG
    [  FAILED  ] ConnectionTest.test_maybesendnoop_buffer_full (656 ms)

The test is assuming that it starts running in less than 21 seconds
when setting the DCP noop timeout. Instead of using an absolute '21'
we need to use the current_time + 21.

Change-Id: Id4fe8b372da4c447e00128de2c94c72bb60373e4
Reviewed-on: http://review.couchbase.org/68273
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agoMB-21142: Ensure vbucket states are persisted before the test 64/68264/2
Sriram Ganesan [Mon, 3 Oct 2016 09:14:18 +0000 (14:44 +0530)]
MB-21142: Ensure vbucket states are persisted before the test

In test_tap_filter_stream, vb ids 0 to 3 are set to active
before the test. The test needs to ensure that all the vbucket
states are persisted and the respective couchstore files created
so that there is no race between file creation and operations
on the file

Context: test_tap_filter_stream

Change-Id: Idad34474bdf418504012731459cf4eb3272b1ba9
Reviewed-on: http://review.couchbase.org/68264
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-20744: Increment reject ops on insertion to reject queue 19/68219/10
Sriram Ganesan [Mon, 29 Aug 2016 22:48:40 +0000 (15:48 -0700)]
MB-20744: Increment reject ops on insertion to reject queue

In case of a failure to persist items, the items are added to the
reject queue in which the reject stats needs to be updated for
the associated vbucket.

Change-Id: I15b7ad26d8bcab5b6a437b8d172d8b914df8b683
Reviewed-on: http://review.couchbase.org/68219
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-21140: Fix race in test_access_scanner_settings (gmtime_r) 51/68151/4
Dave Rigby [Fri, 23 Sep 2016 13:59:34 +0000 (14:59 +0100)]
MB-21140: Fix race in test_access_scanner_settings (gmtime_r)

When calculating access scanner adjusted time values, use the
thread-safe variant (gmtime_r) so the test doesn't conflict with the
ep_engine code.

Also use std::chrono to perform the time manipulaton (which handles
any modulus of minutes -> hours etc).

Change-Id: Icf8505e4ce465f382904934dcaa05527efc57454
Reviewed-on: http://review.couchbase.org/68151
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20623: Warmup: Process the access log in chunks 45/68045/7
Dave Rigby [Tue, 23 Aug 2016 14:28:07 +0000 (15:28 +0100)]
MB-20623: Warmup: Process the access log in chunks

Instead of loading the entire Access log into memory, and then
applying in one (potentially very large) getMulti; process it in
warmup_batch_size chunks (default 10,000 items).

This sigificantly reduces the amount of temporary memory consumed
during warmup, which in turn means this memory can be used for
document values.

Results: in the following workload we manage to load 19.2M items (or
4.2GB) into memory during warmup, where previously only two items
(yes, *two*) were warmed up due to the size of the temporary data
structures - because the temporary data structures consumed ~4.2GB.

* 1 bucket, 10,000MB quota, 1 node.
* 30M items, 300bytes each; giving a residency ratio of ~60%. Dataset
  generated by:

    cbc-pillowfight -U couchbase://localhost:12000 -I 30000000 -m 300 -M 300 -t16

Change-Id: I511b70d5ea9c9c6b9556249a936030a67bf70c02
Reviewed-on: http://review.couchbase.org/68045
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20623: Warmup: Implement MutationLog::iterator copy assignment 44/68044/8
Dave Rigby [Tue, 23 Aug 2016 14:21:22 +0000 (15:21 +0100)]
MB-20623: Warmup: Implement MutationLog::iterator copy assignment

MutationLog::iterator doesn't follow the Rule of Three - it doesn't
implement the copy-assigment operator. This means that it's not a
complete iterator implementation.

Fix this, and add a unit test for it.

Change-Id: I12d67bc072d72e481e6a195e2d45b16c0318fdc0
Reviewed-on: http://review.couchbase.org/68044
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20623: Remove 'fetches' vector from MutationLogHarvester::apply 43/68043/7
Dave Rigby [Tue, 23 Aug 2016 11:12:05 +0000 (12:12 +0100)]
MB-20623: Remove 'fetches' vector from MutationLogHarvester::apply

MutationLogHarvester::apply() builds a vector of keys (`fetches`) to
fetch from disk. This is essentially identical to the contents of
`committed`, except it only contains keys which currently exist in the
VBucket.

We can optimize this by removing fetches, and instead erasing the
elements from `committed[vb]` which are no longer valid. This also
means we no longer have to check for duplicates in batchWarmupCallback
(which was kinda pointless even before), as the data structure passed
in is now a set (and hence cannot by definition contain dupes).

Results in a reduction in the memory used by these temporary warmup
data structures - from 3876MB to 3218MB (17%) for the following
workload:

* 1 bucket, 10,000MB quota, 1 node.
* 30M items, 300bytes each. Dataset generated by:
    cbc-pillowfight -U couchbase://localhost:12000 -I 30000000 -m 300 -M 300 -t16

Change-Id: I1c2e480e53626be41fa20dcd44abe4d6fa327e4c
Reviewed-on: http://review.couchbase.org/68043
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20623: Remove unused fields from temporary warmup data structures 42/68042/7
Dave Rigby [Mon, 22 Aug 2016 14:43:29 +0000 (15:43 +0100)]
MB-20623: Remove unused fields from temporary warmup data structures

Warmup currently creates three data structures (per vBucket) in memory
when warming up using the access.log:

1. A map of keys -> sequence number at the time the access log was
   generated - MutationLogHarvester::committed

2. A list of {key, hashTable_seqNo} pairs -
   MutationLogHarvester::apply::fetches

3. A map of key -> VBucketBGFetchItem for every key in the list above
- batchWarmupCallback::items2fetch

There are a number of inefficiencies in this implementation, the first
of which is that we record sequence numbers in the first two data
structures which are never actually used - the final BGfetch doesn't
need them.

This patch therefore removes the recording of sequence numbers. This
changes the data structures to:

1. MutationLogHarvester::committed - a set of keys (per vBucket).
2. MutationLogHarvester::apply::fetches - a vector of keys.

Results in a reduction in the memory used by these temporary warmup
data structures - from 4356MB to 3876MB (11%) for the following
workload:

* 1 bucket, 10,000MB quota, 1 node.
* 30M items, 300bytes each. Dataset generated by:
    cbc-pillowfight -U couchbase://localhost:12000 -I 30000000 -m 300 -M 300 -t16

Change-Id: I0666e2d1a8a0d9e996cdbdd61d41118d2c2d6dfc
Reviewed-on: http://review.couchbase.org/68042
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20623: MutationLog: Remove unused Delete and Uncommitted functionality 41/68041/7
Dave Rigby [Mon, 22 Aug 2016 13:10:17 +0000 (14:10 +0100)]
MB-20623: MutationLog: Remove unused Delete and Uncommitted functionality

The MutationLog (when actually used for Mutations) could record
Deletes (in addition to 'adds') of keys, and return a list of
'uncommitted' keys.  However this functionality has long been unused
(since March 2013, http://review.couchbase.org/26943).

Remove this unused code to simplify the way the MutationLog class is
now used - for the access log.

Change-Id: I5642553708c7d5cf320cec5524ae49643a414192
Reviewed-on: http://review.couchbase.org/68041
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20623: Convert MutationLog unit tests to GTest 40/68040/8
Dave Rigby [Tue, 27 Sep 2016 14:00:03 +0000 (15:00 +0100)]
MB-20623: Convert MutationLog unit tests to GTest

Convert to using the GoogleTest framework, and integrate into
ep-engine_ep_unit_tests binary.

Change-Id: I7caff50202abc4ecf55c513f4b6859d5c5d48d40
Reviewed-on: http://review.couchbase.org/68040
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20623: re-enable and fix MutationLog unit test 39/68039/10
Dave Rigby [Mon, 22 Aug 2016 13:46:01 +0000 (14:46 +0100)]
MB-20623: re-enable and fix MutationLog unit test

This test appears to have been not built/run a while back. Resurrect
it.

Change-Id: I4b35291239882e5e58bc2c7d435e3c793a7ae6ba
Reviewed-on: http://review.couchbase.org/68039
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoAdd LoggedAtomic<> debug class 44/67944/6
Dave Rigby [Fri, 23 Sep 2016 10:29:54 +0000 (11:29 +0100)]
Add LoggedAtomic<> debug class

A Debugging wrapper around std::atomic which print all accesses to the
atomic value to stderr.

Drop-in compatible with AtomicValue for simple use-cases - currently
only implements load() / store().

Change-Id: I78cec4d8bad55588900573f201d81a38f16f97ee
Reviewed-on: http://review.couchbase.org/67944
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
4 years agoMB-20822: Erase all diverged branch entries correctly in failover table 81/67481/6
Manu Dhundi [Wed, 14 Sep 2016 19:12:29 +0000 (12:12 -0700)]
MB-20822: Erase all diverged branch entries correctly in failover table

When we add an entry in failover table we must erase all the other
entries with higher seqno because they are from a diverged branch.

This commit fixes a bug in the loop that was erasing diverged entires.
In a std::list when we erase an entry, the function returns the
iterator pointing to next element and hence we must be careful not to
double increment it.

Change-Id: I9275fba8057f26e2853a8d7d359f1d01f107f2be
Reviewed-on: http://review.couchbase.org/67481
Reviewed-by: Dave Rigby <daver@couchbase.com>
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-16181: Add replicate/persist traits to Item 90/67990/4
Jim Walker [Mon, 26 Sep 2016 09:29:10 +0000 (10:29 +0100)]
MB-16181: Add replicate/persist traits to Item

Provide an abstraction for the flusher and DCP that tells them
if an item should be persisted or replicated.

This provides a stepping stone towards system owned items in
checkpoints.

Change-Id: Ie5e65a2b20d0d162e1b8fe3e439219c34fb7b508
Reviewed-on: http://review.couchbase.org/67990
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20869: Simplify test to fix intermittent test_access_scanner failure 84/67884/11
Dave Rigby [Wed, 7 Sep 2016 14:40:08 +0000 (15:40 +0100)]
MB-20869: Simplify test to fix intermittent test_access_scanner failure

In test_access_scanner the check that the number of non-resident items
was less than 6% of the total is incorrect - it was using integer
division (6/100) and hence the RHS of the check was always zero, which
would always succeed. Fix the check so it calculates the percentage
correctly.

Additionally we did not wait for the previous AccessScanner run to
finalize - which meant that the second start of the access acanner
could fail to start (and consequently we would timeout with the
message:

    Exceeded maximum wait time of 60000000us waiting for stat
    'ep_num_access_scanner_runs' to be greater or equal than 8 -
    aborting.

To solve this, change to just one iteration of creating the file (not
sure why it did two before), and move the increment of stats.alogRuns
to /after/ the access log files are renamed.

Change-Id: I6c3b6eeed91302fd7fa67dfade6ae954078fc125
Reviewed-on: http://review.couchbase.org/67884
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years ago[BP] MB-20598: AddressSanitizer problems 06/68106/3
Jim Walker [Fri, 19 Aug 2016 15:19:51 +0000 (15:19 +0000)]
[BP] MB-20598: AddressSanitizer problems

Two leaks and one stack overflow.

The forest-kvstore code should use dynamic_cast (like couch-kvstore)
else when the incoming callback is not a RememberingCallback, we will
access outside of the incoming object.

ep_testsuite has a leak in tap code where we must release items
during iteration.

Change-Id: If0db9e936ee141299c5a579235e828c7309b8118
Reviewed-on: http://review.couchbase.org/68106
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years ago[BP] MB-20598: AddressSanitizer problem in perfsuite 07/68107/2
Jim Walker [Thu, 25 Aug 2016 09:47:14 +0000 (09:47 +0000)]
[BP] MB-20598: AddressSanitizer problem in perfsuite

This test was being OOM killed so was left out of
the orginal set of fixes. Tap iterator mutation/deletions
need to be released by the client.

Change-Id: Ib65f386f1080cb2a130cfdd7d90c85dd4a871989
Reviewed-on: http://review.couchbase.org/68107
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-19578: Fail warmup only when explicitly configured 53/67353/9
Sriram Ganesan [Sat, 3 Sep 2016 23:49:53 +0000 (16:49 -0700)]
MB-19578: Fail warmup only when explicitly configured

- Instead of unconditionally failing warmup in case of a OOM failure,
  fail it only when it is configured to do so. Note that ns_server
  explicitly sets this value to false.
- Remove an unnecessary check in the bucket initialization path to
  check for warmup failures as soon as warmup is started. The warmup
  process is asynchronous and thus checking for failures right away
  is futile.

Change-Id: I5725644a3731e7cd75e98b0b77ca6ae8a3d88ab9
Reviewed-on: http://review.couchbase.org/67353
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20528: unconditionally increment get meta ops 07/67307/12
Sriram Ganesan [Thu, 1 Sep 2016 00:02:55 +0000 (17:02 -0700)]
MB-20528: unconditionally increment get meta ops

The get meta operations prior to introducing bloom filters was
always incremented whether the key was present or not present
in the bucket. Restore the behavior where the get meta ops stat
is always incremented.

Change-Id: I09fadedb71929d0d078d0ca6c49ef27de3fd5bef
Reviewed-on: http://review.couchbase.org/67307
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852: ep_unit_tests_main: Show DEBUG logs with -v 88/67888/5
Dave Rigby [Wed, 21 Sep 2016 14:33:18 +0000 (15:33 +0100)]
MB-20852: ep_unit_tests_main: Show DEBUG logs with -v

Previously we would only print INFO level log messages when the -v
flag was given, even though the code claimed to have enabled DEBUG
level.

This was because we didn't get the log level early enough - the
extension defaults to INFO, and will propogate that down to ep_engine
(overwriting what we specify). Fix by setting the log level in the
server API to debug.

Change-Id: I8a192169721fad631965600b69975d882b6221f8
Reviewed-on: http://review.couchbase.org/67888
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoRemove default parameter from queueDirty 23/67623/12
Daniel Owen [Tue, 13 Sep 2016 11:14:32 +0000 (12:14 +0100)]
Remove default parameter from queueDirty

The EventuallyPersistentStore method queueDirty has a parameter called
notifyReplicator, which is defaulted to true.  The parameter is used
to specify whether or not to notify the replicator.

However the queueDirty method either uses the notifyReplicator default
of true, or is passed in true.

This patch removes the unrequired parameter from the queueDirty
definition and simpifies the body of the method.

Change-Id: I6340e522f0e6137bc744450ddc90e418ed7716a2
Reviewed-on: http://review.couchbase.org/67623
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Reviewed-by: Sriram Ganesan <sriram@couchbase.com>
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20834: Use get_available_cpu_count() for executorpool #threads calculation 93/67493/3
Dave Rigby [Thu, 8 Sep 2016 13:32:45 +0000 (14:32 +0100)]
MB-20834: Use get_available_cpu_count() for executorpool #threads calculation

Use the new Couchbase::get_available_cpu_count() when calculating how
many executor pool threads to create. get_available_cpu_count()
accounts for the number of logical cores a process is allowed to run
on, which is the mechanism cgroups (and hence Docker) uses to limit
the CPUs available to a container.

This fixes the problem of us creating "too many" executorpool threads
under Docker containers which have the --cpuset-cpus option set.

Change-Id: I3e3b91eecc51aea298ae9aceb9e8c6d3f16b7612
Reviewed-on: http://review.couchbase.org/67493
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoRemove default parameter from deleteItem 24/67624/6
Daniel Owen [Tue, 13 Sep 2016 10:25:39 +0000 (11:25 +0100)]
Remove default parameter from deleteItem

The EventuallyPersistentStore method deleteItem has a parameter
called tapBackfill, which is defaulted to false.  The parameter
is used to specify if an item deletion is from a TAP backfill
stream.

However the deleteItem method is never passed a tapBackfill
parameter and therefore the default of false is always used.

This patch removes the unrequired parameter from the deleteItem
definition and simpifies the body of the method.

Change-Id: Ic1aa9a41bd68411f9b29b61333f66b4c1ae35278
Reviewed-on: http://review.couchbase.org/67624
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Well-Formed: buildbot <build@couchbase.com>

4 years agoMB-21036: Fix intermittent failure in shutdown_snapshot_range 48/67948/6
Dave Rigby [Fri, 23 Sep 2016 16:52:52 +0000 (17:52 +0100)]
MB-21036: Fix intermittent failure in shutdown_snapshot_range

Issue is that the test attempts to create exactly 3 checkpoints,
however this is load-dependent (i.e. how quickly the flusher runs and
creates checkpoints).

Remove this intermediate checks in the test, and just check the
sequence numbers.

Change-Id: Ic7c0a9217afcdc8bd65680efb992b09db0f5023b
Reviewed-on: http://review.couchbase.org/67948
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
4 years agoMB-21034: Fix race in test_expiry_pager_settings (gmtime_r) 46/67946/5
Dave Rigby [Fri, 23 Sep 2016 13:59:34 +0000 (14:59 +0100)]
MB-21034: Fix race in test_expiry_pager_settings (gmtime_r)

When calculating expiry pager adjusted time values, use the
thread-safe variant (gmtime_r) so the test doesn't conflict with the
ep_engine code.

Also use std::chrono to perform the time manipulaton (which handles
any modulus of minutes -> hours etc).

Change-Id: Iee39a86b73a71642b9dab4ff2821d589699731ac
Reviewed-on: http://review.couchbase.org/67946
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21029: Don't return ep_access_scanner_task_time until it has been scheduled 43/67943/7
Dave Rigby [Fri, 23 Sep 2016 10:23:11 +0000 (11:23 +0100)]
MB-21029: Don't return ep_access_scanner_task_time until it has been scheduled

Fix a latent race condition in ep_access_scanner_task_time stat
(exposed by intermittent failure in access_scanner_settings test),
whereby we can return a value for ep_access_scanner_task_time of the
Unix epoch (1970-01-01 00:00:00) if the AccessScanner has been enabled
but not yet scheduled.

Solution is to not return a value (and instead 'NOT SCHEDULED') until
scheduling has occurred.

Change-Id: Id85a602b3424c30c8795884207a1b0d31cb3c75a
Reviewed-on: http://review.couchbase.org/67943
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-21029: ep_test_apis: make wait_for_stat_XXX functions templated 42/67942/6
Dave Rigby [Fri, 23 Sep 2016 10:12:36 +0000 (11:12 +0100)]
MB-21029: ep_test_apis: make wait_for_stat_XXX functions templated

There is a range of helper functions to wait for a stat to meet some
criteria (equal, not equal, greater, ...), and currently these are
duplicated for each type (int, string, ...) required.

As the subsequent fix for MB-21029 needs a wait_for_stat_to_change for
the std::string type, genericize the current wait_for_stat function so
it can be used with any type.

Change-Id: I681218d31c4dcd1ef8b34de225efd13e99bbf8db
Reviewed-on: http://review.couchbase.org/67942
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852: ep_test_apis: report final value in wait_for_stat... 87/67887/8
Dave Rigby [Wed, 21 Sep 2016 12:32:14 +0000 (13:32 +0100)]
MB-20852: ep_test_apis: report final value in wait_for_stat...

Change-Id: I5aeae80ffcc0791701cf5982df4b11d00b288c8a
Reviewed-on: http://review.couchbase.org/67887
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20852: Remove unused 'force' parameter from scheduleVBStatePersist() 76/67876/6
Dave Rigby [Tue, 13 Sep 2016 13:06:04 +0000 (14:06 +0100)]
MB-20852: Remove unused 'force' parameter from scheduleVBStatePersist()

'force' is always false, so remove it.

Change-Id: I02e245b2a0237697c9abc4d55e2a9117cf4ca214
Reviewed-on: http://review.couchbase.org/67876
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20697: ep_testsuite: restore deleted DB dir before cleanup 81/67681/4
Dave Rigby [Wed, 14 Sep 2016 17:52:49 +0000 (18:52 +0100)]
MB-20697: ep_testsuite: restore deleted DB dir before cleanup

In some environments (seen on Windows VM), the regression test for
MB-20697 can hang forever during cleanup due to the writer thread being
stuck in an infinite loop trying to flush to disk.

To address this, re-create the database directory before shutting down
EPStore (after the test is complete).

Change-Id: I474e77bafbe5b30858d9a306669c52713890f846
Reviewed-on: http://review.couchbase.org/67681
Reviewed-by: Daniel Owen <owend@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMerge remote-tracking branch 'couchbase/4.5.1' into watson 00/68000/1 v4.5.1-MP1
Dave Rigby [Mon, 26 Sep 2016 13:54:02 +0000 (14:54 +0100)]
Merge remote-tracking branch 'couchbase/4.5.1' into watson

* couchbase/4.5.1:
  MB-20943: Set state to dead when deleting vbucket

Change-Id: I79ef4c2d0e66cf18e08f884ad7fecf2afe3b5578

4 years agoMB-20943: Set state to dead when deleting vbucket 73/67873/8 4.5.1 v4.5.1
Daniel Owen [Wed, 21 Sep 2016 09:50:33 +0000 (10:50 +0100)]
MB-20943: Set state to dead when deleting vbucket

When executing the VBucketMemoryDeletionTask the vbucket state is
unchanged.  notifyAllPendingConnsFailed is called in the run
function of VBucketMemoryDeletionTask.  This inturn calls fireAllOps,
which ensures all pending ops are cleared if the vbucket is in an
active state.

However if the vbucket is in a pending state is does nothing and
therefore the pending operations remain.  This results in connections
not being closed down.

The solution provided is to set the vbucket state to dead in
deleteVBucket, prior to calling scheduleVBDeletion.

A corresponding test has been added, which without the fix will hang.

Change-Id: I09cd4597b26576dd4b99d26f3a60c031e1b5f41d
Reviewed-on: http://review.couchbase.org/67873
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-20771: Remove unnecessary defragmenter_test operator overloads 75/67675/3
Dave Rigby [Wed, 14 Sep 2016 14:49:42 +0000 (15:49 +0100)]
MB-20771: Remove unnecessary defragmenter_test operator overloads

The defragmenter & ep_unit tests have their own operator new/delete
overload, to improve interopability with Valgrind (so Valgrind saw
explicit calls our malloc / free).

However the global_new_replacement in platform does this anyway now, so
this code is no longer needed.

Change-Id: I49dbc9e14582d39b58b1db48a0c7772d4f3855c9
Reviewed-on: http://review.couchbase.org/67675
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
4 years agoMB-20586: Update ep-engine to use cb_malloc memory API 13/67313/8
Dave Rigby [Fri, 26 Aug 2016 12:37:57 +0000 (13:37 +0100)]
MB-20586: Update ep-engine to use cb_malloc memory API

Similar to changes in memcached, update all C-style memory allocation
uses to use cb_malloc instead of raw system malloc.

Change-Id: Ic9bce029c34f74e161aed20b99129985264e0d4c
Reviewed-on: http://review.couchbase.org/67313
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Daniel Owen <owend@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20586: ep_unit_tests: Use real memory tracking code 42/67442/2
Dave Rigby [Fri, 12 Aug 2016 14:08:39 +0000 (15:08 +0100)]
MB-20586: ep_unit_tests: Use real memory tracking code

Use the 'real' memory tracking hooks instead of alloc_hooks_dummy in
the ep-engine unit tests. This more accurately reflects how our code
used in the 'real world'

Change-Id: I231a179e7765d46a63c72686c0279983db21cf0b
Reviewed-on: http://review.couchbase.org/67442
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-20586: DefragmenterTest.MappedMemory: Fix on OS X 12/67312/6
Dave Rigby [Fri, 26 Aug 2016 12:39:59 +0000 (13:39 +0100)]
MB-20586: DefragmenterTest.MappedMemory: Fix on OS X

This test currently fails as it does not have a way of tracking memory
usage, due to us not initializing the memory hooks (which installs a
custom zone allocator).

Fix by initializing the allocator before we run any tests in this
suite.

Change-Id: I3b567001b070483d19d16ff1787d29ebd9669bfa
Reviewed-on: http://review.couchbase.org/67312
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-20586: Configuration: Reduce use of C-style memory allocation 11/67311/6
Dave Rigby [Wed, 17 Aug 2016 14:00:20 +0000 (15:00 +0100)]
MB-20586: Configuration: Reduce use of C-style memory allocation

Remove C-style allocation where possible from Confiuration, replacing
with C++ owning containers.

Change-Id: I009a07064fb5c2727ab4d62f85aa9efd7f79684a
Reviewed-on: http://review.couchbase.org/67311
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-20586: MemoryTracker: Use new[] instead of calloc() 10/67310/7
Dave Rigby [Thu, 18 Aug 2016 16:04:02 +0000 (17:04 +0100)]
MB-20586: MemoryTracker: Use new[] instead of calloc()

Change-Id: I44882ee3d385c3ab70787fd187105fcdeb26545e
Reviewed-on: http://review.couchbase.org/67310
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-20586: Use cJSON_Free() to free allocations made by cJSON_Print 69/67069/6
Dave Rigby [Thu, 18 Aug 2016 16:11:29 +0000 (17:11 +0100)]
MB-20586: Use cJSON_Free() to free allocations made by cJSON_Print

Otherwise we can get mismatched malloc/free when using cbmalloc or
another custom allocator.

Change-Id: Ifdd2fe031cb6d6c785a77d552b966fa173de1593
Reviewed-on: http://review.couchbase.org/67069
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMB-20716: Ensure DCP consumers in EWOULDBLOCK are unpaused on bucket delete 75/67375/6
Dave Rigby [Tue, 6 Sep 2016 08:04:16 +0000 (09:04 +0100)]
MB-20716: Ensure DCP consumers in EWOULDBLOCK are unpaused on bucket delete

This is a follow-up to 8734958 - in addition to ensuring that DCP
producers are unpaused, also unpause DCP _consumers_.

Change-Id: I538e7bca865c4fa41240263da1c92312b3866bfa
Reviewed-on: http://review.couchbase.org/67375
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20751: Fix lock cycle (deadlock) during bucket delete & client disconnect 52/67252/4
Dave Rigby [Thu, 1 Sep 2016 15:30:41 +0000 (16:30 +0100)]
MB-20751: Fix lock cycle (deadlock) during bucket delete & client disconnect

MB-20716 recently fixed an issue where idle DCP connections (in
EWOULDBLOCK state) would not be notified (woken up) in the frontend
when a bucket was deleted. The fix for this was to trigger a notify
(via producer->notifyPaused()) as part of ep-engine bucket delete.

Unfortunately this introduced a lock-order-inversion (deadlock)
between two mutxes, which caused memcached to hang during shutdown,
as one (or more) worker threads would never terminate.

The issue is between:

1. Frontend_worker thread mutex (threadLock)
2. ConnMap::connsLock

And at least two threads (although normally 3 in the wild):

T1: Frontend worker thread
T2: DestroyBucket thread
(optional T3: A NONIO thread running ConnManager)

During bucket delete, the follow sequence occurs which creates a cycle
between threadLock and connsLock:

T1<Worker>:
    event_handler() ... conn_pending_close()
      -> LOCK(threadLock)
    DcpConnMap::disconnect()
      -> ACQUIRE(connsLock)

T2<DeleteBucket>:
    EventuallyPersistentEngine::handleDeleteBucket() ...
    DcpConnMap::shutdownAllConnections()
      -> LOCK(connsLock)
    notifyIOComplete() ... DcpProducer::notifyPaused()
      -> ACQUIRE(threadLock)

Part of the issue here is that DcpProducer::notifyPaused() *must* be
called with schedule==false, as there is no longer a ConnNotifier task
running on another thread (which never acquires the connsLock and
hence avoids any deadlock), as the ConnNotifier has been shutdown in
DcpConnMap::shutdownAllConnections previously. Therefore we need to
use the variant of notifyPaused which calls notify_IO_complete in the
same thread.

The solution chosen is to essentially drop the connsLock in
shutdownAllConnections before calling notify. We achive this by taking
a _copy_ of the connections map (under connsLock), and then iterating
over this copy and calling notify etc. This is safe as the elements of
the map are all ref-counted pointers.

Change-Id: I73f9b7576e42030a9f5219ae51e604e36fabcac7
Reviewed-on: http://review.couchbase.org/67252
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
4 years agoMB-20697: Return false if commit2couchstore fails 93/67093/3
Dave Rigby [Fri, 26 Aug 2016 18:29:43 +0000 (19:29 +0100)]
MB-20697: Return false if commit2couchstore fails

This ensures that callers are notified of the failure, and
specifically we correctly increment the ep_item_commit_failed stat

Change-Id: I56f2591479c45c03fba184236aa3790a67290b38
Reviewed-on: http://review.couchbase.org/67093
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Well-Formed: buildbot <build@couchbase.com>

4 years agoMB-20716: Ensure DCP connections in EWOULDBLOCK are unpaused on bucket delete 69/67169/4
Dave Rigby [Tue, 30 Aug 2016 14:10:32 +0000 (15:10 +0100)]
MB-20716: Ensure DCP connections in EWOULDBLOCK are unpaused on bucket delete

When a bucket delete occurs, memcached notifies the deleted engine via
the ON_DELETE_BUCKET callback, which in turn calls
DCPConnmap::shutdownAllConnections(). This correctly shuts down all
the DCP streams associated with DCP connections, however if any of
these DCP connections are in the EWOULDBLOCK state - i.e. the frontend
is waiting for a notify_IO_complete call to "wake" them up, then the
frontend will be blocked waiting for a notify_IO_complete which will
never arrive.

This behaviour is essentially a latent bug, however prior to the fix
for MB-20549, memcached would (incorrectly) call signalIfIdle on
connections in the EWOULDBLOCK state, forcing them to wake up. With
that fix in place this longer occurs.

The solution here is to explictly unpause all producer connections
when all streams are closed.

Change-Id: Ia105e78304f5481bb56a0c0ff1cfc973959e1016
Reviewed-on: http://review.couchbase.org/67169
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20645: Don't request stats from null DCP backfill manager 25/67025/3
Dave Rigby [Wed, 24 Aug 2016 10:54:13 +0000 (11:54 +0100)]
MB-20645: Don't request stats from null DCP backfill manager

If a DCP Producer has DcpProducer::addStats called on it after its
been disconnected (but before it's removed from the connMap) then we
end up dereferencing a null backfillMgr pointer.

Fix by adding a guard that the manager is valid before including its
stats.

Change-Id: Idc97b447090f5390054a9c40f207dae5494e63b9
Reviewed-on: http://review.couchbase.org/67025
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
4 years agoMB-20425: Change options parameter to correct values 37/66537/9
Daniel Owen [Fri, 5 Aug 2016 11:50:09 +0000 (12:50 +0100)]
MB-20425: Change options parameter to correct values

Updates epstore get to use the options passed in.
Requires the call to ep_engine get from ep_engine
arithmetic to be updated to use the following
options:
QUEUE_BG_FETCH | HONOR_STATES |
TRACK_REFERENCE | HIDE_LOCKED_CAS

Requires the call to ep_engine get from epstore
store to be updated to use the following options:
QUEUE_BG_FETCH | HONOR_STATES | TRACK_REFERENCE |
DELETE_TEMP | HIDE_LOCKED_CAS

Also adds an associated test, where the bloom filter
is disabled which in the presense of the bug will
cause the test to hang.

Change-Id: I8fd275c3e14b0050e172b32f15fb7ed555e4b0c2
Reviewed-on: http://review.couchbase.org/66537
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
4 years agoMB-20425: Remove default options parameter from get functions 31/66531/4
Daniel Owen [Fri, 5 Aug 2016 10:20:30 +0000 (11:20 +0100)]
MB-20425: Remove default options parameter from get functions

The ep_engine get function defaults the option parameter.
The ep_store get function also defaults the option parameter.

These multiple levels of defaulting has made it difficult to
track the value of the options parameter for different calls.
Therefore the use of defaults are removed for these cases.

This will make the change that addresses the regression of
MB-20425 much easier to understand.  This patch makes no
functional change.

Change-Id: I69aaa31a9a437f13299eb019956aa0488f13b95a
Reviewed-on: http://review.couchbase.org/66531
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
4 years agoMerge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson' 25/66325/11
Jim Walker [Wed, 3 Aug 2016 13:54:49 +0000 (14:54 +0100)]
Merge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson'

* commit '1301ca609be559248af78d6fa52ce766dd8e4915':
  MB-20307: Re-enable dcp ep_dcp_dead_conn_count
  MB-20312: Initialise snapshot task priority
  MB-20330: ReaderLockHolder with no lvalue

Change-Id: I5878d95f8d792971fbb4ab5342baf4b017b6614a

4 years agoMerge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson' 17/66317/12
Jim Walker [Wed, 3 Aug 2016 13:53:54 +0000 (14:53 +0100)]
Merge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson'

* commit '62f88138da834e216b953d3cf8064accb521c205':
  MB-19837: Increase number of NONIO threads
  MB-18453: Make task scheduling fairer
  [BP] MB-18452: Single threaded test harness improvements

Change-Id: If16ed42aed060f94d3180e832aaae0a7f5c5f052

4 years agoMerge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson' 16/66316/12
Jim Walker [Tue, 2 Aug 2016 09:08:25 +0000 (10:08 +0100)]
Merge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson'

* commit '36d772883b9bf2179694f2ca9d0575ed52135a66':
  MB-20182: Update checkpoint snapshot correctly during TAP backfill
  MB-20105: Ensure purge_seq is not reset when no items are purged in a compaction
  MB-20054: Fix windows build error by adding size() func in class AtomicQueue
  MB-20054: Fix windows build error by including a missing header file
  MB-20054: Regression test - bucket is deleted with DCPBackfill running
  MB-20054: Account for memory alloc/dealloc in unregisterBucket
  MB-20054: [BP] Add verbose (logging) output to ep_unit_tests_main
  MB-20054: Backport ep-engine_unit_tests from watson to 3.0.x

Change-Id: I4e82985e4ed7c506faa44b19b456b98d1067ed6a

4 years agoMerge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson' 98/66298/5
Jim Walker [Tue, 2 Aug 2016 09:07:21 +0000 (10:07 +0100)]
Merge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson'

* commit '6403bc0e8bbd7e94bb03672f505d99ff68d56c36':
  MB-18453: Give all tasks their own stats and priority
  [BP] MB-18580: Wait for VB state to be persisted before starting tests

Change-Id: Ia573467344f4c9ee2e2092322e54f7788201310e

4 years agoMerge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson' 96/66296/6
Jim Walker [Tue, 2 Aug 2016 09:05:49 +0000 (10:05 +0100)]
Merge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson'

* commit 'c509624bda146dfdc26ebaf8044657ecc1160912':
  MB-19948: enable disabled meta-data tests.
  MB-19948: Handle 18 bytes of metadata
  MB-19948: CouchKVStore metadata tests
  MB-19897: Fix the data race on lastSendTime between stats and dcp worker threads

Change-Id: Ia8b67427b969eefc50310762b92e9aa6a4662003

4 years agoMB-20351: Fix lock-order inversion in ~CheckpointManager 57/66357/3
Dave Rigby [Mon, 1 Aug 2016 13:24:39 +0000 (14:24 +0100)]
MB-20351: Fix lock-order inversion in ~CheckpointManager

As identified by TSan. Seen whilst testing sherlock->watson merge,
analysed the code and it seems this is a latent issue and hard to
re-produce.

The issue is that when the executor thread does a reset on the current
task, the VBCBAdapator is the last one holding the ref-counted
vbucket, so destruction occurs and ~VBucket calls the destructor of
the checkpoint manager, which is the reverse locks ordering of a
previous code path.

Number of threads in play here, but main ones of interest:

WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=170834)
  Cycle in lock order graph: M_checkpoint (0x7d640002e9a8) => M_exepool (0x7d4c00008288) => M_taskqueue (0x7d4400008e80) => M_exethread (0x7d380000df60) => M_checkpoint

  Mutex M_exepool acquired here while holding mutex M_checkpoint in thread T35:
    #0 pthread_mutex_lock <null> (engine_testapp+0x000000486760)
    #1 std::mutex::lock() /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/x86_64-linux-gnu/c++/4.9/bits/gthr-default.h:748 (ep.so+0x0000000f47b0)
    #2 ExecutorPool::wake(unsigned long) ep-engine/src/executorpool.cc:355 (ep.so+0x0000000f48f1)
    #3 Flusher::wake() ep-engine/src/flusher.cc:155 (ep.so+0x000000101ee6)
    #4 NotifyFlusherCB::callback(unsigned short&) ep-engine/src/flusher.h:88 (ep.so+0x00000010d194)
    #5 Checkpoint::queueDirty(SingleThreadedRCPtr<Item> const&, CheckpointManager*) ep-engine/src/checkpoint.h:675 (ep.so+0x0000000271b0)
    #6 CheckpointManager::closeOpenCheckpoint_UNLOCKED() ep-engine/src/checkpoint.cc:454 (ep.so+0x000000028dcb)
    #7 CheckpointManager::addNewCheckpoint_UNLOCKED(unsigned long, unsigned long, unsigned long) ep-engine/src/checkpoint.cc:371 (ep.so+0x00000002881f)
    #8 CheckpointManager::checkOpenCheckpoint_UNLOCKED(bool, bool) ep-engine/src/checkpoint.cc:361 (ep.so+0x00000002bd71)
    #9 CheckpointVisitor::visitBucket(RCPtr<VBucket>&) ep-engine/src/checkpoint_remover.cc:43 (ep.so+0x00000003c3bd)
    #10 VBCBAdaptor::run() ep-engine/src/ep.cc:3924 (ep.so+0x0000000a6174)
    #11 ExecutorThread::run() ep-engine/src/executorthread.cc:115 (ep.so+0x0000000fe1b6)
    #12 launch_executor_thread(void*) ep-engine/src/executorthread.cc:33 (ep.so+0x0000000fdd15)
    #13 platform_thread_wrap(void*) platform/src/cb_pthreads.cc:54 (libplatform.so.0.1.0+0x0000000057fb)

...

  Mutex M_checkpoint acquired here while holding mutex M_exethread in thread T36:
    #0 pthread_mutex_lock <null> (engine_testapp+0x000000486760)
    #1 CheckpointManager::~CheckpointManager() /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/x86_64-linux-gnu/c++/4.9/bits/gthr-default.h:748 (ep.so+0x000000027fdd)
    #2 VBucket::~VBucket() ep-engine/src/vbucket.cc:152 (ep.so+0x00000014a018)
    #3 PagingVisitor::~PagingVisitor() ep-engine/src/atomic.h:190 (ep.so+0x00000010a5e6)
    #4 PagingVisitor::~PagingVisitor() ep-engine/src/item_pager.cc:43 (ep.so+0x00000010a645)
    #5 std::_Sp_counted_ptr<PagingVisitor*, (__gnu_cxx::_Lock_policy)2>::_M_dispose() /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/shared_ptr_base.h:373 (ep.so+0x00000010a2b0)
    #6 VBCBAdaptor::~VBCBAdaptor() /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/shared_ptr_base.h:149 (ep.so+0x0000000aea7e)
    #7 ExecutorThread::run() ep-engine/src/atomic.h:325 (ep.so+0x0000000fdee4)
    #8 launch_executor_thread(void*) ep-engine/src/executorthread.cc:33 (ep.so+0x0000000fdd15)
    #9 platform_thread_wrap(void*) platform/src/cb_pthreads.cc:54 (libplatform.so.0.1.0+0x0000000057fb)

Change-Id: I0a966b3d112963243e17647184123fd8b3200656
Reviewed-on: http://review.couchbase.org/66357
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>
4 years agoMerge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson' 94/66294/1
Jim Walker [Thu, 28 Jul 2016 18:29:17 +0000 (19:29 +0100)]
Merge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson'

* commit 'dd3b6ae5e919bf51adaf5183fc8f1a076eac5357':
  MB-19982: Don't hold connsLock for duration of dcp stats
  MB-19982: Fix potential deadlock between DcpConsumer::bufMutex & connsLock
  MB-14859: Handle quick successive BG Fetch of a key interleaved with exp pager

Change-Id: Ie620baa1dc2151124f072084868020d3067c5fb2

4 years agoMB-20307: Re-enable dcp ep_dcp_dead_conn_count 84/66284/4 sherlock v4.1.2 v4.1.2-MP1 v4.1.2-MP2
Jim Walker [Thu, 28 Jul 2016 10:38:45 +0000 (11:38 +0100)]
MB-20307: Re-enable dcp ep_dcp_dead_conn_count

The call to collect this stat was dropped in
a recent merge commit. This commit adds it back.

Change-Id: I06d1d18cb4479edb2a74d899d4c3a8089a0c4656
Reviewed-on: http://review.couchbase.org/66284
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
Well-Formed: buildbot <build@couchbase.com>

4 years agoMB-20312: Initialise snapshot task priority 83/66283/3
Jim Walker [Thu, 28 Jul 2016 10:36:53 +0000 (11:36 +0100)]
MB-20312: Initialise snapshot task priority

The internal priority of VBSnapshotTask is not
intitialised, it is likely tasks requested to run at
low may actually become high (or vice versa).

Note this is not the GlobalTask priority, just an internal
one to this particular task.

Change-Id: Iabf91a8fe6fee0a8cf8bce99e72e4b22dd57040b
Reviewed-on: http://review.couchbase.org/66283
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
Well-Formed: buildbot <build@couchbase.com>

4 years agoMB-20330: ReaderLockHolder with no lvalue 82/66282/2
Jim Walker [Thu, 28 Jul 2016 10:32:46 +0000 (11:32 +0100)]
MB-20330: ReaderLockHolder with no lvalue

3.x merge brought in the wrong version of some
code meaning that a read lock is never acquired.

Change-Id: I139ac041d54fdf8d459f4309a9c2be22e40afb8e
Reviewed-on: http://review.couchbase.org/66282
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
Well-Formed: buildbot <build@couchbase.com>

4 years agoMerge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson' 64/66064/4
Jim Walker [Wed, 27 Jul 2016 09:20:31 +0000 (10:20 +0100)]
Merge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson'

* commit '9b194271f12e9b620c803a11b77a62e5402fb346': (22 commits)
  MB-19886: Fix data race on ActiveStream::curChkSeqno by making it atomic
  MB-19886: In markDiskSnapshot() get current vb snapshot info outside streamMutex
  MB-19843: Modify the end_seqno in DCP stream request after checking for rollback
  MB-19732: Fix the data race on lastSendTime between stats and dcp worker threads
  MB-19732: Record time for all DCP consumer messages
  MB-19732: Only update sendTime if successfully send noop
  MB-19691: Address data race on vb_state::high_seqno
  MB-19678: Merge backfill and in-memory snapshots correctly on replica vb
  MB-19636: Initialise failovers correctly from 2.5.x vbstate
  MB-19673: Log the actual last seqno sent before closing the stream.
  MB-19503: Fix ConnMap so notifications don't go missing [2]
  MB-19503: Fix ConnMap so notifications don't go missing.
  MB-19404: [BP] Address data race in DCP-Producer seen while making a stats request
  MB-19405: [BP] Address possible data races in PassiveStream context
  MB-19359: [3] Address lock inversion with vb's state lock and snapshot lock
  MB-19383: [BP] Address possible data race with startuptime
  MB-19380: Address data race observed with vb's pendingBGFetches
  MB-19360: Init mock server in stream module tests
  MB-19382: [BP] Create a variable to get correct locking scope
  MB-19359: [2] Address lock inversion with vb's state lock and snapshot lock
  ...

Change-Id: I70a7a29f33c7ec276e3bc99bc80a9e6fd739281a

4 years agoMerge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson' 60/66060/5
Jim Walker [Wed, 27 Jul 2016 09:19:50 +0000 (10:19 +0100)]
Merge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson'

* commit 'ac78070d8dae90427c4bd3030a7be4ab09f920ae':
  [BP] MB-16366: Obtain vbstate readlock in numerous operations
  MB-19280: Fix data race in CouchKVStore stats access
  MB-19279: Fix race in use of gmtime()
  MB-19113: Suppress test_mb16357 when on thread sanitizer
  MB-19278: Fix lock-order inversion on ActiveStream::streamMutex
  MB-19277: Set executorThread's waketime to atomic
  MB-19276: Fix data race on ExecutorThread::taskStart
  MB-19275: Address data race on a DCP stream's state
  MB-19273: Fix data race on PassiveStream::buffer.{bytes,items}
  MB-19260: Make cookie atomic to serialize set/get in ConnHandler
  MB-19259: Fix data race on DcpConsumer::backoffs
  MB-19258: Address data race with replicationThrottle parameters
  MB-19281: [BP] Add template class RelaxedAtomic<>
  MB-19257: Fix data race on ExecutorThread::now
  MB-19256: Address possible data race on VBCBAdaptor::currentvb

Change-Id: Ie8194d570b1d367a90d277ed086dec90eb99d6e9

4 years agoMerge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson' 59/66059/5
Jim Walker [Wed, 27 Jul 2016 09:19:09 +0000 (10:19 +0100)]
Merge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson'

* commit '644c09df35ac4e05006347140240819704848d0f':
  MB-19253: Fix race in void ExecutorPool::doWorkerStat
  MB-19252: Fix data race on Stream::readyQueueMemory
  MB-19251: Fix race in updating Vbucket.file{SpaceUsed,Size}
  MB-19249: Address possible data races in ConnHandler context
  MB-19248: Fix race in TaskQueue.{ready,future,pending}Queue access
  MB-19247: Fix possible data race in workload.h: workloadPattern
  MB-19246: Fix potentially incorrect persist_time in OBSERVE response
  MB-19229: Address possible data race in vbucket.cc: numHpChks
  MB-19228: Address possible data races in ActiveStream context
  MB-19227: Fix race in ConnNotifier.task access
  MB-19226: Address potential data races in the warmup code
  MB-19225: Fix data race on Flusher::taskId
  MB-19225: Fix race in Flusher._state
  MB-19224: Address possible data race with global task's waketime
  MB-19223: Switch to hrtime from timeval in Global Thread Pool

Change-Id: I6cab0405f2ce779e2cf9849fa5364a9549382905

4 years agoMerge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson' 54/66054/6
Jim Walker [Wed, 27 Jul 2016 08:03:35 +0000 (09:03 +0100)]
Merge remote-tracking branch 'couchbase/sherlock' into 'couchbase/watson'

* commit '8cbe913fa9a2f78388adb2d2ce6dbfeee1e23e6e':
  MB-19222: Fix race condition in TaskQueue shutdown
  MB-19220: Ensure HashTable::size is atomic
  MB-19204: ep_testsuite: Don't release the item while we're using it
  MB-19204: Address data race in ep_test_apis/testsuite
  MB-19204: ep_testsuite: Use std::string for last_key/body
  MB-19204: Remove alarm() call from atomic_ptr_test, reduce iteration count
  MB-19204: hash_table_test: Fix TSan issues
  MB-16656: Send snapshotEnd as highSeqno for replica vb in GET_ALL_VB_SEQNOS call
  MB-19153: Break circular dependency while deleting bucket
  MB-19113: Address false positive lock inversion seen with test_mb16357

Change-Id: I708f67379ab38ea1af8c1602b790e590c3038806

4 years agoMB-20224: [BP] Replace ThreadLocal '#define' with a using 10/66210/2
Will Gardner [Tue, 29 Mar 2016 12:26:35 +0000 (13:26 +0100)]
MB-20224: [BP] Replace ThreadLocal '#define' with a using

Using a define causes issues inside of GoogleTest which has its
own ThreadLocal class. By replacing it with 'using' we avoid
the collision.

*Required to build MB-20224 on watson.*

Change-Id: I05d3e25efc0eb361f7dbe82074d806ba116781c5
Reviewed-on: http://review.couchbase.org/66210
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
4 years agoMB-19837: Increase number of NONIO threads 34/65934/6
Jim Walker [Tue, 19 Jul 2016 14:18:48 +0000 (15:18 +0100)]
MB-19837: Increase number of NONIO threads

We've found that the NONIO threads can become
heavily utilised. On smaller systems only 1 or 2
threads were created, easily overwhelmed during
rebalance leading to rebalance failures.

This commit changes the code from creating
NONIO as 10% of nCPU to be 30% of nCPU and
ensuring at least 2 are always present.

The % is still hardwired because the thread count is global
and would be intialised by the first bucket's config.

Given that we can already override with a config flag the changes
are hardwired to give better throughput for nearly all users.

Comparison of old vs new.

CPU count = 1 NONIO threads old{1} new{2}
CPU count = 2 NONIO threads old{1} new{2}
CPU count = 4 NONIO threads old{1} new{2}
CPU count = 8 NONIO threads old{1} new{2}
CPU count = 16 NONIO threads old{2} new{3}
CPU count = 32 NONIO threads old{3} new{7}
CPU count = 36 NONIO threads old{3} new{8}
CPU count = 64 NONIO threads old{5} new{8}
CPU count = 128 NONIO threads old{8} new{8}

Change-Id: Ifa56730ad934ca9ae83993b3c539f4a725872696
Reviewed-on: http://review.couchbase.org/65934
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-18453: Make task scheduling fairer 29/65929/3
Jim Walker [Thu, 30 Jun 2016 10:23:20 +0000 (11:23 +0100)]
MB-18453: Make task scheduling fairer

The MB identified that we can starve tasks by scheduling
a higher priority task via ExecutorPool::wake().

This occurs because ExecutorPool::wake() pushes tasks
into the readyQueue enabling frequent wakes to trigger
the starvation bug.

The fix is to remove readyQueue.push from wake, so that we only
push to the readyQueue. The fetch side of scheduling only looks at
the futureQueue once the readyQueue is empty, thus the identified
starvation won't happen.

A unit-test demonstrates the fix using the single-threaded harness and
expects that two tasks of differing priorities get executed, rather
than the wake() starving the low-priority task.

This test drives:
 - ExecutorPool::schedule
 - ExecutorPool::reschedule
 - ExecutorPool::wake

These are all the methods which can add tasks into the scheduler
queue.

The fetch side is also covered:
 - ExecutorPool::fetchNextTask

This commit is an update to a previous commit that was reverted due
to performance issues. The original commit was reverted to minimise
disruption.

- original commit is e22c9ebeda1aac
- revert is 27cb1120e3e37

Change-Id: I70a4dcf7cd1c3a6f04548e9bbc3f95e24cdf50ad
Reviewed-on: http://review.couchbase.org/65929
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years ago[BP] MB-18452: Single threaded test harness improvements 28/65928/3
Jim Walker [Thu, 2 Jun 2016 15:05:50 +0000 (16:05 +0100)]
[BP] MB-18452: Single threaded test harness improvements

Refactor parts of the very new evp_store_single_threaded_test so that
it's simpler to drive tasks making new tests easier to write.

The main change is to provide helper methods for running any task from
a queue (with some checks) and a way to push a clean shutdown.

Change-Id: I7add574f0768c642f3c6c7c64293e882337a1cdc
Reviewed-on: http://review.couchbase.org/65928
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMB-20182: Update checkpoint snapshot correctly during TAP backfill 66/65866/4
Manu Dhundi [Tue, 19 Jul 2016 18:21:11 +0000 (11:21 -0700)]
MB-20182: Update checkpoint snapshot correctly during TAP backfill

When we do a TAP backfill we must update checkpoint snapshot start
and end correctly. Otherwise, if we immediately proceed to DCP
after TAP backfill, the checkpoint mgr will have incorrect combination
of {snap_start, snap_end, vb_high_seqno}

Change-Id: I2b738fd3b24486dadbd2962e81e0c3820c5a8786
Reviewed-on: http://review.couchbase.org/65866
Tested-by: buildbot <build@couchbase.com>
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Manu Dhundi <manu@couchbase.com>
4 years agoRevert "MB-18453: Make task scheduling fairer" 80/65780/2
Jim Walker [Thu, 14 Jul 2016 17:07:29 +0000 (10:07 -0700)]
Revert "MB-18453: Make task scheduling fairer"

When running in a >1 node cluster memcached CPU is running
very high. The original fix has introduced a problem which
needs further investigation (fetchTask is very very cpu hot).

This reverts commit e22c9ebeda1aac2fc8f4325cc39a93c3bcefffab.

Change-Id: If53a3a60692fbaaef4e54462f99284a8044cd899
Reviewed-on: http://review.couchbase.org/65780
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
4 years agoMerge remote-tracking branch 'couchbase/3.0.x' 57/65657/2
Manu Dhundi [Mon, 11 Jul 2016 22:10:48 +0000 (15:10 -0700)]
Merge remote-tracking branch 'couchbase/3.0.x'

*
|\
| * 6e10f8a 2016-07-11 | MB-20105: Ensure purge_seq is not reset when no items are purged in a compaction
| * 1fe3aac 2016-07-07 | MB-20054: Fix windows build error by adding size() func in class AtomicQueue
| * 536e32f 2016-07-07 | MB-20054: Fix windows build error by including a missing header file

Change-Id: Ib01ecb053ffa4b6fe2a6bac6cfbe6eccc3630549

4 years agoMB-20105: Ensure purge_seq is not reset when no items are purged in a compaction 26/65626/5 v3.1.6
Manu Dhundi [Mon, 11 Jul 2016 21:38:15 +0000 (14:38 -0700)]
MB-20105: Ensure purge_seq is not reset when no items are purged in a compaction

When a compaction request is made, we initially set the purge_seqno in the req
to 0, hoping to update it when we purge items. However, if there are no purged
items in a compaction call, then we end up reseting the purge_seqno
(correct one) set by the previous compaction call.

This commit addresses the problem by setting the purge seqno in the request
to current purge seqno in the ep-engine.

Change-Id: I9581abe7a4cb9d7cd84c1bf5563b98c91dc67525
Reviewed-on: http://review.couchbase.org/65626
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
4 years agoMerge remote-tracking branch 'couchbase/3.0.x' into sherlock 15/65615/3
Dave Rigby [Fri, 8 Jul 2016 13:44:29 +0000 (14:44 +0100)]
Merge remote-tracking branch 'couchbase/3.0.x' into sherlock

* couchbase/3.0.x:
  MB-20054: Regression test - bucket is deleted with DCPBackfill running
  MB-20054: Account for memory alloc/dealloc in unregisterBucket
  MB-20054: [BP] Add verbose (logging) output to ep_unit_tests_main

Change-Id: I5f05dd3355cc0d581350db65463c6c1dc155f3c6

4 years agoMerge remote-tracking branch 'couchbase/3.0.x' into sherlock 13/65613/3
Dave Rigby [Fri, 8 Jul 2016 09:49:51 +0000 (10:49 +0100)]
Merge remote-tracking branch 'couchbase/3.0.x' into sherlock

* couchbase/3.0.x:
  MB-20054: Backport ep-engine_unit_tests from watson to 3.0.x

Change-Id: I811e0b796be8611a4a574ab6b6a488ef50219bbf

4 years agodocs/Testing.md: Document the different test types 85/65585/4
Dave Rigby [Thu, 7 Jul 2016 15:03:05 +0000 (16:03 +0100)]
docs/Testing.md: Document the different test types

Change-Id: I3fcdb9e7cb347fa63f94e5c7a760bb54749aa375
Reviewed-on: http://review.couchbase.org/65585
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Dave Rigby <daver@couchbase.com>
4 years agoMB-20054: Fix windows build error by adding size() func in class AtomicQueue 96/65596/2
Manu Dhundi [Thu, 7 Jul 2016 21:28:52 +0000 (14:28 -0700)]
MB-20054: Fix windows build error by adding size() func in class AtomicQueue

Change-Id: I808e31c9a9ba97b67e75c07534350aa91cb040a2
Reviewed-on: http://review.couchbase.org/65596
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Sriram Ganesan <sriram@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
4 years agoMB-20054: Fix windows build error by including a missing header file 89/65589/2
Manu Dhundi [Thu, 7 Jul 2016 17:50:45 +0000 (10:50 -0700)]
MB-20054: Fix windows build error by including a missing header file

Change-Id: Ifdc8d6a09e8ff1bd68218066cffa44bc9de0a5a3
Reviewed-on: http://review.couchbase.org/65589
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Sriram Ganesan <sriram@couchbase.com>
4 years agoMB-20054: Regression test - bucket is deleted with DCPBackfill running 20/65520/14
Dave Rigby [Thu, 7 Jul 2016 08:23:25 +0000 (09:23 +0100)]
MB-20054: Regression test - bucket is deleted with DCPBackfill running

Regression test for MB-20054 - the following abort is encountered when
a DCPBackfill task is still running when a bucket is deleted:

    Assertion failed: (engine), function verifyEngine, file
    ep-engine/src/objectregistry.cc, line 58.

This issue occurs because the DCPBackfill object (and associated
objects ActiveStream and importantly ActiveStreams' readyQ of Items)
is not deleted earlier in the shutdown sequence (via EvpDestroy), as
we use ref-counted pointers for it and there is a still an outstanding
reference by the AuxIO Thread which is running the task. Hence the
DCPBackfill object is only deleted when we finally unregister the
deleted bucket from the shared ExecutorPool - see the following
backtrace:

    #1  0x00007f513b75a085 in abort () from /lib64/libc.so.6
    #2  0x00007f51337034e2 in ObjectRegistry::onDeleteItem (pItem=<value optimized out>) at ep-engine/src/objectregistry.cc:157
    #3  0x00007f5133652094 in ~Item (this=<value optimized out>) at ep-engine/src/item.h:352
    #4  SingleThreadedRCPtr<Item>::~SingleThreadedRCPtr (this=<value optimized out>) at ep-engine/src/atomic.h:430
    #5  0x00007f51336c7f47 in ~MutationResponse (this=0x3cd87880) at ep-engine/src/dcp-response.h:275
    #6  MutationResponse::~MutationResponse (this=0x3cd87880) at ep-engine/src/dcp-response.h:275
    #7  0x00007f51336d86aa in clear_UNLOCKED (this=0x7a3f5fa0) at ep-engine/src/dcp-stream.cc:201
    #8  ~ActiveStream (this=0x7a3f5fa0) at ep-engine/src/dcp-stream.h:178
    #9  ActiveStream::~ActiveStream (this=0x7a3f5fa0) at ep-engine/src/dcp-stream.h:179
    #10 0x00007f51336cc808 in RCPtr<Stream>::~RCPtr (this=0xb1823780) at ep-engine/src/atomic.h:348
    #11 0x00007f51336d77c7 in ~DCPBackfill (this=0xb1823740) at ep-engine/src/dcp-stream.cc:114
    #12 DCPBackfill::~DCPBackfill (this=0xb1823740) at ep-engine/src/dcp-stream.cc:114
    #13 0x00007f513368d95f in ~SingleThreadedRCPtr (this=0x5b55a20, e=0x59c4000, taskType=NO_TASK_TYPE) at ep-engine/src/atomic.h:430
    #14 ExecutorPool::_stopTaskGroup (this=0x5b55a20, e=0x59c4000, taskType=NO_TASK_TYPE) at ep-engine/src/executorpool.cc:532
    #15 0x00007f513368dad3 in ExecutorPool::_unregisterBucket (this=0x5b55a20, engine=0x59c4000) at ep-engine/src/executorpool.cc:551
    #16 0x00007f513368e143 in ExecutorPool::unregisterBucket (this=0x5b55a20, engine=0x59c4000) at ep-engine/src/executorpool.cc:602
    #17 0x00007f5133655f82 in EventuallyPersistentStore::~EventuallyPersistentStore (this=0x59e6000)
        at ep-engine/src/ep.cc:365
    #18 0x00007f5133672a25 in EventuallyPersistentEngine::~EventuallyPersistentEngine (this=0x59c4000)
        at ep-engine/src/ep_engine.cc:5791
    #19 0x00007f5133672c95 in EvpDestroy (handle=0x59c4000, force=<value optimized out>) at ep-engine/src/ep_engine.cc:143

To actually reproduce the issue is somewhat involved - we need to
orchestrate the world such that we delete the engine while a
DCPBackfill task is still running. We spin up a separate thread which
will run the DCPBackfill task concurrently with destroy - specifically
DCPBackfill must start running (and add items to the readyQ) before
destroy(), it must then continue running (stop after) _stopTaskGroup
is invoked.  To achieve this we use a couple of condition variables to
synchronise between the two threads - the timeline needs to look like:

    auxIO thread:  [------- DCPBackfill ----------]
     main thread:      [--destroy()--]       [ExecutorPool::_stopTaskGroup]

    --------------------------------------------------------> time

Change-Id: Ic64c419cb8e4e0af2378efba9711b121aacee15b
Reviewed-on: http://review.couchbase.org/65520
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>