Add Client Workaround With a Load Balancer

[JiaYingZhang]

This PR add client workaound with a load balancer.

Client Workaround With a Load Balancer

The implementation method references Python rstream client.

• Add a new load_balancer_mode option to ClientOptions, defaulting to false.
• Implemented the requirement for the producer to connect to the leader node.
• Added usage instructions to Readme.md.

[Gsantomaggio]

@JiaYingZhang Thank you.

• It seems there is a format problem.
• is there a way to create tests to cover the code?

@wolf4ood @DanielePalaia do you have a chance to check the PR?

[JiaYingZhang]

yes，I’ll fix it right away

…

---- Replied Message ---- | From | Gabriele ***@***.***> | | Date | 07/29/2024 19:59 | | To | rabbitmq/rabbitmq-stream-rust-client ***@***.***> | | Cc | kaka ***@***.***>, Mention ***@***.***> | | Subject | Re: [rabbitmq/rabbitmq-stream-rust-client] Add Client Workaround With a Load Balancer (PR #220) | @JiaYingZhang Thank you. It seems there is a format problem. is there a way to create tests to cover the code? @***@***.*** do you have a chance to check the PR? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[wolf4ood]

Please remove the unwrap, if the property advertised_host is not found return an error.
Same for the producer

[JiaYingZhang]

I can't find a suitable error return in ConsumerCreateError, and I don't think it's necessary to add a new error type for this. I will remove unwrap , if there is no advertised_host value in connection_properties, it will create the new connection and ignore the current one. Is this approach acceptable? | | 张嘉颖 | | ***@***.*** | ---- Replied Message ---- | From | Enrico ***@***.***> | | Date | 7/30/2024 15:09 | | To | ***@***.***> | | Cc | ***@***.***> , ***@***.***> | | Subject | Re: [rabbitmq/rabbitmq-stream-rust-client] Add Client Workaround With a Load Balancer (PR #220) | @wolf4ood requested changes on this pull request. In src/consumer.rs:

@@ -76,12 +76,26 @@ impl ConsumerBuilder {

metadata.replicas, stream ); - client = Client::connect(ClientOptions { - host: replica.host.clone(), - port: replica.port as u16, - ..self.environment.options.client_options - }) - .await?; + let load_balancer_mode = self.environment.options.client_options.load_balancer_mode; + if load_balancer_mode { + let options = self.environment.options.client_options.clone(); + loop { + let temp_client = Client::connect(options.clone()).await?; + let mapping = temp_client.connection_properties().await; + let advertised_host = mapping.get("advertised_host").unwrap(); Please remove the unwrap, if the property advertised_host is not found return an error. Same for the producer — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[wolf4ood]

@JiaYingZhang

i guess it's fine to skip the current tmp_client one if there is no advertised_host property

[JiaYingZhang]

@Gsantomaggio @wolf4ood
Is anything I need to do?

[Gsantomaggio]

@JiaYingZhang thanks a lot.
There is an error on the CI:

    Checking rabbitmq-stream-client v0.4.2 (/home/runner/work/rabbitmq-stream-rust-client/rabbitmq-stream-rust-client)
error: this bound is already specified as the supertrait of `MetricsCollector`
   --> src/environment.rs:124:44
    |
124 |         collector: impl MetricsCollector + Send + Sync + 'static,
    |                                            ^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#implied_bounds_in_impls
    = note: `-D clippy::implied-bounds-in-impls` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::implied_bounds_in_impls)]`
help: try removing this bound
    |
124 -         collector: impl MetricsCollector + Send + Sync + 'static,
124 +         collector: impl MetricsCollector + Sync + 'static,
    |

error: this bound is already specified as the supertrait of `MetricsCollector`
   --> src/environment.rs:124:51
    |
124 |         collector: impl MetricsCollector + Send + Sync + 'static,
    |                                                   ^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#implied_bounds_in_impls
help: try removing this bound
    |
124 -         collector: impl MetricsCollector + Send + Sync + 'static,
124 +         collector: impl MetricsCollector + Send + 'static,
    |

error: could not compile `rabbitmq-stream-client` (lib) due to 2 previous errors
make: *** [Makefile:8: clippy] Error 101
Error

Not sure it is from this PR. @wolf4ood ?

[wolf4ood]

Seems a new clippy rule, @JiaYingZhang I think you can simply run
cargo clippy --fix --lib -p rabbitmq-stream-client and it should fix it

[JiaYingZhang]

@wolf4ood
It does't work，nothing changes.
Is this related to the version of Cargo?
My cargo version:

cargo 1.77.1 (e52e36006 2024-03-26)

[wolf4ood]

@JiaYingZhang you might need to update the toolchain for automatically applying the fix,

otherwise you can do it manually by just removing Send + Sync there

[JiaYingZhang]

@wolf4ood
Yes, you're right. Because this isn't my code, I'm being cautious : )
i will fix it up right now. Thank you for your guidance.

[Gsantomaggio]

Tests are green. There is a problem with Codecov ( I will maybe remove it or consider it optional).
Can we merge @DanielePalaia @wolf4ood ?
Thank you all