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

'Re-starting finished log context handle_presence_timeouts' #8597

Closed
richvdh opened this issue Oct 20, 2020 · 5 comments
Closed

'Re-starting finished log context handle_presence_timeouts' #8597

richvdh opened this issue Oct 20, 2020 · 5 comments
Labels
A-Logging Synapse's logs (structured or otherwise). Not metrics. z-bug (Deprecated Label)

Comments

@richvdh
Copy link
Member

richvdh commented Oct 20, 2020

When synapse is busy, we log the following warning:

2020-10-20 11:24:03,215 - synapse.logging.context - 70 - WARNING - handle_presence_timeouts-1- Re-starting finished log context handle_presence_timeouts-1

It looks like the call to FederationSender.send_presence isn't setting up log contexts correctly.

@anoadragon453
Copy link
Member

As it's the only function in the codebase to use the @preserve_fn decorator I'm a little suspicious that that may be the cause. Upon investigating the decorator though I don't see anything inherently wrong with it:

def preserve_fn(f):
"""Function decorator which wraps the function with run_in_background"""
def g(*args, **kwargs):
return run_in_background(f, *args, **kwargs)
return g

@anoadragon453 anoadragon453 added z-bug (Deprecated Label) A-Logging Synapse's logs (structured or otherwise). Not metrics. labels Oct 20, 2020
@richvdh
Copy link
Member Author

richvdh commented Oct 20, 2020

yes, it's the use of @preserve_fn that is the problem. It means that we can still be doing work within that function after the handle_presence_timeouts context has finished.

(Apart from the warning, it's bad in that it means that such work isn't making it into the prometheus CPU/database metrics.)

@clokep
Copy link
Member

clokep commented Oct 20, 2020

There's a similar-sounding wrap_as_background_task which might do what is wanted? 🤷

@richvdh
Copy link
Member Author

richvdh commented Oct 20, 2020

probably. there is a question to be answered over whether this work should run in the background at all, or if the caller should be waiting for it to complete.

@richvdh
Copy link
Member Author

richvdh commented Jun 10, 2021

I think this was fixed by #9828.

@richvdh richvdh closed this as completed Jun 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Logging Synapse's logs (structured or otherwise). Not metrics. z-bug (Deprecated Label)
Projects
None yet
Development

No branches or pull requests

3 participants