Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

amqp: heartbeat support #2676

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@nobles
Copy link
Contributor

commented Apr 12, 2019

Users can provide heartbeat in their configuration, that is negotiated with the server. In case heartbeat is enabled, amqp destination will send heartbeats periodically.

Heartbeat is measured in seconds. Default value is zero.

Example configuration:

    destination { amqp(vhost("/")
                  exchange("logs")
                  body("hello world")
                  heartbeat(10)
                  username(guest) password(guest)); };

Fixes: #189

@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

This user does not have permission to start the build. Can one of the admins verify this patch and start the build?
(admin: you have the next options (make sure you checked the code):
"ok to test" to accept this pull request (and further changes) for testing
"test this please" for a one time test run
do nothing -> CI won't start)

1 similar comment
@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

This user does not have permission to start the build. Can one of the admins verify this patch and start the build?
(admin: you have the next options (make sure you checked the code):
"ok to test" to accept this pull request (and further changes) for testing
"test this please" for a one time test run
do nothing -> CI won't start)

@furiel

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

@kira-syslogng ok to test

@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

Build SUCCESS

@nobles nobles force-pushed the nobles:amqp-heartbeat branch 2 times, most recently from 61de053 to 6d48377 Apr 12, 2019

@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

Build FAILURE

@furiel

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

@kira-syslogng retest this please

@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

Build SUCCESS

self->auth_method, self->user, self->password);
ret = amqp_login(self->conn, self->vhost, self->max_channel, self->frame_size,
self->offered_heartbeat, self->auth_method, self->user, self->password);
self->heartbeat = amqp_get_heartbeat(self->conn);

This comment has been minimized.

Copy link
@Kokan

Kokan Apr 15, 2019

Member

Optional: for debugging purpose it would be nice to print the self->heartbeat, as it can differ from the value provided by user. Possible on verbose level.

@@ -451,10 +467,12 @@ afamqp_dd_login(AMQPDestDriver *self)
switch (self->auth_method)
{
case AMQP_SASL_METHOD_PLAIN:
ret = amqp_login(self->conn, self->vhost, self->max_channel, self->frame_size, 0,
self->auth_method, self->user, self->password);
ret = amqp_login(self->conn, self->vhost, self->max_channel, self->frame_size,

This comment has been minimized.

Copy link
@Kokan

Kokan Apr 15, 2019

Member

There is also a AMQP_SASL_METHOD_EXTERNAL authentication, would you please fill the heartbeat filed in that case as well ?

Show resolved Hide resolved modules/afamqp/afamqp.c

nobles added some commits Apr 12, 2019

amqp: heartbeat support
Users can provide heartbeat in their configuration,
that is negotiated with the server. In case heartbeat is enabled,
amqp destination will send heartbeats periodically.

Signed-off-by: Terez Nemes <terez.noble@gmail.com>
afamqp: drop connection in case of read error from server
In case of read error from server during heartbeat (for example when
server misses out heartbeats), we tear down the connection.

Signed-off-by: Terez Nemes <terez.noble@gmail.com>

@nobles nobles force-pushed the nobles:amqp-heartbeat branch from 6d48377 to aca722a Apr 17, 2019

@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2019

Build FAILURE

@nobles

This comment has been minimized.

Copy link
Contributor Author

commented Apr 17, 2019

@kira-syslogng retest this please

1 similar comment
@furiel

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2019

@kira-syslogng retest this please

@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2019

Build SUCCESS

@Kokan

Kokan approved these changes Apr 17, 2019

Copy link
Member

left a comment

thanks for fixing one of the oldest issue/feature request 🥇

@gaborznagy gaborznagy self-requested a review Apr 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.