Skip to content

Fix flakiness in a test - [MOD-8611]#5541

Merged
GuyAv46 merged 2 commits intomasterfrom
guyav-fix_flakiness
Jan 23, 2025
Merged

Fix flakiness in a test - [MOD-8611]#5541
GuyAv46 merged 2 commits intomasterfrom
guyav-fix_flakiness

Conversation

@GuyAv46
Copy link
Collaborator

@GuyAv46 GuyAv46 commented Jan 23, 2025

Describe the changes in the pull request

Fix flakiness in the test test_mod_8142, caused due to timing with a redundant background index scan

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

@GuyAv46 GuyAv46 enabled auto-merge January 23, 2025 07:52
@GuyAv46 GuyAv46 requested a review from kei-nan January 23, 2025 08:12
@codecov
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.20%. Comparing base (5a6f77a) to head (14c5911).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5541   +/-   ##
=======================================
  Coverage   87.20%   87.20%           
=======================================
  Files         196      196           
  Lines       35230    35230           
=======================================
+ Hits        30722    30724    +2     
+ Misses       4508     4506    -2     

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

@GuyAv46 GuyAv46 added this pull request to the merge queue Jan 23, 2025
@GuyAv46 GuyAv46 changed the title Fix flakiness in a test Fix flakiness in a test - [MOD-8611] Jan 23, 2025

# Verify that the vector search doesn't affect the scoring or result set
env.expect('FT.ALTER', 'idx', 'SCHEMA', 'ADD', 'v', 'VECTOR', 'FLAT', '6', 'TYPE', 'FLOAT32', 'DIM', '2', 'DISTANCE_METRIC', 'L2').ok()
env.expect('FT.ALTER', 'idx', 'SKIPINITIALSCAN', 'SCHEMA', 'ADD', 'v', 'VECTOR', 'FLAT', '6', 'TYPE', 'FLOAT32', 'DIM', '2', 'DISTANCE_METRIC', 'L2').ok()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this doesn't index anything, how does it change the score?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vector field is missing from the hash at the beginning, and we add them in the following HSET commands which index them in place

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That I see. I don't see how the SKIPINITIALSCAN should change the score.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(After discussion) Relates to another bug

Merged via the queue into master with commit d9bef07 Jan 23, 2025
10 checks passed
@GuyAv46 GuyAv46 deleted the guyav-fix_flakiness branch January 23, 2025 10:22
redisearch-backport-pull-request bot pushed a commit that referenced this pull request Jan 23, 2025
* fix flakiness

* revert whitespace change

(cherry picked from commit d9bef07)
@redisearch-backport-pull-request
Copy link
Contributor

Successfully created backport PR for 2.8:

redisearch-backport-pull-request bot pushed a commit that referenced this pull request Jan 23, 2025
* fix flakiness

* revert whitespace change

(cherry picked from commit d9bef07)
@redisearch-backport-pull-request
Copy link
Contributor

Successfully created backport PR for 2.6:

@redisearch-backport-pull-request
Copy link
Contributor

Successfully created backport PR for 2.10:

redisearch-backport-pull-request bot pushed a commit that referenced this pull request Jan 23, 2025
* fix flakiness

* revert whitespace change

(cherry picked from commit d9bef07)
@redisearch-backport-pull-request
Copy link
Contributor

Successfully created backport PR for 8.0:

redisearch-backport-pull-request bot pushed a commit that referenced this pull request Jan 23, 2025
* fix flakiness

* revert whitespace change

