Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] RedisModule_SignalKeyAsReady not working as expected #7878

Closed
manast opened this issue Oct 2, 2020 · 6 comments
Closed

[Bug] RedisModule_SignalKeyAsReady not working as expected #7878

manast opened this issue Oct 2, 2020 · 6 comments
Assignees

Comments

@manast
Copy link

manast commented Oct 2, 2020

Describe the bug
RedisModule_SignalKeyAsReady does not work as expected.

To reproduce

Here a minimal module that reproduces the issue:

#include "redismodule.h"
#include <stdlib.h>

int BlockedReplyCallback(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)  {
    RedisModule_Log(ctx, "warning", "Reply callback!");
    RedisModule_ReplyWithSimpleString(ctx, "We are done!");
    return REDISMODULE_OK;
}

int TimeoutReplyCallback(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)  {
    RedisModule_Log(ctx, "warning", "Timed out!");

    RedisModule_ReplyWithSimpleString(ctx, "Timed Out");
    return REDISMODULE_OK;
}


void BlockedFreePrivdata(RedisModuleCtx *ctx, void *privdata) {
    RedisModule_Log(ctx, "warning", "Freeing all priv data");
}

int HelloworldRand_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {

    RedisModuleBlockedClient* bc = RedisModule_BlockClientOnKeys(ctx, 
            BlockedReplyCallback, 
            TimeoutReplyCallback, 
            BlockedFreePrivdata,
            10000, 
            &argv[1],
            1, 
            NULL);

    RedisModule_SignalKeyAsReady(ctx, argv[1]);

    return REDISMODULE_OK;
}

int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
    if (RedisModule_Init(ctx,"helloworld",1,REDISMODULE_APIVER_1)
        == REDISMODULE_ERR) return REDISMODULE_ERR;

    if (RedisModule_CreateCommand(ctx,"bug1",
        HelloworldRand_RedisCommand, "",
        0, 0, 0) == REDISMODULE_ERR)
        return REDISMODULE_ERR;

    return REDISMODULE_OK;
}

Just load the module and run the command "bug1 123". It will be blocked until timing out after 10 seconds despite having called RedisModule_SignalKeyAsReady directly after blocking.
Note, I also tested to calling RedisModule_SignalKeyAsReady from a timer callback with the exact same result.

Expected behavior

The BlockedReplyCallback should be called after signaling that the key is ready.

Additional information

This is on a Mac running redis 6.0.8 and the latest redismodule.h (from yesterday).

@manast manast changed the title Possibly RedisModule_SignalKeyAsReady not working as expected [Bug] RedisModule_SignalKeyAsReady not working as expected Oct 3, 2020
@manast manast changed the title [Bug] RedisModule_SignalKeyAsReady not working as expected [Bug] `RedisModule_SignalKeyAsReady not working as expected Oct 3, 2020
@manast manast changed the title [Bug] `RedisModule_SignalKeyAsReady not working as expected [Bug] RedisModule_SignalKeyAsReady not working as expected Oct 3, 2020
@manast
Copy link
Author

manast commented Oct 3, 2020

I did some test with redis-cli and I noticed that if the key that you listen to exists before calling the command, then it the reply callback is called, so it seems that the bug happens when the key does not exist at the time of blocking.

@guybe7
Copy link
Collaborator

guybe7 commented Oct 12, 2020

hello @manast
what you described is actually not a bug, it's a misuse of the API: you're not supposed to "signal a key as ready" when the key doesn't exist (and thus, obviously not "ready")

@guybe7
Copy link
Collaborator

guybe7 commented Oct 12, 2020

may i know the reason you want to signal a non-existing key as ready? if it's a compelling enough we may consider to relax the definition of a "ready" key (i.e. now we assume a "ready" key must exist)

@manast
Copy link
Author

manast commented Oct 12, 2020

@guybe7 This was me trying to find all corner cases for SignalKeyAsReady, however the case that actually was a problem for me was this one (I have finally found a workaround using UnblockClient instead that works for my specific use case): #7880
so in other words, it makes sense that it does not work for non-existing keys but it would be probably good to have a one liner comment about this "limitation" even if it may be obvious for many.

@guybe7
Copy link
Collaborator

guybe7 commented Oct 12, 2020

@manast NP i will push a commit with updated docs/comments

@guybe7
Copy link
Collaborator

guybe7 commented Oct 12, 2020

closing

@guybe7 guybe7 closed this as completed Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants