Skip to content

[2.10] CP TIMEOUT related bugs & Query Debug Mechanism #5734

Merged
meiravgri merged 8 commits into2.10from
backport-5554-to-2.10
Mar 17, 2025
Merged

[2.10] CP TIMEOUT related bugs & Query Debug Mechanism #5734
meiravgri merged 8 commits into2.10from
backport-5554-to-2.10

Conversation

@meiravgri
Copy link
Collaborator

@meiravgri meiravgri commented Mar 9, 2025

backport #5554 to 2.10

fixing:
MOD-8483, MOD-8482, MOD-8515, MOD-8606, MOD-8794

debug query mechanism mod:
MOD-8605

Background

In master, src/module.c combines both coordinator and standalone logic, allowing to implementation of the debug mechanism for both modes in this file. However, in version 2.10, we still maintain separate files for each module:

  • Coordinator: coord/module.c
  • Standalone: src/module.c

Similarly, debug_commands.c was merged into a single file in master.

Due to these structural differences, this cherry-pick had to be done manually to properly extract coordinator-related code and place it in the correct files.

Changes in Coordinator & Standalone Implementations

Component Master 2.10
expose Dist<Aggregate/Search>Command src/module.h coord/src/coord_module.h
handle _FT.DEBUG arg in cluster mode src/module.c coord/src/coord_module.c
debug_commands.c registrater FT.DEBUG FT.SEARCH/AGGREGATE && FT.DEBUG _FT.SEARCH/_FT.AGGREGATE, Registers FT.DEBUG FT.SEARCH/AGGREGATE for standalone mode or _FT.SEARCH/_FT.AGGREGATE when built with COORD=1
coord/src/debug_command.c N/A Registers FT.DEBUG FT.SEARCH/AGGREGATE
RPTimeoutAfterCount_SimulateTimeout parent->sctx->time.timeout parent->sctx->timeout
  • GetNumShards_UnSafe was not cherry-picked, as it is not in use. Cluster mode is checked testing if RS_COORDINATOR macro is defined.

meiravgri and others added 4 commits March 9, 2025 13:09
module.h merged with 0 attention

debug_commands.c: Removed ACL verification (debugCommandsEnabled) from the debug commands
decalre DistAggregateCommand in coord_module.h

expose FT.DEBUG FT.AGGREGATE/SEARCH in coord debug commands

merge coordinator code that was riginially (in master) in module, to coord/module
…ommandHandler

skip wrokers if MT build is disabled
@codecov
Copy link

codecov bot commented Mar 16, 2025

Codecov Report

Attention: Patch coverage is 98.25581% with 6 lines in your changes missing coverage. Please review.

Project coverage is 87.74%. Comparing base (3d3e505) to head (95a1b58).
Report is 6 commits behind head on 2.10.

Files with missing lines Patch % Lines
coord/src/dist_aggregate.c 95.83% 3 Missing ⚠️
coord/src/module.c 98.63% 1 Missing ⚠️
src/aggregate/aggregate_debug.c 98.36% 1 Missing ⚠️
src/aggregate/aggregate_exec.c 98.24% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             2.10    #5734      +/-   ##
==========================================
+ Coverage   87.66%   87.74%   +0.07%     
==========================================
  Files         196      197       +1     
  Lines       34247    34509     +262     
==========================================
+ Hits        30022    30279     +257     
- Misses       4225     4230       +5     

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@meiravgri meiravgri changed the title WIP!! [2.10] CP TIMEOUT related bugs & Query Debug Mechanism Mar 16, 2025
@meiravgri meiravgri requested review from GuyAv46 and raz-mon March 16, 2025 16:13
Copy link
Collaborator

@raz-mon raz-mon left a comment

Choose a reason for hiding this comment

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

LGTM 👍
One include maybe can be removed

@meiravgri meiravgri added this pull request to the merge queue Mar 17, 2025
Merged via the queue into 2.10 with commit c27cf7f Mar 17, 2025
10 checks passed
@meiravgri meiravgri deleted the backport-5554-to-2.10 branch March 17, 2025 17:49
@meiravgri
Copy link
Collaborator Author

/backport

@redisearch-backport-pull-request
Copy link
Contributor

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-5734-to-2.8 origin/2.8
cd .worktree/backport-5734-to-2.8
git switch --create backport-5734-to-2.8
git cherry-pick -x c27cf7f3ec3b6c722a44643f0a2ced064e959458

@redisearch-backport-pull-request
Copy link
Contributor

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

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

git fetch origin 2.6
git worktree add -d .worktree/backport-5734-to-2.6 origin/2.6
cd .worktree/backport-5734-to-2.6
git switch --create backport-5734-to-2.6
git cherry-pick -x c27cf7f3ec3b6c722a44643f0a2ced064e959458

@alonre24 alonre24 restored the backport-5554-to-2.10 branch March 20, 2025 06:35
@alonre24 alonre24 deleted the backport-5554-to-2.10 branch March 20, 2025 06:36
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