Skip to content
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 sliding sync on workers #17649

Merged
merged 3 commits into from
Sep 4, 2024
Merged

Fix sliding sync on workers #17649

merged 3 commits into from
Sep 4, 2024

Conversation

erikjohnston
Copy link
Member

Broke in #17630

@erikjohnston erikjohnston marked this pull request as ready for review September 2, 2024 14:01
@erikjohnston erikjohnston requested a review from a team as a code owner September 2, 2024 14:01
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

So the problem was that this background update could only run on the main process, not the background worker?

synapse/storage/databases/main/roommember.py Outdated Show resolved Hide resolved
@erikjohnston erikjohnston merged commit dce38f3 into develop Sep 4, 2024
8 checks passed
@erikjohnston erikjohnston deleted the erikj/ss_worker_mode branch September 4, 2024 09:52
_BackgroundUpdates.SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE,
)
)

Copy link
Contributor

Choose a reason for hiding this comment

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

What was the problem on workers in the first place?

How does this fix the problem?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not all of the storage classes are used on workers, i.e. there are actually two mixins of data stores: one for workers and one for the master. This store isn't in the worker data store.

Ideally we'd merge the two datastores and add asserts to the functions that can't be run on workers.

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

Successfully merging this pull request may close these issues.

3 participants