Skip to content

Commit

Permalink
Avoid calling RM_BlockedClientMeasureTimeEnd in the main thread
Browse files Browse the repository at this point in the history
  • Loading branch information
@sundb
sundb committed Jan 11, 2024
1 parent 54524ca commit 17efc14
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 16 deletions.
50 changes: 39 additions & 11 deletions tests/modules/blockonbackground.c
View file
Expand Up @@ -19,8 +19,6 @@ int HelloBlock_Reply(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
int HelloBlock_Timeout(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
UNUSED(argv);
UNUSED(argc);
RedisModuleBlockedClient *bc = RedisModule_GetBlockedClientHandle(ctx);
RedisModule_BlockedClientMeasureTimeEnd(bc);
return RedisModule_ReplyWithSimpleString(ctx,"Request timedout");
}

Expand All @@ -42,19 +40,47 @@ void *BlockDebug_ThreadMain(void *arg) {
RedisModuleBlockedClient *bc = targ[0];
long long delay = (unsigned long)targ[1];
long long enable_time_track = (unsigned long)targ[2];
long long timeout = (unsigned long)targ[3];

/* Get Redis module context */
RedisModuleCtx *ctx = RedisModule_GetThreadSafeContext(bc);

if (enable_time_track)
RedisModule_BlockedClientMeasureTimeStart(bc);
RedisModule_Free(targ);

/* Divide the sleep into two phases, which allows us to immediately
* respond to the user if a timeout occurs during sleep. */
long long sleep_phase_1 = delay > timeout ? timeout : delay;
long long sleep_phase_2 = delay - sleep_phase_1;

struct timespec ts;
ts.tv_sec = delay / 1000;
ts.tv_nsec = (delay % 1000) * 1000000;
ts.tv_sec = sleep_phase_1 / 1000;
ts.tv_nsec = (sleep_phase_1 % 1000) * 1000000;
nanosleep(&ts, NULL);
int *r = RedisModule_Alloc(sizeof(int));
*r = rand();

if (sleep_phase_2 > 0 && timeout) {
/* If further sleep is required but a timeout has occurred,
* we track the end time and respond timeout to the user. */
if (enable_time_track)
RedisModule_BlockedClientMeasureTimeEnd(bc);
RedisModule_ReplyWithSimpleString(ctx,"Request timedout");
} else {
/* Continue the unfinished sleep. */
ts.tv_sec = sleep_phase_2 / 1000;
ts.tv_nsec = (sleep_phase_2 % 1000) * 1000000;
nanosleep(&ts, NULL);

if (enable_time_track)
RedisModule_BlockedClientMeasureTimeEnd(bc);
RedisModule_UnblockClient(bc,r);
RedisModule_ReplyWithLongLong(ctx, rand());
}

RedisModule_UnblockClient(bc, NULL);

/* Free the Redis module context */
RedisModule_FreeThreadSafeContext(ctx);

return NULL;
}

Expand Down Expand Up @@ -106,7 +132,7 @@ int HelloBlock_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int a
}

pthread_t tid;
RedisModuleBlockedClient *bc = RedisModule_BlockClient(ctx,HelloBlock_Reply,HelloBlock_Timeout,HelloBlock_FreeData,timeout);
RedisModuleBlockedClient *bc = RedisModule_BlockClient(ctx,NULL,NULL,HelloBlock_FreeData,0);

/* Here we set a disconnection handler, however since this module will
* block in sleep() in a thread, there is not much we can do in the
Expand All @@ -116,11 +142,12 @@ int HelloBlock_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int a
/* Now that we setup a blocking client, we need to pass the control
* to the thread. However we need to pass arguments to the thread:
* the delay and a reference to the blocked client handle. */
void **targ = RedisModule_Alloc(sizeof(void*)*3);
void **targ = RedisModule_Alloc(sizeof(void*)*4);
targ[0] = bc;
targ[1] = (void*)(unsigned long) delay;
// pass 1 as flag to enable time tracking
targ[2] = (void*)(unsigned long) 1;
targ[3] = (void*)(unsigned long) timeout;

if (pthread_create(&tid,NULL,BlockDebug_ThreadMain,targ) != 0) {
RedisModule_AbortBlock(bc);
Expand All @@ -147,7 +174,7 @@ int HelloBlockNoTracking_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **a
}

pthread_t tid;
RedisModuleBlockedClient *bc = RedisModule_BlockClient(ctx,HelloBlock_Reply,HelloBlock_Timeout,HelloBlock_FreeData,timeout);
RedisModuleBlockedClient *bc = RedisModule_BlockClient(ctx,NULL,NULL,HelloBlock_FreeData,0);

/* Here we set a disconnection handler, however since this module will
* block in sleep() in a thread, there is not much we can do in the
Expand All @@ -157,11 +184,12 @@ int HelloBlockNoTracking_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **a
/* Now that we setup a blocking client, we need to pass the control
* to the thread. However we need to pass arguments to the thread:
* the delay and a reference to the blocked client handle. */
void **targ = RedisModule_Alloc(sizeof(void*)*3);
void **targ = RedisModule_Alloc(sizeof(void*)*4);
targ[0] = bc;
targ[1] = (void*)(unsigned long) delay;
// pass 0 as flag to enable time tracking
targ[2] = (void*)(unsigned long) 0;
targ[3] = (void*)(unsigned long) timeout;

if (pthread_create(&tid,NULL,BlockDebug_ThreadMain,targ) != 0) {
RedisModule_AbortBlock(bc);
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/moduleapi/blockonbackground.tcl
View file
Expand Up @@ -15,14 +15,14 @@ start_server {tags {"modules"}} {
if {!$::no_latency} {
assert_equal [r slowlog len] 0
}
r block.debug 0 10000
assert_not_equal [r block.debug 0 10000] "Request timedout"
if {!$::no_latency} {
assert_equal [r slowlog len] 0
}
r config resetstat
r config set latency-tracking yes
r config set latency-tracking-info-percentiles "50.0"
r block.debug 200 10000
assert_not_equal [r block.debug 200 10000] "Request timedout"
if {!$::no_latency} {
assert_equal [r slowlog len] 1
}
Expand All @@ -43,12 +43,12 @@ start_server {tags {"modules"}} {
if {!$::no_latency} {
assert_equal [r slowlog len] 0
}
r block.debug 0 20000
assert_not_equal [r block.debug 0 20000] "Request timedout"
if {!$::no_latency} {
assert_equal [r slowlog len] 0
}
r config resetstat
r block.debug 20000 500
assert_equal [r block.debug 20000 500] "Request timedout"
if {!$::no_latency} {
assert_equal [r slowlog len] 1
}
Expand Down Expand Up @@ -88,7 +88,7 @@ start_server {tags {"modules"}} {
if {!$::no_latency} {
assert_equal [r slowlog len] 0
}
r block.debug_no_track 200 1000
assert_not_equal [r block.debug_no_track 200 1000] "Request timedout"
# ensure slowlog is still empty
if {!$::no_latency} {
assert_equal [r slowlog len] 0
Expand Down

0 comments on commit 17efc14

Please sign in to comment.