Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow running Synapse with the asyncioreactor enabled #1301

Merged
merged 1 commit into from
Oct 14, 2022

Conversation

sandhose
Copy link
Member

@sandhose sandhose commented Oct 6, 2022

This adds a flag --asyncio-reactor, which sets the SYNAPSE_ASYNCIO_REACTOR environment variable to 1.
It also adds a ASYNCIO_REACTOR environment variable to the Docker image to set this flag.

Note that it currently also sets SYNAPSE_ASYNCIO_REACTOR to 0 when the flag is not set, which currently enables the asyncioreactor anyway, so this will require some changes in Synapse first: matrix-org/synapse#14092

Related: matrix-org/synapse#14091

Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

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

It seems a bit odd to accept an env var ASYNCIO_REACTOR, convert that to a command-line flag --asyncio-reactor and then use the flag to set a different env var SYNAPSE_ASYNC_IO_REACTOR. Does SYNAPSE_ASYNC_IO_REACTOR=1 synapse_sytest.sh pass through the environment var to the synapse processes?

Well, I took a quick look at the perl and I have no idea if the environment will pass through when we spawn the process.

my $proc = IO::Async::Process->new(
%process_params,
stdout => { on_read => $on_output },
stderr => { on_read => $on_output },
on_finish => $self->_capture_weakself( '_on_finish' ),
);

@DMRobertson
Copy link
Contributor

In the interests of getting asyncio tested sooner rather than later, let's roll with this. Note that matrix-org/synapse#14092 has already landed.

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.

2 participants