Skip to content

Commit f75f3b9

Browse files
GuyAv46GuyAv46
committed
Refactor keys dict - [MOD-13151] (#7843)
* mark legacy API as such * refactor spec's keysDict to hold char buffer * cleanups * make keysDict a dict for inverted indexes only (simplify value struct) * use bool * some comment fixes * address review comment * dead code cleanup (cherry picked from commit 692e14f)
1 parent d5bc361 commit f75f3b9

7 files changed

Lines changed: 94 additions & 121 deletions

File tree

‎src/fork_gc.c‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -584,13 +584,11 @@ static FGCError FGC_parentHandleTerms(ForkGC *gc) {
584584
if (InvertedIndex_NumDocs(idx) == 0) {
585585

586586
// inverted index was cleaned entirely lets free it
587-
RedisModuleString *termKey = fmtRedisTermKey(sctx, term, len);
588-
size_t formatedTremLen;
589-
const char *formatedTrem = RedisModule_StringPtrLen(termKey, &formatedTremLen);
590587
if (sctx->spec->keysDict) {
588+
CharBuf termKey = {.buf = term, .len = len};
591589
// get memory before deleting the inverted index
592590
size_t inv_idx_size = InvertedIndex_MemUsage(idx);
593-
if (dictDelete(sctx->spec->keysDict, termKey) == DICT_OK) {
591+
if (dictDelete(sctx->spec->keysDict, &termKey) == DICT_OK) {
594592
info.nbytesCollected += inv_idx_size;
595593
}
596594
}
@@ -602,7 +600,6 @@ static FGCError FGC_parentHandleTerms(ForkGC *gc) {
602600
}
603601
sctx->spec->stats.numTerms--;
604602
sctx->spec->stats.termsSize -= len;
605-
RedisModule_FreeString(sctx->redisCtx, termKey);
606603
if (sctx->spec->suffix) {
607604
deleteSuffixTrie(sctx->spec->suffix, term, len);
608605
}
@@ -625,7 +622,6 @@ static FGCError FGC_parentHandleNumeric(ForkGC *gc) {
625622
size_t fieldNameLen;
626623
char *fieldName = NULL;
627624
const FieldSpec *fs = NULL;
628-
RedisModuleString *keyName = NULL;
629625
uint64_t rtUniqueId;
630626
NumericRangeTree *rt = NULL;
631627
FGCError status = recvNumericTagHeader(gc, &fieldName, &fieldNameLen, &rtUniqueId);
@@ -725,7 +721,6 @@ static FGCError FGC_parentHandleTags(ForkGC *gc) {
725721
while (status == FGC_COLLECTED) {
726722
InvertedIndexGcDelta *delta = NULL;
727723
II_GCScanStats info = {0};
728-
RedisModuleString *keyName = NULL;
729724
TagIndex *tagIdx = NULL;
730725
char *tagVal = NULL;
731726
size_t tagValLen;

‎src/info/field_spec_info.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ size_t IndexSpec_VectorIndexesSize(IndexSpec *sp) {
184184
return stats.memory;
185185
}
186186

187-
// Get the stats of the vector field `fs` in the index `sp`.
187+
// Get the stats of the vector field `fs`.
188188
VectorIndexStats IndexSpec_GetVectorIndexStats(FieldSpec *fs){
189189
VectorIndexStats stats = {0};
190190
VecSimIndex *vecsim = openVectorIndex(fs, DONT_CREATE_INDEX);
@@ -211,7 +211,7 @@ VectorIndexStats IndexSpec_GetVectorIndexesStats(IndexSpec *sp) {
211211
return stats;
212212
}
213213

214-
// Get the stats of the field `fs` in the index `sp`.
214+
// Get the stats of the field `fs`.
215215
FieldSpecStats IndexSpec_GetFieldStats(FieldSpec *fs){
216216
FieldSpecStats stats = {0};
217217
stats.type = fs->types;
@@ -224,7 +224,7 @@ FieldSpecStats IndexSpec_GetFieldStats(FieldSpec *fs){
224224
}
225225
}
226226

227-
// Get the information of the field `fs` in the index `sp`.
227+
// Get the information of the field `fs`.
228228
FieldSpecInfo FieldSpec_GetInfo(FieldSpec *fs, bool obfuscate) {
229229
FieldSpecInfo info = {0};
230230
FieldSpecInfo_SetIdentifier(&info, FieldSpec_FormatPath(fs, obfuscate));

‎src/query.c‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -518,13 +518,12 @@ static void QueryNode_Expand(RSQueryTokenExpander expander, RSQueryExpanderCtx *
518518

519519
QueryIterator *Query_EvalTokenNode(QueryEvalCtx *q, QueryNode *qn) {
520520
RS_LOG_ASSERT(qn->type == QN_TOKEN, "query node type should be token")
521-
RSQueryTerm *term = NewQueryTerm(&qn->tn, q->tokenId++);
522521

523522
if (q->sctx->spec->diskSpec) {
524523
RS_LOG_ASSERT(q->sctx->spec->diskSpec, "Disk spec should be open");
525-
return SearchDisk_NewTermIterator(q->sctx->spec->diskSpec, term->str, EFFECTIVE_FIELDMASK(q, qn), qn->opts.weight);
524+
return SearchDisk_NewTermIterator(q->sctx->spec->diskSpec, qn->tn.str, EFFECTIVE_FIELDMASK(q, qn), qn->opts.weight);
526525
} else {
527-
return Redis_OpenReader(q->sctx, term, q->docTable, EFFECTIVE_FIELDMASK(q, qn), qn->opts.weight);
526+
return Redis_OpenReader(q->sctx, &qn->tn, q->tokenId++, q->docTable, EFFECTIVE_FIELDMASK(q, qn), qn->opts.weight);
528527
}
529528
}
530529

@@ -538,15 +537,14 @@ static inline void addTerm(char *str, size_t tok_len, QueryEvalCtx *q,
538537
.str = str
539538
};
540539

541-
RSQueryTerm *term = NewQueryTerm(&tok, q->tokenId++);
542540
QueryIterator *ir = NULL;
543541

544542
if (q->sctx->spec->diskSpec) {
545543
RS_LOG_ASSERT(q->sctx->spec->diskSpec, "Disk spec should be open");
546-
ir = SearchDisk_NewTermIterator(q->sctx->spec->diskSpec, term->str, q->opts->fieldmask & opts->fieldMask, 1);
544+
ir = SearchDisk_NewTermIterator(q->sctx->spec->diskSpec, tok.str, q->opts->fieldmask & opts->fieldMask, 1);
547545
} else {
548546
// Open an index reader
549-
ir = Redis_OpenReader(q->sctx, term, &q->sctx->spec->docs,
547+
ir = Redis_OpenReader(q->sctx, &tok, q->tokenId++, &q->sctx->spec->docs,
550548
q->opts->fieldmask & opts->fieldMask, 1);
551549
}
552550

@@ -782,11 +780,10 @@ static int runeIterCb(const rune *r, size_t n, void *p, void *payload) {
782780
RSToken tok = {0};
783781
tok.str = runesToStr(r, n, &tok.len);
784782
QueryIterator *ir = NULL;
785-
RSQueryTerm *term = NewQueryTerm(&tok, ctx->q->tokenId++);
786783
if (q->sctx->spec->diskSpec) {
787-
ir = SearchDisk_NewTermIterator(q->sctx->spec->diskSpec, term->str, q->opts->fieldmask & ctx->opts->fieldMask, 1);
784+
ir = SearchDisk_NewTermIterator(q->sctx->spec->diskSpec, tok.str, q->opts->fieldmask & ctx->opts->fieldMask, 1);
788785
} else {
789-
ir = Redis_OpenReader(q->sctx, term, &q->sctx->spec->docs,
786+
ir = Redis_OpenReader(q->sctx, &tok, ctx->q->tokenId++, &q->sctx->spec->docs,
790787
q->opts->fieldmask & ctx->opts->fieldMask, 1);
791788
}
792789
rm_free(tok.str);
@@ -805,12 +802,11 @@ static int charIterCb(const char *s, size_t n, void *p, void *payload) {
805802
return REDISEARCH_ERR;
806803
}
807804
RSToken tok = {.str = (char *)s, .len = n};
808-
RSQueryTerm *term = NewQueryTerm(&tok, q->tokenId++);
809805
QueryIterator *ir = NULL;
810806
if (q->sctx->spec->diskSpec) {
811-
ir = SearchDisk_NewTermIterator(q->sctx->spec->diskSpec, term->str, q->opts->fieldmask & ctx->opts->fieldMask, 1);
807+
ir = SearchDisk_NewTermIterator(q->sctx->spec->diskSpec, tok.str, q->opts->fieldmask & ctx->opts->fieldMask, 1);
812808
} else {
813-
ir = Redis_OpenReader(q->sctx, term, &q->sctx->spec->docs,
809+
ir = Redis_OpenReader(q->sctx, &tok, q->tokenId++, &q->sctx->spec->docs,
814810
q->opts->fieldmask & ctx->opts->fieldMask, 1);
815811
}
816812
if (ir) {

‎src/redis_index.c‎

Lines changed: 31 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
#include "rmalloc.h"
2020
#include <stdio.h>
2121

22-
RedisModuleType *InvertedIndexType;
23-
2422
static inline void updateTime(SearchTime *searchTime, int32_t durationNS) {
2523
if (RS_IsMock) return;
2624

@@ -49,7 +47,7 @@ static inline void updateTime(SearchTime *searchTime, int32_t durationNS) {
4947
/**
5048
* Format redis key for a term.
5149
*/
52-
RedisModuleString *fmtRedisTermKey(const RedisSearchCtx *ctx, const char *term, size_t len) {
50+
RedisModuleString *Legacy_fmtRedisTermKey(const RedisSearchCtx *ctx, const char *term, size_t len) {
5351
char buf_s[1024] = {"ft:"};
5452
size_t offset = 3;
5553
size_t nameLen = 0;
@@ -72,12 +70,15 @@ RedisModuleString *fmtRedisTermKey(const RedisSearchCtx *ctx, const char *term,
7270
return ret;
7371
}
7472

75-
RedisModuleString *fmtRedisSkipIndexKey(const RedisSearchCtx *ctx, const char *term, size_t len) {
73+
#define SKIPINDEX_KEY_FORMAT "si:%s/%.*s"
74+
#define SCOREINDEX_KEY_FORMAT "ss:%s/%.*s"
75+
76+
RedisModuleString *Legacy_fmtRedisSkipIndexKey(const RedisSearchCtx *ctx, const char *term, size_t len) {
7677
return RedisModule_CreateStringPrintf(ctx->redisCtx, SKIPINDEX_KEY_FORMAT, HiddenString_GetUnsafe(ctx->spec->specName, NULL),
7778
(int)len, term);
7879
}
7980

80-
RedisModuleString *fmtRedisScoreIndexKey(const RedisSearchCtx *ctx, const char *term, size_t len) {
81+
RedisModuleString *Legacy_fmtRedisScoreIndexKey(const RedisSearchCtx *ctx, const char *term, size_t len) {
8182
return RedisModule_CreateStringPrintf(ctx->redisCtx, SCOREINDEX_KEY_FORMAT, HiddenString_GetUnsafe(ctx->spec->specName, NULL),
8283
(int)len, term);
8384
}
@@ -146,46 +147,37 @@ void SearchCtx_Free(RedisSearchCtx *sctx) {
146147
rm_free(sctx);
147148
}
148149

149-
static InvertedIndex *openIndexKeysDict(const RedisSearchCtx *ctx, RedisModuleString *termKey,
150-
int write, bool *outIsNew) {
151-
KeysDictValue *kdv = dictFetchValue(ctx->spec->keysDict, termKey);
152-
if (kdv) {
153-
if (outIsNew) {
154-
*outIsNew = false;
155-
}
156-
return kdv->p;
157-
}
158-
if (!write) {
159-
return NULL;
160-
}
161-
150+
static InvertedIndex *openIndexKeysDict(const RedisSearchCtx *ctx, CharBuf *termKey,
151+
bool write, bool *outIsNew) {
152+
InvertedIndex *idx = dictFetchValue(ctx->spec->keysDict, termKey);
162153
if (outIsNew) {
163-
*outIsNew = true;
154+
*outIsNew = idx == NULL;
164155
}
165-
kdv = rm_calloc(1, sizeof(*kdv));
166-
kdv->dtor = (void (*)(void *))InvertedIndex_Free;
167-
size_t index_size;
168-
kdv->p = NewInvertedIndex(ctx->spec->flags, &index_size);
169-
ctx->spec->stats.invertedSize += index_size;
170-
dictAdd(ctx->spec->keysDict, termKey, kdv);
171-
return kdv->p;
156+
if (write && !idx) {
157+
size_t index_size;
158+
idx = NewInvertedIndex(ctx->spec->flags, &index_size);
159+
ctx->spec->stats.invertedSize += index_size;
160+
dictAdd(ctx->spec->keysDict, termKey, idx);
161+
}
162+
return idx;
172163
}
173164

174-
InvertedIndex *Redis_OpenInvertedIndex(const RedisSearchCtx *ctx, const char *term, size_t len, int write, bool *outIsNew) {
175-
RedisModuleString *termKey = fmtRedisTermKey(ctx, term, len);
176-
InvertedIndex *idx = openIndexKeysDict(ctx, termKey, write, outIsNew);
177-
RedisModule_FreeString(ctx->redisCtx, termKey);
165+
InvertedIndex *Redis_OpenInvertedIndex(const RedisSearchCtx *ctx, const char *term, size_t len, bool write, bool *outIsNew) {
166+
CharBuf termKeyBuf = {
167+
.buf = (char *)term,
168+
.len = len,
169+
};
170+
InvertedIndex *idx = openIndexKeysDict(ctx, &termKeyBuf, write, outIsNew);
178171
return idx;
179172
}
180173

181-
QueryIterator *Redis_OpenReader(const RedisSearchCtx *ctx, RSQueryTerm *term, DocTable *dt,
174+
QueryIterator *Redis_OpenReader(const RedisSearchCtx *ctx, RSToken *tok, int tok_id, DocTable *dt,
182175
t_fieldMask fieldMask, double weight) {
183176

184-
RedisModuleString *termKey = fmtRedisTermKey(ctx, term->str, term->len);
185177
InvertedIndex *idx = NULL;
186-
RedisModuleKey *k = NULL;
178+
CharBuf termKey = {.buf = tok->str, .len = tok->len};
187179

188-
idx = openIndexKeysDict(ctx, termKey, 0, NULL);
180+
idx = openIndexKeysDict(ctx, &termKey, false, NULL);
189181
if (!idx) {
190182
goto err;
191183
}
@@ -198,27 +190,18 @@ QueryIterator *Redis_OpenReader(const RedisSearchCtx *ctx, RSQueryTerm *term, Do
198190
}
199191

200192
FieldMaskOrIndex fieldMaskOrIndex = {.isFieldMask = true, .value.mask = fieldMask};
193+
RSQueryTerm *term = NewQueryTerm(tok, tok_id);
201194
QueryIterator *it = NewInvIndIterator_TermQuery(idx, ctx, fieldMaskOrIndex, term, weight);
202-
RedisModule_FreeString(ctx->redisCtx, termKey);
203195
return it;
204196

205197
err:
206-
if (k) {
207-
RedisModule_CloseKey(k);
208-
}
209-
if (termKey) {
210-
RedisModule_FreeString(ctx->redisCtx, termKey);
211-
}
212-
if (term) {
213-
Term_Free(term);
214-
}
215198
return NULL;
216199
}
217200

218-
int Redis_DropScanHandler(RedisModuleCtx *ctx, RedisModuleString *kn, void *opaque) {
201+
int Redis_LegacyDropScanHandler(RedisModuleCtx *ctx, RedisModuleString *kn, void *opaque) {
219202
// extract the term from the key
220203
RedisSearchCtx *sctx = opaque;
221-
RedisModuleString *pf = fmtRedisTermKey(sctx, "", 0);
204+
RedisModuleString *pf = Legacy_fmtRedisTermKey(sctx, "", 0);
222205
size_t pflen, len;
223206
RedisModule_StringPtrLen(pf, &pflen);
224207
RedisModule_FreeString(sctx->redisCtx, pf);
@@ -227,8 +210,8 @@ int Redis_DropScanHandler(RedisModuleCtx *ctx, RedisModuleString *kn, void *opaq
227210
k += pflen;
228211
// char *term = rm_strndup(k, len - pflen);
229212

230-
RedisModuleString *sck = fmtRedisScoreIndexKey(sctx, k, len - pflen);
231-
RedisModuleString *sik = fmtRedisSkipIndexKey(sctx, k, len - pflen);
213+
RedisModuleString *sck = Legacy_fmtRedisScoreIndexKey(sctx, k, len - pflen);
214+
RedisModuleString *sik = Legacy_fmtRedisSkipIndexKey(sctx, k, len - pflen);
232215

233216
RedisModuleCallReply *rep = RedisModule_Call(ctx, "DEL", "sss", kn, sck, sik);
234217
if (rep) {

‎src/redis_index.h‎

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,44 +19,24 @@
1919
/* Open an inverted index reader on a redis DMA string, for a specific term.
2020
* If singleWordMode is set to 1, we do not load the skip index, only the score index
2121
*/
22-
QueryIterator *Redis_OpenReader(const RedisSearchCtx *ctx, RSQueryTerm *term, DocTable *dt,
22+
QueryIterator *Redis_OpenReader(const RedisSearchCtx *ctx, RSToken *tok, int tok_id, DocTable *dt,
2323
t_fieldMask fieldMask, double weight);
2424

2525
InvertedIndex *Redis_OpenInvertedIndex(const RedisSearchCtx *ctx, const char *term, size_t len,
26-
int write, bool *outIsNew);
27-
28-
/*
29-
* Select a random term from the index that matches the index prefix and inveted key format.
30-
* It tries RANDOMKEY 10 times and returns NULL if it can't find anything.
31-
*/
32-
const char *Redis_SelectRandomTerm(const RedisSearchCtx *ctx, size_t *tlen);
33-
34-
#define TERM_KEY_FORMAT "ft:%s/%.*s"
35-
#define TERM_KEY_PREFIX "ft:"
36-
#define SKIPINDEX_KEY_FORMAT "si:%s/%.*s"
37-
#define SCOREINDEX_KEY_FORMAT "ss:%s/%.*s"
26+
bool write, bool *outIsNew);
3827

3928
#define DONT_CREATE_INDEX false
4029
#define CREATE_INDEX true
4130

42-
typedef int (*ScanFunc)(RedisModuleCtx *ctx, RedisModuleString *keyName, void *opaque);
43-
44-
/* Optimize the buffers of a speicif term hit */
45-
int Redis_OptimizeScanHandler(RedisModuleCtx *ctx, RedisModuleString *kn, void *opaque);
46-
4731
int Redis_LegacyDeleteKey(RedisModuleCtx *ctx, RedisModuleString *s);
4832
int Redis_DeleteKeyC(RedisModuleCtx *ctx, char *cstr);
4933

5034
/* Drop all the index's internal keys using this scan handler */
51-
int Redis_DropScanHandler(RedisModuleCtx *ctx, RedisModuleString *kn, void *opaque);
35+
int Redis_LegacyDropScanHandler(RedisModuleCtx *ctx, RedisModuleString *kn, void *opaque);
5236

53-
/* Collect memory stas on the index */
54-
int Redis_StatsScanHandler(RedisModuleCtx *ctx, RedisModuleString *kn, void *opaque);
5537
/**
5638
* Format redis key for a term.
57-
* TODO: Add index name to it
5839
*/
59-
RedisModuleString *fmtRedisTermKey(const RedisSearchCtx *ctx, const char *term, size_t len);
60-
RedisModuleString *fmtRedisSkipIndexKey(const RedisSearchCtx *ctx, const char *term, size_t len);
40+
RedisModuleString *Legacy_fmtRedisTermKey(const RedisSearchCtx *ctx, const char *term, size_t len);
6141

6242
#endif

0 commit comments

Comments
 (0)