MOD-7329: Determine if fields and documents are expired before returning them as valid results by kei-nan · Pull Request #4858 · RediSearch/RediSearch

kei-nan · 2024-07-11T12:46:58Z

Add an internal ttl table to contain document and field expiration information
- there will be a thread local timestamp that we check the expiration times against
- it will be updated when a batch of results is obtained from the pipeline
In our field related iterators check if a field is expired before returning the document as a possible result
- In query and hybrid iterators it required adding a loop, we might need to consider handling timeout
- note it will require a lot of fields being expired at roughly the same time
In our result processor check if the document itself was expired
This set of changes helps in the case of lazy expiration to some degree.
- if the document/field expires between us retrieving it and opening it and retrieving its fields then the problematic edge case remains.
- We will need a change at the redis level to be able to solve the problem 100%.

A clear and concise description of what the PR is solving, including:

Documents are expired during the query in a lazy fashion which causes empty documents to be returned to the user
- For fields the issue is even trickier since we can return what the user perceive as incorrect results.
Try and minimize the edge case by checking for the expiration before returning a result up the chain
Less empty documents will be returned to the user.

Which issues this PR fixes

MOD-7329

Main objects this PR modified

Iterators relating to fields
Document Table

Mark if applicable

This PR introduces API changes
This PR introduces serialization changes

codecov · 2024-07-16T07:52:53Z

Codecov Report

Attention: Patch coverage is 94.18103% with 27 lines in your changes missing coverage. Please review.

Project coverage is 86.18%. Comparing base (428f802) to head (1a62cdd).
Report is 22 commits behind head on master.

Files with missing lines	Patch %	Lines
src/debug_commands.c	84.84%	5 Missing ⚠️
src/ttl_table.c	93.05%	5 Missing ⚠️
src/hybrid_reader.c	87.09%	4 Missing ⚠️
src/inverted_index.c	90.00%	4 Missing ⚠️
src/query_optimizer.c	25.00%	3 Missing ⚠️
src/spec.c	92.30%	3 Missing ⚠️
src/numeric_index.c	83.33%	2 Missing ⚠️
src/geometry/query_iterator.cpp	93.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4858      +/-   ##
==========================================
+ Coverage   86.12%   86.18%   +0.05%     
==========================================
  Files         192      193       +1     
  Lines       34416    34704     +288     
==========================================
+ Hits        29642    29910     +268     
- Misses       4774     4794      +20

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kei-nan · 2024-09-29T19:04:40Z

/backport

github-actions · 2024-09-29T19:04:54Z

Backport failed for 2.10, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.10
git worktree add -d .worktree/backport-4858-to-2.10 origin/2.10
cd .worktree/backport-4858-to-2.10
git switch --create backport-4858-to-2.10
git cherry-pick -x 4a67a87bbe11cdb8d4d852ee3b71fd21824f4047

