praekeltfoundation / vumi

Messaging engine for the delivery of SMS, Star Menu and chat messages to diverse audiences in emerging markets and beyond.
BSD 3-Clause "New" or "Revised" License
421 stars 131 forks source link

Remove references to redis sub managers #1031

Closed rudigiesler closed 8 years ago

rudigiesler commented 8 years ago

For the txredis manager, a list of submanagers is maintained for updating the clients of those submanagers in the case of a reconnection. This can cause problems if many sub managers are used, as when the sub manager goes out of scope, it is still being held on in memory by the list on the base manager.

The plan is to create a client proxy object that each submanager points to, which contains a reference to the client. That way, when we update the client in a reconnection, we only need to update the reference in the client proxy object, and not all the references for each submanager, meaning we no longer need the list of submanagers.

rudigiesler commented 8 years ago

Test failures are unrelated to this PR, and are being resolved in https://github.com/praekelt/vumi/pull/1032

hodgestar commented 8 years ago

Left one minor comment, otherwise code looks good to me.

@rudi Is this successfully preventing us leaking sub managers?

rudigiesler commented 8 years ago

@hodgestar Comment addressed.

Yes this does prevent us from leaking sub managers. That list no longer exists, so the sub managers can disappear. I've tested this branch with Junebug benchmarks, and there are no longer any sub managers left over.

hodgestar commented 8 years ago

:+1: