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.

Sign up for GitHub

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

Jump to bottom

[hashset feature] Convert SET datatype to use hashset instead of dict #1176

Open
wants to merge 2 commits into
base: hashset
Choose a base branch
from
+472 −504
Open

[hashset feature] Convert SET datatype to use hashset instead of dict #1176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8fe59b3
Replace dict with hashset in command tables (#1065)
SoftlyRaining Sep 27, 2024
add04d0
Convert SET from dict -> hashset (squashed)
SoftlyRaining Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 31 additions & 31 deletions src/acl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,14 +652,14 @@ void ACLChangeSelectorPerm(aclSelector *selector, struct serverCommand *cmd, int
unsigned long id = cmd->id;
ACLSetSelectorCommandBit(selector, id, allow);
ACLResetFirstArgsForCommand(selector, id);
if (cmd->subcommands_dict) {
dictEntry *de;
dictIterator *di = dictGetSafeIterator(cmd->subcommands_dict);
while ((de = dictNext(di)) != NULL) {
struct serverCommand *sub = (struct serverCommand *)dictGetVal(de);
if (cmd->subcommands_set) {
hashsetIterator iter;
hashsetInitSafeIterator(&iter, cmd->subcommands_set);
struct serverCommand *sub;
while (hashsetNext(&iter, (void **)&sub)) {
ACLSetSelectorCommandBit(selector, sub->id, allow);
}
dictReleaseIterator(di);
hashsetResetIterator(&iter);
}
}

Expand All @@ -669,19 +669,19 @@ void ACLChangeSelectorPerm(aclSelector *selector, struct serverCommand *cmd, int
* value. Since the category passed by the user may be non existing, the
* function returns C_ERR if the category was not found, or C_OK if it was
* found and the operation was performed. */
void ACLSetSelectorCommandBitsForCategory(dict *commands, aclSelector *selector, uint64_t cflag, int value) {
dictIterator *di = dictGetIterator(commands);
dictEntry *de;
while ((de = dictNext(di)) != NULL) {
struct serverCommand *cmd = dictGetVal(de);
void ACLSetSelectorCommandBitsForCategory(hashset *commands, aclSelector *selector, uint64_t cflag, int value) {
hashsetIterator iter;
hashsetInitIterator(&iter, commands);
struct serverCommand *cmd;
while (hashsetNext(&iter, (void **)&cmd)) {
if (cmd->acl_categories & cflag) {
ACLChangeSelectorPerm(selector, cmd, value);
}
if (cmd->subcommands_dict) {
ACLSetSelectorCommandBitsForCategory(cmd->subcommands_dict, selector, cflag, value);
if (cmd->subcommands_set) {
ACLSetSelectorCommandBitsForCategory(cmd->subcommands_set, selector, cflag, value);
}
}
dictReleaseIterator(di);
hashsetResetIterator(&iter);
}

/* This function is responsible for recomputing the command bits for all selectors of the existing users.
Expand Down Expand Up @@ -732,26 +732,26 @@ int ACLSetSelectorCategory(aclSelector *selector, const char *category, int allo
return C_OK;
}

void ACLCountCategoryBitsForCommands(dict *commands,
void ACLCountCategoryBitsForCommands(hashset *commands,
aclSelector *selector,
unsigned long *on,
unsigned long *off,
uint64_t cflag) {
dictIterator *di = dictGetIterator(commands);
dictEntry *de;
while ((de = dictNext(di)) != NULL) {
struct serverCommand *cmd = dictGetVal(de);
hashsetIterator iter;
hashsetInitIterator(&iter, commands);
struct serverCommand *cmd;
while (hashsetNext(&iter, (void **)&cmd)) {
if (cmd->acl_categories & cflag) {
if (ACLGetSelectorCommandBit(selector, cmd->id))
(*on)++;
else
(*off)++;
}
if (cmd->subcommands_dict) {
ACLCountCategoryBitsForCommands(cmd->subcommands_dict, selector, on, off, cflag);
if (cmd->subcommands_set) {
ACLCountCategoryBitsForCommands(cmd->subcommands_set, selector, on, off, cflag);
}
}
dictReleaseIterator(di);
hashsetResetIterator(&iter);
}

/* Return the number of commands allowed (on) and denied (off) for the user 'u'
Expand Down Expand Up @@ -1163,7 +1163,7 @@ int ACLSetSelector(aclSelector *selector, const char *op, size_t oplen) {
return C_ERR;
}

if (cmd->subcommands_dict) {
if (cmd->subcommands_set) {
/* If user is trying to allow a valid subcommand we can just add its unique ID */
cmd = ACLLookupCommand(op + 1);
if (cmd == NULL) {
Expand Down Expand Up @@ -2754,22 +2754,22 @@ sds getAclErrorMessage(int acl_res, user *user, struct serverCommand *cmd, sds e
* ==========================================================================*/

/* ACL CAT category */
void aclCatWithFlags(client *c, dict *commands, uint64_t cflag, int *arraylen) {
dictEntry *de;
dictIterator *di = dictGetIterator(commands);
void aclCatWithFlags(client *c, hashset *commands, uint64_t cflag, int *arraylen) {
hashsetIterator iter;
hashsetInitIterator(&iter, commands);

while ((de = dictNext(di)) != NULL) {
struct serverCommand *cmd = dictGetVal(de);
struct serverCommand *cmd;
while (hashsetNext(&iter, (void **)&cmd)) {
if (cmd->acl_categories & cflag) {
addReplyBulkCBuffer(c, cmd->fullname, sdslen(cmd->fullname));
(*arraylen)++;
}

if (cmd->subcommands_dict) {
aclCatWithFlags(c, cmd->subcommands_dict, cflag, arraylen);
if (cmd->subcommands_set) {
aclCatWithFlags(c, cmd->subcommands_set, cflag, arraylen);
}
}
dictReleaseIterator(di);
hashsetResetIterator(&iter);
}

/* Add the formatted response from a single selector to the ACL GETUSER
Expand Down
12 changes: 4 additions & 8 deletions src/config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,6 @@ void loadServerConfigFromString(char *config) {
loadServerConfig(argv[1], 0, NULL);
} else if (!strcasecmp(argv[0], "rename-command") && argc == 3) {
struct serverCommand *cmd = lookupCommandBySds(argv[1]);
int retval;

if (!cmd) {
err = "No such command in rename-command";
Expand All @@ -541,16 +540,13 @@ void loadServerConfigFromString(char *config) {

/* If the target command name is the empty string we just
* remove it from the command table. */
retval = dictDelete(server.commands, argv[1]);
serverAssert(retval == DICT_OK);
serverAssert(hashsetDelete(server.commands, argv[1]));

/* Otherwise we re-add the command under a different name. */
if (sdslen(argv[2]) != 0) {
sds copy = sdsdup(argv[2]);

retval = dictAdd(server.commands, copy, cmd);
if (retval != DICT_OK) {
sdsfree(copy);
sdsfree(cmd->fullname);
cmd->fullname = sdsdup(argv[2]);
if (!hashsetAdd(server.commands, cmd)) {
err = "Target command name already exists";
goto loaderr;
}
Expand Down
64 changes: 45 additions & 19 deletions src/db.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,7 @@ int objectTypeCompare(robj *o, long long target) {
}
/* This callback is used by scanGenericCommand in order to collect elements
* returned by the dictionary iterator into a list. */
void scanCallback(void *privdata, const dictEntry *de) {
void dictScanCallback(void *privdata, const dictEntry *de) {
scanData *data = (scanData *)privdata;
list *keys = data->keys;
robj *o = data->o;
Expand All @@ -911,8 +911,6 @@ void scanCallback(void *privdata, const dictEntry *de) {

if (o == NULL) {
key = keysds;
} else if (o->type == OBJ_SET) {
key = keysds;
} else if (o->type == OBJ_HASH) {
key = keysds;
if (!data->only_keys) {
Expand All @@ -926,13 +924,37 @@ void scanCallback(void *privdata, const dictEntry *de) {
val = sdsnewlen(buf, len);
}
} else {
serverPanic("Type not handled in SCAN callback.");
serverPanic("Type not handled in dict SCAN callback.");
}

listAddNodeTail(keys, key);
if (val) listAddNodeTail(keys, val);
}

void hashsetScanCallback(void *privdata, void *voidElement) {
scanData *data = (scanData *)privdata;
sds key = (sds)voidElement;

list *keys = data->keys;
robj *o = data->o;
data->sampled++;

/* o and typename can not have values at the same time. */
serverAssert(!((data->type != LLONG_MAX) && o));

// currently only implemented for SET scan
serverAssert(o && o->type == OBJ_SET && o->encoding == OBJ_ENCODING_HASHSET);

/* Filter element if it does not match the pattern. */
if (data->pattern) {
if (!stringmatchlen(data->pattern, sdslen(data->pattern), key, sdslen(key), 0)) {
return;
}
}

listAddNodeTail(keys, key);
}

/* Try to parse a SCAN cursor stored at object 'o':
* if the cursor is valid, store it as unsigned integer into *cursor and
* returns C_OK. Otherwise return C_ERR and send an error to the
Expand Down Expand Up @@ -996,7 +1018,6 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
sds typename = NULL;
long long type = LLONG_MAX;
int patlen = 0, use_pattern = 0, only_keys = 0;
dict *ht;

/* Object must be NULL (to iterate keys names), or the type of the object
* must be Set, Sorted Set, or Hash. */
Expand Down Expand Up @@ -1065,34 +1086,37 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
* just return everything inside the object in a single call, setting the
* cursor to zero to signal the end of the iteration. */

/* Handle the case of a hash table. */
ht = NULL;
/* Handle the case of a dict or hashset. */
dict *dictTable = NULL;
hashset *hashsetTable = NULL;
if (o == NULL) {
ht = NULL;
} else if (o->type == OBJ_SET && o->encoding == OBJ_ENCODING_HT) {
ht = o->ptr;
dictTable = NULL;
} else if (o->type == OBJ_SET && o->encoding == OBJ_ENCODING_HASHSET) {
hashsetTable = o->ptr;
} else if (o->type == OBJ_HASH && o->encoding == OBJ_ENCODING_HT) {
ht = o->ptr;
dictTable = o->ptr;
} else if (o->type == OBJ_ZSET && o->encoding == OBJ_ENCODING_SKIPLIST) {
zset *zs = o->ptr;
ht = zs->dict;
dictTable = zs->dict;
}

list *keys = listCreate();
/* Set a free callback for the contents of the collected keys list.
* For the main keyspace dict, and when we scan a key that's dict encoded
* (we have 'ht'), we don't need to define free method because the strings
* in the list are just a shallow copy from the pointer in the dictEntry.
* For the main keyspace dict, when we scan a key that's dict encoded
* (we have 'dictTable'), or when we scan a key that's hashset encoded
* (we have 'hashsetTable') we don't need to define free method because the
* strings in the list are just a shallow copy from the pointer in the
* dictEntry.
* When scanning a key with other encodings (e.g. listpack), we need to
* free the temporary strings we add to that list.
* The exception to the above is ZSET, where we do allocate temporary
* strings even when scanning a dict. */
if (o && (!ht || o->type == OBJ_ZSET)) {
if (o && ((!dictTable && !hashsetTable) || o->type == OBJ_ZSET)) {
listSetFreeMethod(keys, (void (*)(void *))sdsfree);
}

/* For main dictionary scan or data structure using hashtable. */
if (!o || ht) {
if (!o || dictTable || hashsetTable) {
/* We set the max number of iterations to ten times the specified
* COUNT, so if the hash table is in a pathological state (very
* sparsely populated) we avoid to block too much time at the cost
Expand Down Expand Up @@ -1130,9 +1154,11 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
/* In cluster mode there is a separate dictionary for each slot.
* If cursor is empty, we should try exploring next non-empty slot. */
if (o == NULL) {
cursor = kvstoreScan(c->db->keys, cursor, onlydidx, scanCallback, NULL, &data);
cursor = kvstoreScan(c->db->keys, cursor, onlydidx, dictScanCallback, NULL, &data);
} else if (dictTable) {
cursor = dictScan(dictTable, cursor, dictScanCallback, &data);
} else {
cursor = dictScan(ht, cursor, scanCallback, &data);
cursor = hashsetScan(hashsetTable, cursor, hashsetScanCallback, &data, 0);
}
} while (cursor && maxiterations-- && data.sampled < count);
} else if (o->type == OBJ_SET) {
Expand Down
29 changes: 17 additions & 12 deletions src/debug.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -915,30 +915,35 @@ void debugCommand(client *c) {
addReplyVerbatim(c, stats, sdslen(stats), "txt");
sdsfree(stats);
} else if (!strcasecmp(c->argv[1]->ptr, "htstats-key") && c->argc >= 3) {
robj *o;
dict *ht = NULL;
int full = 0;

if (c->argc >= 4 && !strcasecmp(c->argv[3]->ptr, "full")) full = 1;

if ((o = objectCommandLookupOrReply(c, c->argv[2], shared.nokeyerr)) == NULL) return;
robj *o = objectCommandLookupOrReply(c, c->argv[2], shared.nokeyerr);
if (o == NULL) return;

/* Get the hash table reference from the object, if possible. */
/* Get the dict reference from the object, if possible. */
dict *d = NULL;
hashset *hs = NULL;
switch (o->encoding) {
case OBJ_ENCODING_SKIPLIST: {
zset *zs = o->ptr;
ht = zs->dict;
d = zs->dict;
} break;
case OBJ_ENCODING_HT: ht = o->ptr; break;
case OBJ_ENCODING_HT: d = o->ptr; break;
case OBJ_ENCODING_HASHSET: hs = o->ptr; break;
}

if (ht == NULL) {
addReplyError(c, "The value stored at the specified key is not "
"represented using an hash table");
} else {
if (d != NULL) {
char buf[4096];
dictGetStats(buf, sizeof(buf), ht, full);
dictGetStats(buf, sizeof(buf), d, full);
addReplyVerbatim(c, buf, strlen(buf), "txt");
} else if (hs != NULL) {
char buf[4096];
hashsetGetStats(buf, sizeof(buf), hs, full);
addReplyVerbatim(c, buf, strlen(buf), "txt");
} else {
addReplyError(c, "The value stored at the specified key is not "
"represented using an hash table");
}
} else if (!strcasecmp(c->argv[1]->ptr, "change-repl-id") && c->argc == 2) {
serverLog(LL_NOTICE, "Changing replication IDs after receiving DEBUG change-repl-id");
Expand Down
Loading
Loading