adding max channels per upstream connection as configurable item

[markusgust]

fixes #225

discussion in slack: https://84codes.slack.com/archives/CGPFUGGS0/p1763573521414509

To be able to set a channel-max on upstream connections. If unset, will default on the brokers configuration, or default to UInt16::MAX if disabled on broker (similar to current behaviour).

Will negotiate the lowest value between what customer specifies and what the broker says to ensure we don't override broker settings.

Will ensure that there is a way to distribute amqproxy connections across if a multi-node cluster is used.

[spuun]

I wonder if we should add support for query parameters in the upstream url instead of new configs? Like we do in amqp-client.cr? channel_max, heartbeat and such.

[spuun]

What if it's @max_upstream_channels is set to 0? It should probably default to `UInt16::MAX in that case?

[markusgust]

yes setting that to 0 does indeed break amqproxy, and yes that sounds like how it "should" work.

[markusgust]

I wonder if we should add support for query parameters in the upstream url instead of new configs? Like we do in amqp-client.cr? channel_max, heartbeat and such.

If I read it right then we are doing both for idle_connection_timeout for example, which is a bit similar? but also it's not really an amqp parameter so maybe it shouldn't be just the same

edit; noticed it is not working, we are extracting query parameters in

amqproxy/src/amqproxy/server.cr

Line 20 in 0df86ca

idle_connection_timeout = url.query_params.fetch("idle_connection_timeout", 5).to_i

but we are only sending the value when creating here so we never fetch the value from the URL?:

amqproxy/src/amqproxy/cli.cr

Line 120 in 0df86ca

server = @server = AMQProxy::Server.new(u.hostname || "", port, tls, @idle_connection_timeout)

so instead we should pass the upstream URL and do the parsing in Server.new? then we can extract the query parameter there? then we won't take config items in consideration (unless we merge them to the url beforehand.)

or.. we just take them in the AMQP_URL, pass them to Server.new, and remove the option to use config file for it :)

[spuun]

If I read it right then we are doing both for idle_connection_timeout for example, which is a bit similar? but also it's not really an amqp parameter so maybe it shouldn't be just the same

edit; noticed it is not working, we are extracting query parameters in

amqproxy/src/amqproxy/server.cr

Line 20 in 0df86ca

idle_connection_timeout = url.query_params.fetch("idle_connection_timeout", 5).to_i

but we are only sending the value when creating here so we never fetch the value from the URL?:

Hm, yeah, the constructor accepting an uri is never used 🤷

We can go with this. If we want to refactor the code in the future it's not a problem, we could still support cli opt/env/config.

Making sure that 0 means UInt16::MAX should be fixed though.

[spuun]

LGTM!