Skip to content

chore: Add capability to crash a worker if triggered from main process#274

Open
jason-famedly wants to merge 3 commits into
masterfrom
jason/desict-redis
Open

chore: Add capability to crash a worker if triggered from main process#274
jason-famedly wants to merge 3 commits into
masterfrom
jason/desict-redis

Conversation

@jason-famedly

Copy link
Copy Markdown
Member

SYN-53

It has been found that sometimes a worker is missed when restarting containers. This adds a new top-level setting force_crash_workers_after_main_restart(bool) to enable sending a SIGTERM signal to restart worker when triggered by redis.

An existing(although currently unused) redis "command" has been reworked for this purpose to send not only the server's instance name but also the name of the worker sending the command and when that process started. It should be received by any workers currently connected. Any worker receiving this command shall check their own start up time to compare against this received value and, if this should not be the main process, raise a SIGTERM. In theory, may not even really need to include the timestamp itself, as this command alone should only be broadcast on the start of the process.

This command needs to be sent before the redis replication process starts or it may interfere with the SUBSCRIBE and REPLICATE commands that follow since we are borrowing that connection

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.42%. Comparing base (0b2b5a4) to head (3e07b87).

Files with missing lines Patch % Lines
synapse/replication/tcp/protocol.py 47.05% 8 Missing and 1 partial ⚠️
synapse/replication/tcp/handler.py 80.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #274      +/-   ##
==========================================
- Coverage   80.43%   80.42%   -0.02%     
==========================================
  Files         502      502              
  Lines       72214    72252      +38     
  Branches    10888    10892       +4     
==========================================
+ Hits        58088    58109      +21     
- Misses      10861    10875      +14     
- Partials     3265     3268       +3     
Files with missing lines Coverage Δ
synapse/config/server.py 68.57% <100.00%> (+0.15%) ⬆️
synapse/replication/tcp/commands.py 95.00% <100.00%> (+0.29%) ⬆️
synapse/replication/tcp/redis.py 81.36% <100.00%> (+0.23%) ⬆️
synapse/replication/tcp/handler.py 88.47% <80.00%> (-0.28%) ⬇️
synapse/replication/tcp/protocol.py 71.86% <47.05%> (-1.23%) ⬇️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b2b5a4...3e07b87. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jason-famedly jason-famedly marked this pull request as ready for review July 13, 2026 12:03
@jason-famedly jason-famedly requested a review from a team as a code owner July 13, 2026 12:03
@jason-famedly

Copy link
Copy Markdown
Member Author

Only test that failed on initial run was cargo-test, and that is a known failure that was determined to be false and has since been fixed upstream. It is unrelated to this change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant