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

Fix exception in background metrics collection #3996

Merged
merged 2 commits into from Oct 3, 2018

Conversation

Projects
None yet
2 participants
@erikjohnston
Member

erikjohnston commented Oct 3, 2018

We attempted to iterate through a list on a separate thread without
doing the necessary copying.

erikjohnston added some commits Oct 3, 2018

Fix exception in background metrics collection
We attempted to iterate through a list on a separate thread without
doing the necessary copying.

@erikjohnston erikjohnston requested a review from matrix-org/synapse-core Oct 3, 2018

@richvdh

can you give an example of the exception you're fixing?

_background_processes_copy = dict(_background_processes)
_background_processes_copy = {
k: list(v)
for k, v in six.iteritems(_background_processes)

This comment has been minimized.

@richvdh

richvdh Oct 3, 2018

Member

what is to say that this will be thread-safe when the loop below was not?

This comment has been minimized.

@erikjohnston

erikjohnston Oct 3, 2018

Member

We're copying the dict and lists from inside a lock?

This comment has been minimized.

@richvdh

richvdh Oct 3, 2018

Member

ffs. I should probably learn to read.

This comment has been minimized.

@erikjohnston

erikjohnston Oct 3, 2018

Member

Glad I'm not the only one 😀

@erikjohnston

This comment has been minimized.

Member

erikjohnston commented Oct 3, 2018

2018-10-03 11:43:23,185 - twisted - 243 - ERROR -  - Traceback (most recent call last):
2018-10-03 11:43:23,186 - twisted - 243 - ERROR -  -   File "/usr/lib/python2.7/SocketServer.py", line 596, in process_request_thread
2018-10-03 11:43:23,186 - twisted - 243 - ERROR -  -     self.finish_request(request, client_address)
2018-10-03 11:43:23,186 - twisted - 243 - ERROR -  -   File "/usr/lib/python2.7/SocketServer.py", line 331, in finish_request
2018-10-03 11:43:23,186 - twisted - 243 - ERROR -  -     self.RequestHandlerClass(request, client_address, self)
2018-10-03 11:43:23,186 - twisted - 243 - ERROR -  -   File "/usr/lib/python2.7/SocketServer.py", line 652, in __init__
2018-10-03 11:43:23,186 - twisted - 243 - ERROR -  -     self.handle()
2018-10-03 11:43:23,186 - twisted - 243 - ERROR -  -   File "/usr/lib/python2.7/BaseHTTPServer.py", line 340, in handle
2018-10-03 11:43:23,186 - twisted - 243 - ERROR -  -     self.handle_one_request()
2018-10-03 11:43:23,186 - twisted - 243 - ERROR -  -   File "/usr/lib/python2.7/BaseHTTPServer.py", line 328, in handle_one_request
2018-10-03 11:43:23,186 - twisted - 243 - ERROR -  -     method()
2018-10-03 11:43:23,187 - twisted - 243 - ERROR -  -   File "/home/matrix/.synapse/local/lib/python2.7/site-packages/prometheus_client/exposition.py", line 95, in do_GET
2018-10-03 11:43:23,187 - twisted - 243 - ERROR -  -     output = generate_latest(registry)
2018-10-03 11:43:23,187 - twisted - 243 - ERROR -  -   File "/home/matrix/.synapse/local/lib/python2.7/site-packages/prometheus_client/exposition.py", line 69, in generate_latest
2018-10-03 11:43:23,187 - twisted - 243 - ERROR -  -     for metric in registry.collect():
2018-10-03 11:43:23,187 - twisted - 243 - ERROR -  -   File "synapse/metrics/__init__.py", line 46, in collect
2018-10-03 11:43:23,187 - twisted - 243 - ERROR -  -   File "/home/matrix/.synapse/local/lib/python2.7/site-packages/prometheus_client/core.py", line 102, in collect
2018-10-03 11:43:23,188 - twisted - 243 - ERROR -  -     for metric in collector.collect():
2018-10-03 11:43:23,188 - twisted - 243 - ERROR -  -   File "synapse/metrics/background_process_metrics.py", line 112, in collect
2018-10-03 11:43:23,188 - twisted - 243 - ERROR -  - RuntimeError: Set changed size during iteration
@richvdh

richvdh approved these changes Oct 3, 2018

lgtm

@erikjohnston erikjohnston merged commit 81e2813 into develop Oct 3, 2018

6 checks passed

ci/circleci: sytestpy2merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy2postgresmerged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3postgresmerged Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

michaelkaye added a commit that referenced this pull request Oct 22, 2018

Merge tag 'v0.33.7' into dinsic
**Warning**: This release removes the example email notification templates from
`res/templates` (they are now internal to the python package). This should only
affect you if you (a) deploy your Synapse instance from a git checkout or a
github snapshot URL, and (b) have email notifications enabled.

If you have email notifications enabled, you should ensure that
`email.template_dir` is either configured to point at a directory where you
have installed customised templates, or leave it unset to use the default
templates.

The configuration parser will try to detect the situation where
`email.template_dir` is incorrectly set to `res/templates` and do the right
thing, but will warn about this.

Features
--------

- Ship the example email templates as part of the package ([\#4052](#4052))
- Add support for end-to-end key backup (MSC1687) ([\#4019](#4019))

Bugfixes
--------

- Fix bug which made get_missing_events return too few events ([\#4045](#4045))
- Fix bug in event persistence logic which caused 'NoneType is not iterable' ([\#3995](#3995))
- Fix exception in background metrics collection ([\#3996](#3996))
- Fix exception handling in fetching remote profiles ([\#3997](#3997))
- Fix handling of rejected threepid invites ([\#3999](#3999))
- Workers now start on Python 3. ([\#4027](#4027))
- Synapse now starts on Python 3.7. ([\#4033](#4033))

Internal Changes
----------------

- Log exceptions in looping calls ([\#4008](#4008))
- Optimisation for serving federation requests ([\#4017](#4017))
- Add metric to count number of non-empty sync responses ([\#4022](#4022))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment