Skip to content

Commit 9d9d266

Browse files
trondndaverigby
authored andcommitted
[Refactor] Add scheduleDcpStep to server cookie iface
This is a step on the way to decouple the tight binding between the cookie used in DCP open and the cookie used to hold the connection in the engine. Change-Id: Ib300a0dcd409d651511f602af35ee559fb79adc1 Reviewed-on: http://review.couchbase.org/c/kv_engine/+/141088 Tested-by: Build Bot <[email protected]> Reviewed-by: Dave Rigby <[email protected]>
1 parent 6c11d50 commit 9d9d266

File tree

9 files changed

+58
-6
lines changed

9 files changed

+58
-6
lines changed

auditd/tests/testauditd.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ class AuditMockServerCookieApi : public ServerCookieIface {
8989
ready = true;
9090
cond.notify_one();
9191
}
92+
void scheduleDcpStep(gsl::not_null<const void*> cookie) override {
93+
throw std::runtime_error("Not implemented");
94+
}
95+
9296
ENGINE_ERROR_CODE reserve(gsl::not_null<const void*> cookie) override {
9397
throw std::runtime_error("Not implemented");
9498
}

daemon/server_api.cc

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,33 @@ struct ServerCookieApi : public ServerCookieIface {
187187
::notify_io_complete(cookie, status);
188188
}
189189

