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

Add a 60s timeout to filtered room directory queries #4461

Merged
merged 6 commits into from Jan 24, 2019

Conversation

Projects
None yet
3 participants
@anoadragon453
Copy link
Member

anoadragon453 commented Jan 24, 2019

Protects against longstanding queries piling up in the background and blocking new ones.

anoadragon453 added some commits Jan 24, 2019

@anoadragon453 anoadragon453 requested a review from matrix-org/synapse-core Jan 24, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 24, 2019

Codecov Report

Merging #4461 into develop will increase coverage by 0.01%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##           develop    #4461      +/-   ##
===========================================
+ Coverage    74.81%   74.82%   +0.01%     
===========================================
  Files          336      336              
  Lines        33998    34003       +5     
  Branches      5527     5529       +2     
===========================================
+ Hits         25434    25444      +10     
+ Misses        7003     6994       -9     
- Partials      1561     1565       +4
# XXX: Quick hack to stop room directory queries taking too long.
# Timeout request after 60s. Probably want a more fundamental
# solution at some point
timeout = datetime.now() + timedelta(seconds=60)

This comment has been minimized.

@richvdh

richvdh Jan 24, 2019

Member

For the record, I'm not generally a fan of datetime objects when a simple "seconds since the epoch" will do and be much more explicit. They have a whole bunch of arithmetic and timezone logic which is pretty redundant here. (In this case, self.clock.time() + 60 will do what you want).

I'm happy if you want to leave this though, given it's a quick hack.

This comment has been minimized.

@richvdh

richvdh Jan 24, 2019

Member

(as a question to ponder: what will this do when the clocks change? I think it will do the right thing, but tbh who knows. It's a question you just don't need to worry about with "seconds since the epoch".)

This comment has been minimized.

@anoadragon453

anoadragon453 Jan 24, 2019

Author Member

I've changed it to just use epoch seconds, thanks for the heads up :)

@richvdh
Copy link
Member

richvdh left a comment

even better!

@@ -87,7 +93,8 @@ def get_local_public_room_list(self, limit=None, since_token=None,
@defer.inlineCallbacks
def _get_public_room_list(self, limit=None, since_token=None,
search_filter=None,
network_tuple=EMPTY_THIRD_PARTY_ID,):
network_tuple=EMPTY_THIRD_PARTY_ID,
timeout=0,):

This comment has been minimized.

@richvdh

richvdh Jan 24, 2019

Member

None is probably better as a magic value than 0, fwiw

This comment has been minimized.

@anoadragon453

anoadragon453 Jan 24, 2019

Author Member

Probs

@anoadragon453 anoadragon453 merged commit 50c396a into develop Jan 24, 2019

5 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

@anoadragon453 anoadragon453 deleted the anoa/room_dir_quick_fix branch Jan 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment