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

Update worker docs with recent enhancements #7969

Merged
merged 26 commits into from
Jul 29, 2020

Conversation

erikjohnston
Copy link
Member

@erikjohnston erikjohnston commented Jul 28, 2020

We also add some of the new config options to the sample config.

This also updates the wording to recommend using Redis and removing suggestion that it is experimental.

Apologies in advance that me bad at words.

@erikjohnston erikjohnston force-pushed the erikj/worker_docs branch 2 times, most recently from f6b63fb to 226a0b6 Compare July 28, 2020 10:50
@erikjohnston erikjohnston requested a review from a team July 28, 2020 12:44
@clokep clokep self-assigned this Jul 28, 2020
docs/workers.md Outdated Show resolved Hide resolved
Comment on lines 99 to 106
# If running with federation sender worker instances then they should
# be listed by their `worker_name` here, as well as setting
# `send_federation` to false.
#
#send_federation: false
#
#federation_sender_instances:
# - federation_sender1
Copy link
Member

Choose a reason for hiding this comment

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

I'm not really sure what this means. I think it can be made clearer that this is what gets set in the shared config?

Copy link
Member Author

Choose a reason for hiding this comment

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

How about now? I'm not 100% convinced about the new wording

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 it is much clearer. Reading workers.md also helps a lot. :)

synapse/config/redis.py Outdated Show resolved Hide resolved
synapse/config/redis.py Show resolved Hide resolved
synapse/config/workers.py Show resolved Hide resolved
synapse/config/workers.py Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
Additionally, processes may make HTTP requests to each other. Typically this is
used for operations which need to wait for a reply - such as sending an event.

As of Synapse v1.13.0, it is possible to configure Synapse to send replication
Copy link
Member

Choose a reason for hiding this comment

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

It could be nice to have a snazzy ASCII art diagram here of the processes communicating...

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Member

Choose a reason for hiding this comment

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

does https://matrix.sw1v.org/_matrix/media/r0/download/sw1v.org/uhqIFIqONRWcRsKgVvbehsqF help? some of the links have been left out for clarity but I think it might help?

Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly? I'm not really sure who it's aimed at.

Copy link
Member Author

Choose a reason for hiding this comment

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

Am happy to put it at the end of the doc, to be clear.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not really sure who it's aimed at.

well, people trying to set up workers. The idea is that a picture helps people get an overview of how it works.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, will add it to the end with a note at the start pointing to it 👍

@erikjohnston erikjohnston requested a review from a team July 28, 2020 16:37
Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Looks good! Can you make sure to link the various documentation issues with this PR?

@erikjohnston
Copy link
Member Author

This needs repointing at release branch.

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.

some initial comments, but I need to go and make dinner

docs/sample_config.yaml Outdated Show resolved Hide resolved
docs/sample_config.yaml Outdated Show resolved Hide resolved
docs/sample_config.yaml Outdated Show resolved Hide resolved
docs/sample_config.yaml Outdated Show resolved Hide resolved
# Configuration for Redis when using workers.
#
redis:
# Uncomment the below to enable using Redis to replicate data between workers.
Copy link
Member

Choose a reason for hiding this comment

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

what happens if it's not enabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I guess we should reword this to make it clear that it needs (unless using old style replication) to be enabled if using workers

Additionally, processes may make HTTP requests to each other. Typically this is
used for operations which need to wait for a reply - such as sending an event.

As of Synapse v1.13.0, it is possible to configure Synapse to send replication
Copy link
Member

Choose a reason for hiding this comment

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

+1

docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Outdated Show resolved Hide resolved
docs/workers.md Show resolved Hide resolved
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 otherwise

docs/workers.md Show resolved Hide resolved
@richvdh
Copy link
Member

richvdh commented Jul 29, 2020

hrm, there was more that github has eaten

erikjohnston and others added 7 commits July 29, 2020 23:20
Co-authored-by: Brendan Abolivier <babolivier@matrix.org>
Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
@erikjohnston erikjohnston changed the base branch from develop to release-v1.18.0 July 29, 2020 22:21
@erikjohnston erikjohnston merged commit 2c1b9d6 into release-v1.18.0 Jul 29, 2020
@erikjohnston erikjohnston deleted the erikj/worker_docs branch July 29, 2020 22:22
anoadragon453 added a commit that referenced this pull request Jul 30, 2020
…bership_join_count

* 'develop' of github.com:matrix-org/synapse:
  Update workers docs (#7990)
  Fix invite rejection when we have no forward-extremeties (#7980)
  Fix typo in docs/workers.md (#7992)
  Convert federation client to async/await. (#7975)
  Convert appservice to async. (#7973)
  Convert some of the data store to async. (#7976)
  Fix formatting of changelog and upgrade notes
  Ensure that remove_pusher is always async (#7981)
  Add deprecation warnings
  1.18.0
  Update worker docs with recent enhancements  (#7969)
  Ensure the msg property of HttpResponseException is a string. (#7979)
  Remove from the event_relations table when purging historical events. (#7978)
  Add additional logging for SAML sessions. (#7971)
  Add MSC reference to changelog for #7736
  Re-implement unread counts (#7736)
  Various improvements to the docs (#7899)
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit 'a9631b7b4':
  1.18.0
  Update worker docs with recent enhancements  (#7969)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants