Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

conf/workers-shared-extra.yaml.j2 is applied to monolith Complement runs #13590

Open
MadLittleMods opened this issue Aug 23, 2022 · 3 comments
Open
Labels
A-Testing Issues related to testing in complement, synapse, etc O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@MadLittleMods
Copy link
Contributor

MadLittleMods commented Aug 23, 2022

docker/complement/conf/workers-shared-extra.yaml.j2 seems to apply even when SYNAPSE_COMPLEMENT_USE_WORKERS=true isn't set. I assume they aren't supposed to stack like that for the monolith?

Noticed because I saw the reject_limit as 99999 when investigating #13541 (comment). Disabling the rate limit could be as expected for the monolith but the way it's inherited from workers seems weird.

This is the command I was running for reference but it probably works in the simplified case as well,

TEST_ONLY_IGNORE_POETRY_LOCKFILE=1 TEST_ONLY_SKIP_DEP_HASH_VERIFICATION=1 COMPLEMENT_DEBUG=1 COMPLEMENT_KEEP_BLUEPRINTS="fed.perf_many_messages.hs1" COMPLEMENT_ALWAYS_PRINT_SERVER_LOGS=1 COMPLEMENT_DIR=../complement ./scripts-dev/complement.sh -run TestMessagesOverFederation -p 1

Dev notes

Related PRs:

@DMRobertson DMRobertson added the T-Other Questions, user support, anything else. label Aug 23, 2022
@DMRobertson
Copy link
Contributor

Don't have much familiarity here. @reivilibre might be able to advise?

@H-Shay H-Shay added O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. and removed T-Other Questions, user support, anything else. labels Aug 24, 2022
@H-Shay
Copy link
Contributor

H-Shay commented Aug 24, 2022

Marking this as occasional as it doesn't occur on all runs, and tolerable as it seems that there is a workaround.

@richvdh
Copy link
Member

richvdh commented Aug 25, 2022

I assume they aren't supposed to stack like that for the monolith?

No, I think this is expected behaviour. As the first two lines in the file say:

This file extends the default 'shared' configuration file (from the 'synapse-workers'
docker image) with Complement-specific tweak.

I think it's just ended up with a bit of a misleading name, so the solution here is just to rename it?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Testing Issues related to testing in complement, synapse, etc O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

No branches or pull requests

4 participants