[MOD-8035] Add field index error for JSON field indexing failures by meiravgri · Pull Request #5150 · RediSearch/RediSearch

meiravgri · 2024-11-03T15:38:47Z

Introduce tracking field-level indexing error in JSON.

move dialects stats to global_stats

The counter is update at runtime, once the error occured. It reflects the total number of indexing errors in all living indices. The counter should be decreased whenever an index is dropped. introduce FieldSpec_AddError that adds an error to a specific field. it also updates the global field error counter. We expose the global counter value in INFO modules, along with each field type stats. the name of the fields count for each type was changed from "Field_type_name" to "Total"

wait for bg index after FT.ALTER to avoid flakyness

…ose_index_errors

codecov · 2024-11-03T16:01:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.55%. Comparing base (5884687) to head (35a1caa).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5150   +/-   ##
=======================================
  Coverage   86.55%   86.55%           
=======================================
  Files         193      193           
  Lines       34756    34757    +1     
=======================================
+ Hits        30083    30084    +1     
  Misses       4673     4673

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

DvirDukhan · 2024-11-10T13:31:59Z

src/global_stats.h

 /**
- * Exposing all the fields that > 0 to INFO command.
+ * Add or increase `toAdd` number of errors to the global index errors counter of field_type.
+ * `toAdd` can be negative to decrease the counter.


in which case?

DvirDukhan · 2024-11-10T13:32:20Z

src/global_stats.c

 }

+void FieldsGlobalStats_UpdateIndexError(FieldType field_type, int toAdd) {
+  FieldIndexErrorCounter[INDEXTYPE_TO_POS(field_type)] += toAdd;


adding int to size_t

we are doing it to allow negative toAdd

DvirDukhan · 2024-11-10T13:34:57Z

src/module-init/module-init.c

+    // indexing errors
+  RedisModule_InfoAddFieldDouble(ctx, "errors_indexing_failures", indexing_errors.indexing_failures);
+  // highest number of failures out of all specs
+  RedisModule_InfoAddFieldDouble(ctx, "errors_indexing_failures_max", indexing_errors.max_indexing_failures);


what is the real world purpose for this field? also consider rename to max_indexing_errors or something similar

DvirDukhan · 2024-11-10T13:44:03Z

tests/pytests/test_index_error.py

-
    env.assertEqual(error_dict, expected_error_dict)
+
+def test_multiple_index_failures_json(env):


skip(no_json=True)

DvirDukhan · 2024-11-10T13:47:42Z

tests/pytests/test_info_modules.py

+  return f'Tag={count}' \
+         f'{",Sortable=" + str(sortable_count) if sortable_count else ""}' \
+         f'{",NoIndex=" + str(no_index_count) if no_index_count else ""}' \
+         f'{",CaseSensitive=" + str(case_sensitive_count) if case_sensitive_count else ""}' \


I know it is not a part of this PR but do we have a tests that checks what happens when the values are 0?

yes
fields options like sortable etc, are only printed when set (you can see here that the string is empty)
IndexErrors is always printed and is 0 zero in some of the tests

…ing_error_in_json

redisearch-backport-pull-request · 2024-11-12T15:38:42Z

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

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

git fetch origin 2.8
git worktree add -d .worktree/backport-5150-to-2.8 origin/2.8
cd .worktree/backport-5150-to-2.8
git switch --create backport-5150-to-2.8
git cherry-pick -x f045e21b8356fe2e4e7e2c4f06d3bed1b5c645b2

) * move fieldsStats from RSGlobalConfig to RSGlobalStats move dialects stats to global_stats * Introduce a global counter for indexing failures for each field type. The counter is update at runtime, once the error occured. It reflects the total number of indexing errors in all living indices. The counter should be decreased whenever an index is dropped. introduce FieldSpec_AddError that adds an error to a specific field. it also updates the global field error counter. We expose the global counter value in INFO modules, along with each field type stats. the name of the fields count for each type was changed from "Field_type_name" to "Total" * fix test * skip test_redis_info_errors in cluster wait for bg index after FT.ALTER to avoid flakyness * revert change in INFO MODULES field count name * fix test format * add field indesing failure when encountered in json * skip json tests of not json (cherry picked from commit f045e21)

redisearch-backport-pull-request · 2024-11-12T15:38:46Z

Successfully created backport PR for 2.10:

[2.10] [MOD-8035] Add field index error for JSON field indexing failures #5202

) * move fieldsStats from RSGlobalConfig to RSGlobalStats move dialects stats to global_stats * Introduce a global counter for indexing failures for each field type. The counter is update at runtime, once the error occured. It reflects the total number of indexing errors in all living indices. The counter should be decreased whenever an index is dropped. introduce FieldSpec_AddError that adds an error to a specific field. it also updates the global field error counter. We expose the global counter value in INFO modules, along with each field type stats. the name of the fields count for each type was changed from "Field_type_name" to "Total" * fix test * skip test_redis_info_errors in cluster wait for bg index after FT.ALTER to avoid flakyness * revert change in INFO MODULES field count name * fix test format * add field indesing failure when encountered in json * skip json tests of not json (cherry picked from commit f045e21)

redisearch-backport-pull-request · 2024-11-12T15:38:50Z

Successfully created backport PR for 8.0:

[8.0] [MOD-8035] Add field index error for JSON field indexing failures #5203

) * move fieldsStats from RSGlobalConfig to RSGlobalStats move dialects stats to global_stats * Introduce a global counter for indexing failures for each field type. The counter is update at runtime, once the error occured. It reflects the total number of indexing errors in all living indices. The counter should be decreased whenever an index is dropped. introduce FieldSpec_AddError that adds an error to a specific field. it also updates the global field error counter. We expose the global counter value in INFO modules, along with each field type stats. the name of the fields count for each type was changed from "Field_type_name" to "Total" * fix test * skip test_redis_info_errors in cluster wait for bg index after FT.ALTER to avoid flakyness * revert change in INFO MODULES field count name * fix test format * add field indesing failure when encountered in json * skip json tests of not json (cherry picked from commit f045e21)

…es (#5203) [MOD-8035] Add field index error for JSON field indexing failures (#5150) * move fieldsStats from RSGlobalConfig to RSGlobalStats move dialects stats to global_stats * Introduce a global counter for indexing failures for each field type. The counter is update at runtime, once the error occured. It reflects the total number of indexing errors in all living indices. The counter should be decreased whenever an index is dropped. introduce FieldSpec_AddError that adds an error to a specific field. it also updates the global field error counter. We expose the global counter value in INFO modules, along with each field type stats. the name of the fields count for each type was changed from "Field_type_name" to "Total" * fix test * skip test_redis_info_errors in cluster wait for bg index after FT.ALTER to avoid flakyness * revert change in INFO MODULES field count name * fix test format * add field indesing failure when encountered in json * skip json tests of not json (cherry picked from commit f045e21) Co-authored-by: meiravgri <[email protected]>

…res (#5202) * [MOD-8035] Add field index error for JSON field indexing failures (#5150) * move fieldsStats from RSGlobalConfig to RSGlobalStats move dialects stats to global_stats * Introduce a global counter for indexing failures for each field type. The counter is update at runtime, once the error occured. It reflects the total number of indexing errors in all living indices. The counter should be decreased whenever an index is dropped. introduce FieldSpec_AddError that adds an error to a specific field. it also updates the global field error counter. We expose the global counter value in INFO modules, along with each field type stats. the name of the fields count for each type was changed from "Field_type_name" to "Total" * fix test * skip test_redis_info_errors in cluster wait for bg index after FT.ALTER to avoid flakyness * revert change in INFO MODULES field count name * fix test format * add field indesing failure when encountered in json * skip json tests of not json (cherry picked from commit f045e21) * add inmport common.skip to test_index_error * remove skip json (not respected and causes the entire test to be skipped) --------- Co-authored-by: meiravgri <[email protected]> Co-authored-by: meiravgri <[email protected]>

…es (#5205) * [MOD-8035] Add field index error for JSON field indexing failures (#5150) * move fieldsStats from RSGlobalConfig to RSGlobalStats move dialects stats to global_stats * Introduce a global counter for indexing failures for each field type. The counter is update at runtime, once the error occured. It reflects the total number of indexing errors in all living indices. The counter should be decreased whenever an index is dropped. introduce FieldSpec_AddError that adds an error to a specific field. it also updates the global field error counter. We expose the global counter value in INFO modules, along with each field type stats. the name of the fields count for each type was changed from "Field_type_name" to "Total" * fix test * skip test_redis_info_errors in cluster wait for bg index after FT.ALTER to avoid flakyness * revert change in INFO MODULES field count name * fix test format * add field indesing failure when encountered in json * skip json tests of not json (cherry picked from commit f045e21) * fix backporting (using IndexError_AddError instead of FieldSpec_AddError that was not backported) * remove skip json * convert num_docs to int

meiravgri added 9 commits October 29, 2024 09:56

move fieldsStats from RSGlobalConfig to RSGlobalStats

6f617fa

move dialects stats to global_stats

Merge branch 'master' into meiravg_mod7955_expose_index_errors

97249cf

fix test

85bc862

skip test_redis_info_errors in cluster

1c47202

wait for bg index after FT.ALTER to avoid flakyness

revert change in INFO MODULES field count name

c147626

Merge remote-tracking branch 'origin/master' into meiravg_mod7955_exp…

f54333b

…ose_index_errors

fix test format

793df00

add field indesing failure when encountered in json

e2556fd

meiravgri changed the title ~~Add field index error for JSON field indexing failures~~ [MOD-8035 ] Add field index error for JSON field indexing failures Nov 3, 2024

meiravgri changed the title ~~[MOD-8035 ] Add field index error for JSON field indexing failures~~ [MOD-8035] Add field index error for JSON field indexing failures Nov 3, 2024

meiravgri requested review from DvirDukhan and GuyAv46 November 3, 2024 15:51

Base automatically changed from meiravg_mod7955_expose_index_errors to master November 4, 2024 17:09

DvirDukhan reviewed Nov 10, 2024

View reviewed changes

meiravgri added 2 commits November 12, 2024 11:25

Merge remote-tracking branch 'origin/master' into meiravg_field_index…

aea4240

…ing_error_in_json

skip json tests of not json

35a1caa

meiravgri requested a review from DvirDukhan November 12, 2024 12:35

DvirDukhan approved these changes Nov 12, 2024

View reviewed changes

meiravgri added backport 2.8 backport 2.10 backport 8.0 labels Nov 12, 2024

meiravgri added this pull request to the merge queue Nov 12, 2024

Merged via the queue into master with commit f045e21 Nov 12, 2024

meiravgri deleted the meiravg_field_indexing_error_in_json branch November 12, 2024 15:38

redisearch-backport-pull-request bot mentioned this pull request Nov 12, 2024

[2.10] [MOD-8035] Add field index error for JSON field indexing failures #5202

Merged

redisearch-backport-pull-request bot mentioned this pull request Nov 12, 2024

[8.0] [MOD-8035] Add field index error for JSON field indexing failures #5203

Merged

meiravgri mentioned this pull request Nov 12, 2024

[2.8] [MOD-8035] Add field index error for JSON field indexing failures #5205

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MOD-8035] Add field index error for JSON field indexing failures#5150

[MOD-8035] Add field index error for JSON field indexing failures#5150
meiravgri merged 11 commits intomasterfrom
meiravg_field_indexing_error_in_json

meiravgri commented Nov 3, 2024 •

edited

Loading

Uh oh!

codecov bot commented Nov 3, 2024 •

edited

Loading

Uh oh!

DvirDukhan Nov 10, 2024

Uh oh!

DvirDukhan Nov 10, 2024

Uh oh!

meiravgri Nov 12, 2024

Uh oh!

DvirDukhan Nov 10, 2024

Uh oh!

DvirDukhan Nov 10, 2024

Uh oh!

DvirDukhan Nov 10, 2024

Uh oh!

meiravgri Nov 12, 2024

Uh oh!

redisearch-backport-pull-request bot commented Nov 12, 2024

Uh oh!

redisearch-backport-pull-request bot commented Nov 12, 2024

Uh oh!

redisearch-backport-pull-request bot commented Nov 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		env.assertEqual(error_dict, expected_error_dict)

		def test_multiple_index_failures_json(env):

Conversation

meiravgri commented Nov 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

DvirDukhan Nov 10, 2024

Choose a reason for hiding this comment

Uh oh!

DvirDukhan Nov 10, 2024

Choose a reason for hiding this comment

Uh oh!

meiravgri Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

DvirDukhan Nov 10, 2024

Choose a reason for hiding this comment

Uh oh!

DvirDukhan Nov 10, 2024

Choose a reason for hiding this comment

Uh oh!

DvirDukhan Nov 10, 2024

Choose a reason for hiding this comment

Uh oh!

meiravgri Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

redisearch-backport-pull-request bot commented Nov 12, 2024

Uh oh!

redisearch-backport-pull-request bot commented Nov 12, 2024

Uh oh!

redisearch-backport-pull-request bot commented Nov 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

meiravgri commented Nov 3, 2024 •

edited

Loading

codecov bot commented Nov 3, 2024 •

edited

Loading