search

Tradeshift / spring-rabbitmq-tuning

This library makes it easy to configure RabbitMQ for use with Spring
MIT License
43 stars 26 forks source link

Feature/request heartbeat #37

Closed santannaf closed 2 years ago

santannaf commented 4 years ago

According to the RabbitMQ docs:

The heartbeat timeout value defines after what period of time the peer TCP connection should be considered unreachable (down) by RabbitMQ and client libraries. This value is negotiated between the client and RabbitMQ server at the time of connection. The client must be configured to request heartbeats. In RabbitMQ versions 3.0 and higher, the broker will attempt to negotiate heartbeats by default (although the client can still veto them). The timeout is in seconds, and default value is 60 (580 prior to release 3.5.5).

I am putting this value so that we do not have an error in runtime, idle or idle connection time. This error, he says that disconnects the channel, will only reactivate again if, for example, send another message.

juliofalbo commented 4 years ago

Hi Thales.

Thanks for the PR.

Actually it is missing one part, the configuration itself. You created the property, now you need to use this property when we are creating the ConnectionFactory and call the method setRequestedHeartbeat. You can do this here: https://github.com/Tradeshift/spring-rabbitmq-tuning/blob/b9faf10e02a067c5d4a92d071a2870c331ac7d49/src/main/java/com/tradeshift/amqp/rabbit/components/RabbitComponentsFactory.java#L51

juliofalbo commented 4 years ago

Hey man sorry!

Now I can see that you already did what I said, so perfect!

Thank you very much again! 🎉

santannaf commented 4 years ago

Yes of course ! I performed a commit with the inclusion of two tests. First by returning a default value, which covers the scenario when the user does not include this property and the other when the user includes a certain value.

santannaf commented 4 years ago

I just couldn't fork the wikki repository to include the property, I don't have access.

santannaf commented 4 years ago

already did the "fork"