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

lazyload aware /messages #3589

Merged
merged 73 commits into from Aug 16, 2018
Merged

lazyload aware /messages #3589

merged 73 commits into from Aug 16, 2018

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented Jul 23, 2018

test in matrix-org/sytest#471

builds on #3574 (and in turn #3568 (in turn #3567 (in turn #3331 (in turn #2970)))). all now merged

This doesn't yet make any effort to suppress redundant members across adjacent calls to /messages.

ara4n and others added 26 commits June 4, 2018 01:04
as per the proposal; we can deduplicate redundant lazy-loaded members
which are sent in the same sync sequence. we do this heuristically
rather than requiring the client to somehow tell us which members it
has chosen to cache, by instead caching the last N members sent to
a client, and not sending them again.  For now we hardcode N to 100.
Each cache for a given (user,device) tuple is in turn cached for up to
X minutes (to avoid the caches building up).  For now we hardcode X to 30.
works, although considers all room membership events rather than just joined
ones as i'm failing to see a nice way to filter based on membership
@ara4n ara4n changed the title first cut at lazyload aware /messages lazyload aware /messages Jul 23, 2018
"r0.1.0",
"r0.2.0",
# we don't need to include the minor values any more, as we've defined
# X.Y.Z to be backwards-compatible with X.(Y-1).* where Y>1
Copy link
Member

Choose a reason for hiding this comment

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

Was this the conclusion of that conversation? I thought for r0 we were saying Y was effectively the major version.

Copy link
Member Author

Choose a reason for hiding this comment

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

i was saying this, but on closer inspection it turned out we’d never defined it as such.

Copy link
Member

Choose a reason for hiding this comment

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

I think this needs a bit more discussion, which should probably happen outside of the comment box on github.

r0.3.0 isn't compatible with r0.2.0 (push rules changed, some APIs removed, event formats changed) and if I recall correctly r0.3.0 was released intentionally as r0 given the constraints of the API.

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed. @richvdh, wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

yes I agree this needs more discussion :/.

For example, presumably the change to push rules strictly means that we don't support < r0.2.0 any more.

My inclination would be to avoid poking at this hornet's nest for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

have unpoked.

@ara4n
Copy link
Member Author

ara4n commented Aug 14, 2018

@richvdh this is the last of the LL series, if it'd be possible to get review.

@richvdh richvdh requested a review from a team August 16, 2018 12:26
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm

# as per MSC1497:
"unstable_features": {
"m.lazy_load_members": True,
}
Copy link
Member

Choose a reason for hiding this comment

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

trailing comma would be nice...

# conscientious we've been in compatibility, and decide whether the
# middle number is the major revision when at 0.X.Y (as opposed to
# X.Y.Z). And we need to decide whether it's fair to make clients
# parse the version string to figure out what's going on.
Copy link
Member

Choose a reason for hiding this comment

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

For documentation purposes in the hopes that it becomes uncovered:

#960 originally added 0.2.0 as well as 0.1.0 after a question asking if it should be added. Not a lot of descriptions for why, however.

richvdh added a commit that referenced this pull request Aug 22, 2018
Features
--------

- Add support for the SNI extension to federation TLS connections. Thanks to @vojeroen! ([\#3439](#3439))
- Add /_media/r0/config ([\#3184](#3184))
- speed up /members API and add `at` and `membership` params as per MSC1227 ([\#3568](#3568))
- implement `summary` block in /sync response as per MSC688 ([\#3574](#3574))
- Add lazy-loading support to /messages as per MSC1227 ([\#3589](#3589))
- Add ability to limit number of monthly active users on the server ([\#3633](#3633))
- Support more federation endpoints on workers ([\#3653](#3653))
- Basic support for room versioning ([\#3654](#3654))
- Ability to disable client/server Synapse via conf toggle ([\#3655](#3655))
- Ability to whitelist specific threepids against monthly active user limiting ([\#3662](#3662))
- Add some metrics for the appservice and federation event sending loops ([\#3664](#3664))
- Where server is disabled, block ability for locked out users to read new messages ([\#3670](#3670))
- set admin uri via config, to be used in error messages where the user should contact the administrator ([\#3687](#3687))
- Synapse's presence functionality can now be disabled with the "use_presence" configuration option. ([\#3694](#3694))
- For resource limit blocked users, prevent writing into rooms ([\#3708](#3708))

Bugfixes
--------

- Fix occasional glitches in the synapse_event_persisted_position metric ([\#3658](#3658))
- Fix bug on deleting 3pid when using identity servers that don't support unbind API ([\#3661](#3661))
- Make the tests pass on Twisted < 18.7.0 ([\#3676](#3676))
- Don’t ship recaptcha_ajax.js, use it directly from Google ([\#3677](#3677))
- Fixes test_reap_monthly_active_users so it passes under postgres ([\#3681](#3681))
- Fix mau blocking calulation bug on login ([\#3689](#3689))
- Fix missing yield in synapse.storage.monthly_active_users.initialise_reserved_users ([\#3692](#3692))
- Improve HTTP request logging to include all requests ([\#3700](#3700))
- Avoid timing out requests while we are streaming back the response ([\#3701](#3701))
- Support more federation endpoints on workers ([\#3705](#3705), [\#3713](#3713))
- Fix "Starting db txn 'get_all_updated_receipts' from sentinel context" warning ([\#3710](#3710))
- Fix bug where `state_cache` cache factor ignored environment variables ([\#3719](#3719))
- Fix bug in v0.33.3rc1 which caused infinite loops and OOMs ([\#3723](#3723))
- Fix bug introduced in v0.33.3rc1 which made the ToS give a 500 error ([\#3732](#3732))

Deprecations and Removals
-------------------------

- The Shared-Secret registration method of the legacy v1/register REST endpoint has been removed. For a replacement, please see [the admin/register API documentation](https://github.com/matrix-org/synapse/blob/master/docs/admin_api/register_api.rst). ([\#3703](#3703))

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

- The test suite now can run under PostgreSQL. ([\#3423](#3423))
- Refactor HTTP replication endpoints to reduce code duplication ([\#3632](#3632))
- Tests now correctly execute on Python 3. ([\#3647](#3647))
- Sytests can now be run inside a Docker container. ([\#3660](#3660))
- Port over enough to Python 3 to allow the sytests to start. ([\#3668](#3668))
- Update docker base image from alpine 3.7 to 3.8. ([\#3669](#3669))
- Rename synapse.util.async to synapse.util.async_helpers to mitigate async becoming a keyword on Python 3.7. ([\#3678](#3678))
- Synapse's tests are now formatted with the black autoformatter. ([\#3679](#3679))
- Implemented a new testing base class to reduce test boilerplate. ([\#3684](#3684))
- Rename MAU prometheus metrics ([\#3690](#3690))
- add new error type ResourceLimit ([\#3707](#3707))
- Logcontexts for replication command handlers ([\#3709](#3709))
- Update admin register API documentation to reference a real user ID. ([\#3712](#3712))
@erikjohnston erikjohnston deleted the matthew/lazy_load_messages_api branch September 20, 2018 13:57
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.

None yet

5 participants