(cherry picked from commit d9bef07)
github-merge-queue bot pushed a commit that referenced this pull request Jan 23, 2025
Fix flakiness in a test (#5541)

* fix flakiness

* revert whitespace change

(cherry picked from commit d9bef07)

Co-authored-by: GuyAv46 <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Jan 23, 2025
Fix flakiness in a test (#5541)

* fix flakiness

* revert whitespace change

(cherry picked from commit d9bef07)

Co-authored-by: GuyAv46 <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Jan 23, 2025
Fix flakiness in a test (#5541)

* fix flakiness

* revert whitespace change

(cherry picked from commit d9bef07)

Co-authored-by: GuyAv46 <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Jan 23, 2025
Fix flakiness in a test (#5541)

* fix flakiness

* revert whitespace change

(cherry picked from commit d9bef07)

Co-authored-by: GuyAv46 <[email protected]>
dor-forer pushed a commit that referenced this pull request Jan 26, 2025
* fix flakiness

* revert whitespace change
github-merge-queue bot pushed a commit that referenced this pull request Jan 30, 2025
* Adding numeric check

* changes

* change to each one

* MOD-6786 Fix search on larger then 128 terms (#5524)

* Move length slicing to NOMODIFY if

* add py test

* fix slicing

* fix test

* fix text skip cluster

* Adding comments

* Update test_issues - skip cluster

* MOD-8561: Fix Inverted Index SeekTo Edge Case (#5528)

* * initial commit

* * simplify the fix

* * revert to old code to solve edge case

* Load config params for Redis 8.0-m03 (#5538)

* Load config params for Redis v7.9.226

* Add step to get latest unreleased redis tag

* Remove commented-out step `Get Latest Release Tag with Prefix`

* Revert: task-get-latest-tag.yml

* MOD-8601: Fix error message for LOAD (#5531)

* Enhance error message for LOAD

* Fix error message

* Address review

* Fix flakiness in a test (#5541)

* fix flakiness

* revert whitespace change

* Fix Max Frequency Misscalculation - [MOD-8158] (#5553)

* fix unrelated test

* add a failing test

* fix issue

* revert whitespace change from test_vecsim.py

* revert whitespace change in test_issues.py

* Fix APPLY/FILTER parser - [MOD-7804] (#5520)

* fix order of operations

* minor improvements to the lexer

* improve functions parsing

* optimize "NOT" logic and perform arithmetic operations immediately

* fix flow tests

* fix grammar optimization

* changed function API

* added a test

* minor fix

* fix precedence

* minor improvement

* reorder rule and fix leak

* another fix

* added test

* more tests for a better coverage

* improved test

* fix assertion

* review fixes

* address code review

* added comments

* remove unncessary

* Added tests for legacy filter empty

* Adding numeric check

* changes

* change to each one

* remove unncessary

* Added tests for legacy filter empty

* * Change the order of params
* Add support in GEOFILTER

* Forgot one file

* * Changed to AC_GetString with no advance
* Added comment
* change the string check

* PR changes

* Changes

* push the test

* change style

---------

Co-authored-by: lerman25 <[email protected]>
Co-authored-by: kei-nan <[email protected]>
Co-authored-by: nafraf <[email protected]>
Co-authored-by: Raz Monsonego <[email protected]>
Co-authored-by: GuyAv46 <[email protected]>
redisearch-backport-pull-request bot pushed a commit that referenced this pull request Jan 30, 2025
* Adding numeric check

* changes

* change to each one

* MOD-6786 Fix search on larger then 128 terms (#5524)

* Move length slicing to NOMODIFY if

* add py test

* fix slicing

* fix test

* fix text skip cluster

* Adding comments

* Update test_issues - skip cluster

* MOD-8561: Fix Inverted Index SeekTo Edge Case (#5528)

* * initial commit

* * simplify the fix

* * revert to old code to solve edge case

* Load config params for Redis 8.0-m03 (#5538)

* Load config params for Redis v7.9.226

* Add step to get latest unreleased redis tag

* Remove commented-out step `Get Latest Release Tag with Prefix`

* Revert: task-get-latest-tag.yml

* MOD-8601: Fix error message for LOAD (#5531)

* Enhance error message for LOAD

* Fix error message

* Address review

* Fix flakiness in a test (#5541)

* fix flakiness

* revert whitespace change

* Fix Max Frequency Misscalculation - [MOD-8158] (#5553)

* fix unrelated test

* add a failing test

* fix issue

* revert whitespace change from test_vecsim.py

* revert whitespace change in test_issues.py

* Fix APPLY/FILTER parser - [MOD-7804] (#5520)

* fix order of operations

* minor improvements to the lexer

* improve functions parsing

* optimize "NOT" logic and perform arithmetic operations immediately

* fix flow tests

* fix grammar optimization

* changed function API

* added a test

* minor fix

* fix precedence

* minor improvement

* reorder rule and fix leak

* another fix

* added test

* more tests for a better coverage

* improved test

* fix assertion

* review fixes

* address code review

* added comments

* remove unncessary

* Added tests for legacy filter empty

* Adding numeric check

* changes

* change to each one

* remove unncessary

* Added tests for legacy filter empty

* * Change the order of params
* Add support in GEOFILTER

* Forgot one file

* * Changed to AC_GetString with no advance
* Added comment
* change the string check

* PR changes

* Changes

* push the test

* change style

---------

Co-authored-by: lerman25 <[email protected]>
Co-authored-by: kei-nan <[email protected]>
Co-authored-by: nafraf <[email protected]>
Co-authored-by: Raz Monsonego <[email protected]>
Co-authored-by: GuyAv46 <[email protected]>
(cherry picked from commit e4d8fa0)
github-merge-queue bot pushed a commit that referenced this pull request Feb 4, 2025
* Fix Empty Numeric Value - [MOD-7244] (#5566)

* Adding numeric check

* changes

* change to each one

* MOD-6786 Fix search on larger then 128 terms (#5524)

* Move length slicing to NOMODIFY if

* add py test

* fix slicing

* fix test

* fix text skip cluster

* Adding comments

* Update test_issues - skip cluster

* MOD-8561: Fix Inverted Index SeekTo Edge Case (#5528)

* * initial commit

* * simplify the fix

* * revert to old code to solve edge case

* Load config params for Redis 8.0-m03 (#5538)

* Load config params for Redis v7.9.226

* Add step to get latest unreleased redis tag

* Remove commented-out step `Get Latest Release Tag with Prefix`

* Revert: task-get-latest-tag.yml

* MOD-8601: Fix error message for LOAD (#5531)

* Enhance error message for LOAD

* Fix error message

* Address review

* Fix flakiness in a test (#5541)

* fix flakiness

* revert whitespace change

* Fix Max Frequency Misscalculation - [MOD-8158] (#5553)

* fix unrelated test

* add a failing test

* fix issue

* revert whitespace change from test_vecsim.py

* revert whitespace change in test_issues.py

* Fix APPLY/FILTER parser - [MOD-7804] (#5520)

* fix order of operations

* minor improvements to the lexer

* improve functions parsing

* optimize "NOT" logic and perform arithmetic operations immediately

* fix flow tests

* fix grammar optimization

* changed function API

* added a test

* minor fix

* fix precedence

* minor improvement

* reorder rule and fix leak

* another fix

* added test

* more tests for a better coverage

* improved test

* fix assertion

* review fixes

* address code review

* added comments

* remove unncessary

* Added tests for legacy filter empty

* Adding numeric check

* changes

* change to each one

* remove unncessary

* Added tests for legacy filter empty

* * Change the order of params
* Add support in GEOFILTER

* Forgot one file

* * Changed to AC_GetString with no advance
* Added comment
* change the string check

* PR changes

* Changes

* push the test

* change style

---------

Co-authored-by: lerman25 <[email protected]>
Co-authored-by: kei-nan <[email protected]>
Co-authored-by: nafraf <[email protected]>
Co-authored-by: Raz Monsonego <[email protected]>
Co-authored-by: GuyAv46 <[email protected]>
(cherry picked from commit e4d8fa0)

* Change python to fit python3.7

---------

Co-authored-by: dor-forer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants