MB-17766: Incorrect ordering of messages during ActiveStream's takeover-send phase 27/59627/7
authorabhinavdangeti <abhinav@couchbase.com>
Tue, 9 Feb 2016 20:18:01 +0000 (12:18 -0800)
committerabhinav dangeti <abhinav@couchbase.com>
Wed, 10 Feb 2016 19:05:35 +0000 (19:05 +0000)
commitba305c4f083b3910d12f470bf9b39b3f4c2f2b97
tree65c20902ac43b3db5058cd416882884e59960444
parent4f39683a9692c1e160211761bf672a8641e89ed9
MB-17766: Incorrect ordering of messages during ActiveStream's takeover-send phase

A race between the step() and ActiveStreamCheckpointProcessorTask
can cause mutations to be queued into the readyQ after the
setVBucketState(active) message.

Here's the scenario in chronology (T: Front-end thread, BT: IO thread):
1. T1: ActiveStream::setVBucketStateAckRecieved()
2. T1: transitionState(takeoverSend) => schedules ActiveStreamCheckpointProcessorTask
3. BT1: manageConnections() notifies memcached about specific conn (max idle time: 5s)
4. BT2: ActiveStreamCheckpointProcessorTask runs, gets all Items For Cursor
5. T1: step() -> takeoverSendPhase() -> readyQ is empty
        => nextCheckpointItem() return false, as getNumItemsForCursor returns 0
        => setVbucketState(active) added to readyQ
6. BT2: ActiveStreamCheckpointProcessorTask continues, adds mutations acquired into readyQ
        -> Note the mutations were acquired in step 4
        => Notified memcached connections
7. T1: step() .. ships messages in incorrect order

On the new master, the vbucket is promoted to active state and then more
mutations are received from the old master. If there were front end ops
at this time, there could be an inconsistency in highSeqno or in worst
cases crashes in checkpoint manager due to highSeqno not belonging in
the designated range.

The fix: Add an atomic flag that is also checked for along with
getNumItemsForCursor in nextCheckpointItem(). This flag is set before
retrieving all items for a cursor (getAllItemsForCursor) and unset after
all the retrieved items have been added to the ready queue of the stream.

Change-Id: I5c04d47cc99c7dd3b2d87cb68dd30d36473226e5
Reviewed-on: http://review.couchbase.org/59627
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Chiyoung Seo <chiyoung@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
src/dcp-stream.cc
src/dcp-stream.h