Options to block startup unless RabbitMQ Transport / Event Handler are connected #3381
Conversation
| @@ -35,6 +35,7 @@ general: { | |||
| #queue_autodelete = false # Whether or not incoming queue should autodelete after janus disconnects from RabbitMQ | |||
| #queue_exclusive = false # Whether or not incoming queue should only allow one subscriber | |||
| #heartbeat = 60 # Defines the seconds without communication that should pass before considering the TCP connection unreachable. | |||
| #block_startup_until_connected = true # Whether to block the server from starting until a connection to the RabbitMQ server is established. | |||
| uint8_t retry_interval = 5; // Retry interval in seconds | ||
|
|
||
| while (!g_atomic_int_get(&stopping)) { | ||
| if (janus_rabbitmqevh_tryconnect() == 0) { |
There was a problem hiding this comment.
Code style: we prefer no space between the check (while, if etc.) and what comes next. As such, this should be
while(...
and
if(...
There's a few other occurrences of the same, so please change them there too.
| @@ -455,7 +461,22 @@ int janus_rabbitmq_init(janus_transport_callbacks *callback, const char *config_ | |||
| rmq_client = g_malloc0(sizeof(janus_rabbitmq_client)); | |||
|
|
|||
| /* Connect */ | |||
| int result = janus_rabbitmq_connect(); | |||
| uint8_t retry_interval = 5; // Retry interval in seconds | |||
| if (!block_startup_until_connected) { | ||
| break; | ||
| } | ||
|
|
There was a problem hiding this comment.
All these empty lines are unneeded: I think you can make this block more compact without them.
| break; | ||
| } | ||
|
|
||
| JANUS_LOG(LOG_ERR, "RabbitMQ: Connection failed, retrying in %d seconds\n", retry_interval); |
There was a problem hiding this comment.
Not sure if this should be a LOG_WARN instead, since you're retrying.
| @@ -87,6 +87,7 @@ static GThread *handler_thread; | |||
| static GThread *in_thread; | |||
| static void *jns_rmqevh_hdlr(void *data); | |||
| static void *jns_rmqevh_hrtbt(void *data); | |||
| int janus_rabbitmqevh_tryconnect(void); | |||
| int janus_rabbitmqevh_connect(void); | |||
There was a problem hiding this comment.
I'm not sure why there's two functions here now. You made it inline in the transport, why not do the same in the event handler?
|
@chriswiggins any update on my comments? |
Apologies I have been away on vacation. Will have a look next week 😀 |
|
@chriswiggins ping 😁 |
|
Hey @lminiero - apologies I keep doing little bits and pieces on this and then getting pulled to other tasks. Will endeavour to get this done this week |
We have been doing some failure scenario planning inside our staging Kubernetes environment and have noticed that if our Janus instances come up before RabbitMQ, they will happily start even though an error is logged to the console.
These changes bring in new options to both RabbitMQ locations:
block_startup_until_connected. Setting this to true in either or both of the respective configuration files will cause the server to infinitely loop until RabbitMQ becomes available.As a side note - it doesn't appear we can get these Transport / Event plugins status information externally. Is that something thats possible?