…ing them as valid results (#4858) * * initial commit * * fix missing case in test_expire.py * * fix coord compilation * * fix doc info test * * try and fix some of the tests * * release iterator * * do not set expiration time in the context of the coordinator since it doesn't have an index spec * * support c++ tests that do not provide spec for iterators * * no point in passing timestamp in coordinator flow * * redis 7.4 version in ci is actually 7.3, for codecov we need our tests to run * * reduce max memory from + 200000 to 150000 * * fix test * * cleanup code * * Code Review: Round #1 * * Add geoshape tests, only pass index instead of pointer to filter ctx * * Add vector test * * Code Review - Round #2 * * try and fix tests * * Code Review - Round #3 * * Fix Sanitizer Build * * Code Review - Round #4 * * Minor change in debug command SET_MONITOR_EXPIRATION * * Code Review - Round #5 * * fix tests * * Code Review - Round #6 * * fix crash * * fix test * * improve field mask translation time * fix compilation * fix compilation error * handle field mask of 64 bit * * Code Review - Round #7 * * fix opening of key edge cases * * free fieldIdToIndex array * Code Review - Round #9 * * fix rebase conflicts * support wild card reader filtering * * mac os: fallback to CLOCK_REALTIME if CLOCK_REALTIME_COARSE is not defined (cherry picked from commit 4a67a87)

kei-nan · 2024-10-13T09:36:24Z

/backport

redisearch-backport-pull-request · 2024-10-13T09:36:44Z

Backport failed for 2.10, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.10
git worktree add -d .worktree/backport-4858-to-2.10 origin/2.10
cd .worktree/backport-4858-to-2.10
git switch --create backport-4858-to-2.10
git cherry-pick -x 4a67a87bbe11cdb8d4d852ee3b71fd21824f4047

redisearch-backport-pull-request · 2024-10-13T09:36:45Z

Backport failed for 8.0, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 8.0
git worktree add -d .worktree/backport-4858-to-8.0 origin/8.0
cd .worktree/backport-4858-to-8.0
git switch --create backport-4858-to-8.0
git cherry-pick -x 4a67a87bbe11cdb8d4d852ee3b71fd21824f4047

…ing them as valid results (#4858) * * initial commit * * fix missing case in test_expire.py * * fix coord compilation * * fix doc info test * * try and fix some of the tests * * release iterator * * do not set expiration time in the context of the coordinator since it doesn't have an index spec * * support c++ tests that do not provide spec for iterators * * no point in passing timestamp in coordinator flow * * redis 7.4 version in ci is actually 7.3, for codecov we need our tests to run * * reduce max memory from + 200000 to 150000 * * fix test * * cleanup code * * Code Review: Round #1 * * Add geoshape tests, only pass index instead of pointer to filter ctx * * Add vector test * * Code Review - Round #2 * * try and fix tests * * Code Review - Round #3 * * Fix Sanitizer Build * * Code Review - Round #4 * * Minor change in debug command SET_MONITOR_EXPIRATION * * Code Review - Round #5 * * fix tests * * Code Review - Round #6 * * fix crash * * fix test * * improve field mask translation time * fix compilation * fix compilation error * handle field mask of 64 bit * * Code Review - Round #7 * * fix opening of key edge cases * * free fieldIdToIndex array * Code Review - Round #9 * * fix rebase conflicts * support wild card reader filtering * * mac os: fallback to CLOCK_REALTIME if CLOCK_REALTIME_COARSE is not defined (cherry picked from commit 4a67a87)

…returning them as valid results (#5118) * MOD-7329: Determine if fields and documents are expired before returning them as valid results (#4858) * * initial commit * * fix missing case in test_expire.py * * fix coord compilation * * fix doc info test * * try and fix some of the tests * * release iterator * * do not set expiration time in the context of the coordinator since it doesn't have an index spec * * support c++ tests that do not provide spec for iterators * * no point in passing timestamp in coordinator flow * * redis 7.4 version in ci is actually 7.3, for codecov we need our tests to run * * reduce max memory from + 200000 to 150000 * * fix test * * cleanup code * * Code Review: Round #1 * * Add geoshape tests, only pass index instead of pointer to filter ctx * * Add vector test * * Code Review - Round #2 * * try and fix tests * * Code Review - Round #3 * * Fix Sanitizer Build * * Code Review - Round #4 * * Minor change in debug command SET_MONITOR_EXPIRATION * * Code Review - Round #5 * * fix tests * * Code Review - Round #6 * * fix crash * * fix test * * improve field mask translation time * fix compilation * fix compilation error * handle field mask of 64 bit * * Code Review - Round #7 * * fix opening of key edge cases * * free fieldIdToIndex array * Code Review - Round #9 * * fix rebase conflicts * support wild card reader filtering * * mac os: fallback to CLOCK_REALTIME if CLOCK_REALTIME_COARSE is not defined (cherry picked from commit 4a67a87) * MOD-7979: Protect Against RS_FIELDMASK_ALL Mask Edge Case (#5128) * * check mask edge case * * Code Review - Round #1 (cherry picked from commit 2c037bc) * [8.0] MOD-7976: Fix HFE Regression (#5336) MOD-7976: Fix HFE Regression (#5318) * * use new expiration API to avoid regression issues with HFE support when expiration isn't being used * * update sdk to get new hash field expiration API * * set monitorFieldExpiration default based on the existence of `RedisModule_HashFieldMinExpire` * * skip field expiration tests if below 8.0 API * * Code Review Comments * Prevent turning on the monitorFieldExpiration if the API does not exist * * move to run against redis 8.0 (cherry picked from commit bebdeb7) * * rebase fixes * * add REDISMODULE_OPEN_KEY_NOEFFECTS to OpenKey when loading json document * * fix for test_debug_commands.py

kei-nan force-pushed the jk_hfe_search_solution branch from d53be30 to 7fae25e Compare July 11, 2024 12:54

kei-nan requested review from GuyAv46 and raz-mon July 11, 2024 12:55

kei-nan force-pushed the jk_hfe_search_solution branch 6 times, most recently from ef277e8 to b07f62d Compare July 16, 2024 07:28

kei-nan force-pushed the jk_hfe_search_solution branch 5 times, most recently from 30a1c7a to 5a72cd3 Compare July 16, 2024 13:56

kei-nan marked this pull request as draft July 16, 2024 14:44

kei-nan self-assigned this Jul 16, 2024

kei-nan marked this pull request as ready for review July 17, 2024 09:12

kei-nan removed request for GuyAv46 and raz-mon July 17, 2024 09:12

kei-nan force-pushed the jk_hfe_search_solution branch 5 times, most recently from 86c301f to 9eb1e30 Compare July 17, 2024 15:27

kei-nan requested review from GuyAv46 and raz-mon and removed request for GuyAv46 and raz-mon July 17, 2024 16:00

kei-nan force-pushed the jk_hfe_search_solution branch from 170b87e to 1f73df0 Compare July 18, 2024 16:17

kei-nan enabled auto-merge September 10, 2024 13:15

raz-mon approved these changes Sep 10, 2024

View reviewed changes

kei-nan added this pull request to the merge queue Sep 10, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 10, 2024

kei-nan enabled auto-merge September 11, 2024 06:52

kei-nan added this pull request to the merge queue Sep 15, 2024

Merged via the queue into master with commit 4a67a87 Sep 15, 2024

kei-nan deleted the jk_hfe_search_solution branch September 15, 2024 18:42

kei-nan added the backport 2.10 label Sep 29, 2024

kei-nan mentioned this pull request Oct 1, 2024

[2.10] MOD-7329: Determine if fields and documents are expired before returning them as valid results #5067

Closed

kei-nan added the backport 8.0 label Oct 13, 2024

kei-nan mentioned this pull request Oct 15, 2024

[8.0] MOD-7329: Determine if fields and documents are expired before returning them as valid results #5118

Merged

This was referenced Nov 21, 2024

[2.10] MOD-8097, MOD-8114 Fix memory counting (#5204) #5241

Merged

[2.8] MOD-8097, MOD-8114 Fix memory counting #5242

Merged

[8.0] MOD-8097, MOD-8114 Fix memory counting #5243

Merged

GuyAv46 removed the backport 2.10 label Jun 24, 2025

GuyAv46 mentioned this pull request Jun 29, 2025

Fix test flakiness - [MOD-9836] #6387

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MOD-7329: Determine if fields and documents are expired before returning them as valid results#4858

MOD-7329: Determine if fields and documents are expired before returning them as valid results#4858
kei-nan merged 34 commits intomasterfrom
jk_hfe_search_solution

kei-nan commented Jul 11, 2024

Uh oh!

codecov bot commented Jul 16, 2024 •

edited

Loading

Uh oh!

Uh oh!

kei-nan commented Sep 29, 2024

Uh oh!

github-actions bot commented Sep 29, 2024

Uh oh!

kei-nan commented Oct 13, 2024

Uh oh!

redisearch-backport-pull-request bot commented Oct 13, 2024

Uh oh!

redisearch-backport-pull-request bot commented Oct 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

kei-nan commented Jul 11, 2024

Uh oh!

codecov bot commented Jul 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

kei-nan commented Sep 29, 2024

Uh oh!

github-actions bot commented Sep 29, 2024

Uh oh!

kei-nan commented Oct 13, 2024

Uh oh!

redisearch-backport-pull-request bot commented Oct 13, 2024

Uh oh!

redisearch-backport-pull-request bot commented Oct 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jul 16, 2024 •

edited

Loading