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

Support reactor timing on more reactors. #16532

Merged
merged 7 commits into from
Nov 6, 2023
Merged

Conversation

clokep
Copy link
Member

@clokep clokep commented Oct 20, 2023

Supports timing reactor ticks on select and poll reactors, as well as epoll (which we previously supported). Also supports reactor timings when using the asyncio reactor (when used with an underlying select, poll, epoll, /dev/poll, or kqueue event loop).

@DMRobertson
Copy link
Contributor

This seems unhappy on Linux (GHA runners, anyway):

  File "/home/runner/work/synapse/synapse/synapse/metrics/__init__.py", line 47, in <module>
    import synapse.metrics._reactor_metrics  # noqa: F401
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/synapse/synapse/synapse/metrics/_reactor_metrics.py", line 122, in <module>
    wrapper = reactor._poller.poll = CallWrapper(reactor._poller.poll)
              ^^^^^^^^^^^^^^^^^^^^
AttributeError: 'select.epoll' object attribute 'poll' is read-only

@clokep
Copy link
Member Author

clokep commented Oct 23, 2023

This seems unhappy on Linux (GHA runners, anyway):

I was unable to fully test on Linux locally, was waiting for builds to run.

@DMRobertson
Copy link
Contributor

Roger. Shout if you want a test minion to try anything out!

@clokep
Copy link
Member Author

clokep commented Oct 23, 2023

🤦 I fixed the epoll issue for asyncio, but not Twisted. Anyway...I think the current version will work?

@clokep clokep marked this pull request as ready for review October 23, 2023 13:11
@clokep clokep requested a review from a team as a code owner October 23, 2023 13:11
@erikjohnston
Copy link
Member

It seems to work happily on jki.re FWIW

@clokep
Copy link
Member Author

clokep commented Oct 23, 2023

It seems to work happily on jki.re FWIW

With asyncio or just in general?

@erikjohnston
Copy link
Member

Sorry, that is using asyncio

@clokep
Copy link
Member Author

clokep commented Nov 2, 2023

I think this is a bit safer now and gives decent warnings like:

Skipping configuring ReactorLastSeenMetric: unexpected asyncio loop selector: <selectors.KqueueSelector object at 0x10de4ea50> via <_UnixSelectorEventLoop running=False closed=False debug=False>

or

Configuring ReactorLastSeenMetric failed: AttributeError("'select.kqueue' object attribute 'control' is read-only")

(the second one was by me introducing a bug intro the code)

@clokep clokep merged commit cc4fe68 into develop Nov 6, 2023
41 checks passed
@clokep clokep deleted the clokep/reactor-times branch November 6, 2023 13:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants