Remove need for worker_main_http_uri setting to use /keys/upload.

[realtyem]

Fixes #13642

worker_main_http_uri is a setting used by a worker yaml config file to direct a worker to proxy the /keys/upload endpoint to the main process. Instead of using a proxy, send the request to main by way of http replication.

To-do:

• Build a new replication http servlet based on ReplicationUserDevicesResyncRestServlet because keys are ultimately about devices anyways.
• Add conditional to main process KeyUploadServlet to use replication if a worker.
• Strip out the old proxy version of KeyUploadServlet that was defined in generic_worker.py, just going to use the existing one for the main process. This functionality is now carried by the replication http servlet.
• Overly long Docstring(please check me for accuracy).
• Update configuration manual to include deprecation of setting.

Some odd things happened while I was working on this. While poking around and trying to figure out replication http, I missed filling out the _serialize_payload() function overload. Sytest correctly caught this and errored. Complement did not. Why? There is only one test for this /keys/upload endpoint, if the request got no information(it's a post request), then shouldn't it have errored since it would not have gotten a user_id nor a device_id(apparently not getting an actual set of keys doesn't matter as the response only sends back a count of whatever onetime_keys it finds and not some kind of acknowledgement).

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
• Pull request includes a sign off
• Code style is correct
  (run the linters)

Signed-off-by: Jason Little [email protected]

[clokep]

We probably want to note that the configuration setting can be removed (and maybe log a deprecation notice if it is set? I don't know how folks feel about that)

[realtyem]

We probably want to note that the configuration setting can be removed (and maybe log a deprecation notice if it is set? I don't know how folks feel about that)

Update docs, ok. The other, no idea how to do that.

[DMRobertson]

Can we log a warning if "worker_main_http_uri" in config?

Something like "The config option worker_main_http_uri is unused since Synapse 1.72. It can be safely removed from your configuration."

[realtyem]

Level INFO ok? Or would DEBUG be better?

[DMRobertson]

Warning, please.

[DMRobertson]

synapse/synapse/config/database.py

Line 154 in 493c2fc

logger.warning(NON_SQLITE_DATABASE_PATH_WARNING)

for example. More generally, grep for logger.warning in synapse/config

[realtyem]

Added in 8b4e099
That look ok?

[DMRobertson]

I suspect the point is:

• for now, there must be exactly one worker writing keys (to avoid write races)
• we haven't found the key writing to be heavy enough that it needs pulling off the master worker
• so it's written by the master.

[realtyem]

I guess that specifically the "cached in the database and streamed to workers." is the part I wanted to make sure was accurate. This is a copy-paste dangler. The Replication HTTP Servlet itself does no caching of it's own(the CACHE modifier is set to False below)

[DMRobertson]

Broadly speaking, the database doesn't cache anything; it is the authoritative source of truth for the state of the homeserver. The application maintains various in-memory caches of the database's data. Workers typically receive cache invalidation messages over redis rather than new updates.

[DMRobertson]

borrowed and expanded from KeyUploadServlet

KeyUploadServlet presumably refers to client/keys.py and not generic_worker (whose KeyUploadServelet was removed above).

I think it makes more sense to

• update the docstring for KeyUploadServelet with your changes
• have this docstring point us to that one. "See KeyUploadServelet for format of the request body and response body."
• keep the POST /_synapse/replication/upload_keys_for_user line here

[realtyem]

Yeah, it's slightly different though. Because of the passing of the information in the _serialize_json() _serialize_payload() includes having the user_id and the device_id which isn't part of the same request as the original prepended to the beginning of the request...(I think). Plus it wraps it into a keys object as well.

[realtyem]

Took me a minute, but I see what you mean. How about f2c780b?

[DMRobertson]

Broadly speaking, the database doesn't cache anything; it is the authoritative source of truth for the state of the homeserver. The application maintains various in-memory caches of the database's data. Workers typically receive cache invalidation messages over redis rather than new updates.

[DMRobertson]

Closes #13642.

[clokep]

I put up matrix-org/sytest#1319 to remove this from sytest.