190+
void scheduleDcpStep(gsl::not_null<const void*> cookie_) override {
191+
auto& cookie = getCookie(cookie_);
192+
auto& connection = cookie.getConnection();
193+
if (!connection.isDCP()) {
194+
LOG_WARNING(
195+
"scheduleDcpStep: Must only be called with a DCP "
196+
"connection: {}",
197+
connection.toJSON().dump());
198+
throw std::logic_error(
199+
"scheduleDcpStep(): Provided cookie is not bound to a "
200+
"connection set up for DCP");
201+
}
202+
203+
if (cookie.getRefcount() == 0) {
204+
LOG_WARNING(
205+
"scheduleDcpStep: DCP connection did not reserve the "
206+
"cookie: {}",
207+
cookie.getConnection().toJSON().dump());
208+
throw std::logic_error("scheduleDcpStep: cookie must be reserved!");
209+
}
210+
211+
// @todo refactor so that notify_io_complete can ensure that it is
212+
// only notified in a blocked state (and for this case we
213+
// notify if it isn't already notified)
214+
::notify_io_complete(&cookie, ENGINE_SUCCESS);
215+
}
216+
190217
ENGINE_ERROR_CODE reserve(gsl::not_null<const void*> void_cookie) override {
191218
getCookie(void_cookie).incrementRefcount();
192219
return ENGINE_SUCCESS;

engines/ep/src/connmap.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ void ConnMap::notifyPausedConnection(const std::shared_ptr<ConnHandler>& conn) {
131131
{
132132
LockHolder rlh(releaseLock);
133133
if (conn.get() && conn->isPaused() && conn->isReserved()) {
134-
engine.notifyIOComplete(conn->getCookie(), ENGINE_SUCCESS);
134+
engine.scheduleDcpStep(conn->getCookie());
135135
}
136136
}
137137
}

engines/ep/src/dcp/dcpconnmap.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ void DcpConnMap::manageConnections() {
402402

403403
for (auto& it : toNotify) {
404404
if (it.get() && it->isReserved()) {
405-
engine.notifyIOComplete(it->getCookie(), ENGINE_SUCCESS);
405+
engine.scheduleDcpStep(it->getCookie());
406406
}
407407
}
408408

engines/ep/src/ep_engine.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6059,6 +6059,12 @@ void EventuallyPersistentEngine::notifyIOComplete(const void* cookie,
60596059
}
60606060
}
60616061

6062+
void EventuallyPersistentEngine::scheduleDcpStep(
6063+
gsl::not_null<const void*> cookie) {
6064+
NonBucketAllocationGuard guard;
6065+
serverApi->cookie->scheduleDcpStep(cookie);
6066+
}
6067+
60626068
ENGINE_ERROR_CODE EventuallyPersistentEngine::getRandomKey(
60636069
const void* cookie,
60646070
const cb::mcbp::Request& request,

engines/ep/src/ep_engine.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,7 @@ class EventuallyPersistentEngine : public EngineIface, public DcpIface {
529529
void setDCPPriority(const void* cookie, ConnectionPriority priority);
530530

531531
void notifyIOComplete(const void* cookie, ENGINE_ERROR_CODE status);
532+
void scheduleDcpStep(gsl::not_null<const void*> cookie);
532533

533534
ENGINE_ERROR_CODE reserveCookie(const void *cookie);
534535
ENGINE_ERROR_CODE releaseCookie(const void *cookie);

engines/ep/tests/module_tests/dcp_test.cc

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ class WrappedServerCookieIface : public ServerCookieIface {
120120
ENGINE_ERROR_CODE status) override {
121121
return wrapped->notify_io_complete(cookie, status);
122122
}
123+
void scheduleDcpStep(gsl::not_null<const void*> cookie) override {
124+
wrapped->scheduleDcpStep(cookie);
125+
}
123126
ENGINE_ERROR_CODE reserve(gsl::not_null<const void*> cookie) override {
124127
return wrapped->reserve(cookie);
125128
}
@@ -1601,8 +1604,7 @@ TEST_P(ConnectionTest, test_mb20716_connmap_notify_on_delete) {
16011604
size_t notify_count = 0;
16021605
class MockServerCookieApi : public WrappedServerCookieIface {
16031606
public:
1604-
void notify_io_complete(gsl::not_null<const void*> cookie,
1605-
ENGINE_ERROR_CODE status) override {
1607+
void scheduleDcpStep(gsl::not_null<const void*> cookie) override {
16061608
auto* notify_ptr = reinterpret_cast<size_t*>(
16071609
wrapped->get_engine_specific(cookie));
16081610
(*notify_ptr)++;
@@ -1661,8 +1663,7 @@ TEST_P(ConnectionTest, test_mb20716_connmap_notify_on_delete_consumer) {
16611663

16621664
class MockServerCookieApi : public WrappedServerCookieIface {
16631665
public:
1664-
void notify_io_complete(gsl::not_null<const void*> cookie,
1665-
ENGINE_ERROR_CODE status) override {
1666+
void scheduleDcpStep(gsl::not_null<const void*> cookie) override {
16661667
auto* notify_ptr = reinterpret_cast<size_t*>(
16671668
get_mock_server_api()->cookie->get_engine_specific(cookie));
16681669
(*notify_ptr)++;

include/memcached/server_cookie_iface.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,15 @@ struct ServerCookieIface {
156156
virtual void notify_io_complete(gsl::not_null<const void*> cookie,
157157
ENGINE_ERROR_CODE status) = 0;
158158

159+
/**
160+
* Request the core to schedule a new call to dcp_step() as soon as
161+
* possible as the underlying engine has data to send.
162+
*
163+
* @param cookie cookie representing the connection (MUST be a DCP
164+
* connection)
165+
*/
166+
virtual void scheduleDcpStep(gsl::not_null<const void*> cookie) = 0;
167+
159168
/**
160169
* Notify the core that we're holding on to this cookie for
161170
* future use. (The core guarantees it will not invalidate the

programs/engine_testapp/mock_server.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,10 @@ struct MockServerCookieApi : public ServerCookieIface {
423423
c->num_io_notifications++;
424424
c->cond.notify_all();
425425
}
426+
427+
void scheduleDcpStep(gsl::not_null<const void*> cookie) override {
428+
notify_io_complete(cookie, ENGINE_SUCCESS);
429+
}
426430
};
427431

428432
ServerApi* get_mock_server_api() {

0 commit comments

Comments
 (0)