Skip to content

Commit 16e4ff4

Browse files
alonre24alonre24
authored
[MOD-7949] Avoid lazy expire upon scan keys in background [2.8] (#5303)
Fix for 2.8
1 parent 09f5b62 commit 16e4ff4

3 files changed

Lines changed: 48 additions & 4 deletions

File tree

‎src/document_basic.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,9 @@ void Document_MakeRefOwner(Document *doc) {
109109
}
110110

111111
int Document_LoadSchemaFieldHash(Document *doc, RedisSearchCtx *sctx, QueryError *status) {
112-
RedisModuleKey *k = RedisModule_OpenKey(sctx->redisCtx, doc->docKey, REDISMODULE_READ);
112+
RedisModuleKey *k = RedisModule_OpenKey(sctx->redisCtx, doc->docKey, REDISMODULE_READ | REDISMODULE_OPEN_KEY_NOEFFECTS);
113113
int rv = REDISMODULE_ERR;
114-
// DvirDu: Is this even possible?
114+
// This is possible if the key has expired for example in previous redis API calls in this notification flow.
115115
if (!k || RedisModule_KeyType(k) != REDISMODULE_KEYTYPE_HASH) {
116116
QueryError_SetErrorFmt(status, QUERY_EINVAL, "Key %s does not exist or is not a hash", RedisModule_StringPtrLen(doc->docKey, NULL));
117117
goto done;
@@ -175,7 +175,7 @@ int Document_LoadSchemaFieldJson(Document *doc, RedisSearchCtx *sctx, QueryError
175175
size_t nitems = sctx->spec->numFields;
176176
JSONResultsIterator jsonIter = NULL;
177177

178-
RedisJSON jsonRoot = japi->openKey(ctx, doc->docKey);
178+
RedisJSON jsonRoot = japi->openKeyWithFlags(ctx, doc->docKey, REDISMODULE_OPEN_KEY_NOEFFECTS);
179179
if (!jsonRoot) {
180180
goto done;
181181
}

‎src/spec.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2024,7 +2024,7 @@ static void Indexes_ScanProc(RedisModuleCtx *ctx, RedisModuleString *keyname, Re
20242024
// RMKey it is provided as best effort but in some cases it might be NULL
20252025
bool keyOpened = false;
20262026
if (!key) {
2027-
key = RedisModule_OpenKey(ctx, keyname, REDISMODULE_READ);
2027+
key = RedisModule_OpenKey(ctx, keyname, REDISMODULE_READ | REDISMODULE_OPEN_KEY_NOEFFECTS);
20282028
keyOpened = true;
20292029
}
20302030

‎tests/pytests/test_expire.py‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,3 +235,47 @@ def test_expire_aggregate(env):
235235
# This test ensures that the flag indicating expiration is cleared and the search result struct is ready to be re-used.
236236
res = conn.execute_command('FT.AGGREGATE', 'idx', '*', 'LOAD', 1, '@t')
237237
env.assertEqual(res, [1, None, ['t', 'arr']])
238+
239+
240+
# Verify that background indexing does not cause lazy expiration of expired documents.
241+
@skip(cluster=True)
242+
def test_background_index_no_lazy_expiration(env):
243+
env.cmd('DEBUG', 'SET-ACTIVE-EXPIRE', '0')
244+
env.expect('HSET', 'doc:1', 't', 'bar').equal(1)
245+
env.expect('HSET', 'doc:2', 't', 'arr').equal(1)
246+
env.expect('EXPIRE', 'doc:1', '1').equal(1)
247+
time.sleep(1.5)
248+
249+
# Expect background indexing to take place after doc:1 has expired.
250+
env.expect('FT.CREATE', 'idx', 'SCHEMA', 't', 'TEXT').equal('OK')
251+
waitForIndex(env, 'idx')
252+
253+
# Validate that doc:1 has expired but not evicted.
254+
env.expect('FT.SEARCH', 'idx', '*').equal([1, 'doc:2', ['t', 'arr']])
255+
env.expect('DBSIZE').equal(2)
256+
257+
# Accessing doc:1 directly should cause lazy expire and its removal from the DB.
258+
env.expect('HGET', 'doc:1', 't').equal(None)
259+
env.expect('DBSIZE').equal(1)
260+
261+
262+
# Same test as the above but for JSON documents.
263+
@skip(cluster=True)
264+
def test_background_index_no_lazy_expiration_json(env):
265+
env.cmd('DEBUG', 'SET-ACTIVE-EXPIRE', '0')
266+
env.expect('JSON.SET', 'doc:1', "$", r'{"t":"bar"}').ok()
267+
env.expect('JSON.SET', 'doc:2', "$", r'{"t":"arr"}').ok()
268+
env.expect('EXPIRE', 'doc:1', '1').equal(1)
269+
time.sleep(1.5)
270+
271+
# Expect background indexing to take place after doc:1 has expired.
272+
env.expect('FT.CREATE', 'idx', 'ON', 'JSON', 'SCHEMA', 't', 'TEXT').equal('OK')
273+
waitForIndex(env, 'idx')
274+
275+
# Validate that doc:1 has expired but not evicted.
276+
env.expect('FT.SEARCH', 'idx', '*').equal([1, 'doc:2', ['$', '{"t":"arr"}']])
277+
env.expect('DBSIZE').equal(2)
278+
279+
# Accessing doc:1 directly should cause lazy expire and its removal from the DB.
280+
env.expect('JSON.GET', 'doc:1', "$").equal(None)
281+
env.expect('DBSIZE').equal(1)

0 commit comments

Comments
 (0)