Page MenuHomePhabricator

[Tunnelbroker] Use rabbitmq for message delivery
ClosedPublic

Authored by jon on Jun 11 2023, 7:52 PM.
Tags
None
Referenced Files
F3245463: D8178.diff
Thu, Nov 14, 6:35 PM
Unknown Object (File)
Sat, Nov 9, 4:04 AM
Unknown Object (File)
Fri, Nov 8, 6:38 AM
Unknown Object (File)
Thu, Nov 7, 11:00 AM
Unknown Object (File)
Mon, Nov 4, 10:56 AM
Unknown Object (File)
Mon, Nov 4, 10:56 AM
Unknown Object (File)
Mon, Nov 4, 10:56 AM
Unknown Object (File)
Thu, Oct 31, 8:51 PM
Subscribers

Details

Summary

Replace hashmap of concurrent connections with
rabbitmq.

Depends on D8177

Part of:

Test Plan

Run the tunnelbroker integration suite. First start docker, then:

nix develop
comm-dev services start

(cd services/tunnelbroker && RUST_LOG=debug cargo run)
(cd services/commtest && cargo test --test tunnelbroker_integration_test)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 11 2023, 7:54 PM
Harbormaster failed remote builds in B20170: Diff 27629!

Rebase on master, improve error handling

services/tunnelbroker/src/websockets/session.rs
88–89 ↗(On Diff #27673)

I'd like to understand this better.

  1. Is it possible that we already had a queue for the given device_id? E.g. in case of reconnection. Does this call fail then? I suspect it's dependant on some QueueDeclareOptions
  2. What happens if we publish a message (channel.basic_publish()) while the queue is not yet declared? Does the "routing key" define the queue name and when it is declared, RabbitMQ redirects it accordingly?
services/tunnelbroker/src/websockets/session.rs
88–89 ↗(On Diff #27673)

Is it possible that we already had a queue for the given device_id? E.g. in case of reconnection. Does this call fail then? I suspect it's dependant on some QueueDeclareOptions

It is possible. The action is idempotent, I can restart tunnelbroker, and redo the logic and it's successful the second time. The options relate to these options, since we are publishing with immediate = true, a lot of these options don't matter as they will just be deleted and kicked back to us if they fail to deliver.

What happens if we publish a message (channel.basic_publish()) while the queue is not yet declared?

I would assume that we would get an error when awaiting the confirmation. But I haven't tested this scenario.

Does the "routing key" define the queue name and when it is declared, RabbitMQ redirects it accordingly?

Rabbitmq allows some flexibility. In our case we have a simple one-to-one correlation of messages routing-key to queue names. However, amqp allows some flexibility, this post should help: https://www.cloudamqp.com/blog/part4-rabbitmq-for-beginners-exchanges-routing-keys-bindings.html

39 ↗(On Diff #27629)

This was refactored out from the previous work.

Previously, the function handled all message requests, but now is scoped down to just the first message.

services/tunnelbroker/src/websockets/session.rs
27 ↗(On Diff #27673)

stream

45 ↗(On Diff #27673)

retrieve

88–89 ↗(On Diff #27673)

since we are publishing with immediate = true

@jon do you mean "since we are publishing with mandatory = true"?

bartek added inline comments.
services/tunnelbroker/src/websockets/session.rs
88–89 ↗(On Diff #27673)

Thanks for the detailed explanation @jon!
I found out that the default QueueDeclareOptions work well for our usecase - the passive bit is set to false - the queue isn't redeclared if already exists and its config hasn't changed

This revision is now accepted and ready to land.Jun 15 2023, 12:13 AM
jon marked 5 inline comments as done.

Address feedback