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>

diff --git a/src/connmap.cc b/src/connmap.cc
index 8786e22..13de1bb 100644 (file)
--- a/src/connmap.cc
+++ b/src/connmap.cc
@@ -1132,6 +1132,10 @@ void DcpConnMap::closeAllStreams_UNLOCKED() {
         if (producer) {
             producer->closeAllStreams();
             producer->clearCheckpointProcessorTaskQueues();
+            // The producer may be in EWOULDBLOCK (if it's idle), therefore
+            // notify him to ensure the front-end connection can close the TCP
+            // connection.
+            producer->notifyPaused(/*schedule*/false);
         } else {
             static_cast<DcpConsumer*>(itr->second.get())->closeAllStreams();
         }

diff --git a/tests/module_tests/dcp_test.cc b/tests/module_tests/dcp_test.cc
index d45d396..37131db 100644 (file)
--- a/tests/module_tests/dcp_test.cc
+++ b/tests/module_tests/dcp_test.cc
@@ -613,6 +613,55 @@ TEST_F(ConnectionTest, test_mb20645_stats_after_closeAllStreams) {
     destroy_mock_cookie(cookie);
 }
 
+// Verify that when a DELETE_BUCKET event occurs, we correctly notify any
+// DCP connections which are currently in ewouldblock state, so the frontend
+// can correctly close the connection.
+// If we don't notify then front-end connections can hang for a long period of
+// time).
+TEST_F(ConnectionTest, test_mb20716_connmap_notify_on_delete) {
+    MockDcpConnMap connMap(*engine);
+    connMap.initialize(DCP_CONN_NOTIFIER);
+    const void *cookie = create_mock_cookie();
+    // Create a new Dcp producer.
+    dcp_producer_t producer = connMap.newProducer(cookie, "mb_20716r",
+                                                  /*notifyOnly*/false);
+
+    // Check preconditions.
+    EXPECT_TRUE(producer->isPaused());
+
+    // Hook into notify_io_complete.
+    // We (ab)use the engine_specific API to pass a pointer to a count of
+    // how many times notify_io_complete has been called.
+    size_t notify_count = 0;
+    SERVER_COOKIE_API* scapi = get_mock_server_api()->cookie;
+    scapi->store_engine_specific(cookie, &notify_count);
+    auto orig_notify_io_complete = scapi->notify_io_complete;
+    scapi->notify_io_complete = [](const void *cookie,
+                                   ENGINE_ERROR_CODE status) {
+        auto* notify_ptr = reinterpret_cast<size_t*>(
+                get_mock_server_api()->cookie->get_engine_specific(cookie));
+        (*notify_ptr)++;
+    };
+
+    // 0. Should start with no notifications.
+    ASSERT_EQ(0, notify_count);
+
+    // 1. Check that the periodic connNotifier (notifyAllPausedConnections)
+    // isn't sufficient to notify (it shouldn't be, as our connection has
+    // no notification pending).
+    connMap.notifyAllPausedConnections();
+    ASSERT_EQ(0, notify_count);
+
+    // 1. Simulate a bucket deletion.
+    connMap.shutdownAllConnections();
+
+    EXPECT_EQ(1, notify_count)
+        << "expected one notify after shutting down all connections";
+
+    // Restore notify_io_complete callback.
+    scapi->notify_io_complete = orig_notify_io_complete;
+    destroy_mock_cookie(cookie);
+}
 
 class NotifyTest : public DCPTest {
 protected: