signal to force beaver to reconnect to redis #75

Closed
kitchen opened this Issue Dec 18, 2012 · 12 comments

Comments

Projects
None yet
3 participants
Contributor

kitchen commented Dec 18, 2012

In my setup I'm thinking about having a number of redis instances behind a load balancer. This is for ease of maintenance and redundancy and such.

However, since beaver's redis connections are sticky, should I take one of them down and put it back in service, it will never get any more connections.

I think being able to send a HUP or something to beaver to make it reconnect to redis would be a great option. This would allow me to then tell my beavers to reconnect and the load balancer should take care of spreading things back out again.

Owner

josegonzalez commented Dec 18, 2012

Should be a general HUP signal, which the transports can ignore or do something about.

Owner

josegonzalez commented Dec 18, 2012

Well I just thought about it, and I have the following thoughts:

  • Shouldn't the current retry connection code handle disconnects? There is a TransportException that should try a reconnect, if thrown. The redis transport does not implement this atm.
  • Beaver does not, and never will, daemonize itself. Use something else to do that. Therefore, shouldn't the other item just restart beaver in the event of a SIGHUP?

Not that I am opposed completely to this issue, but I think it should be handled adequately already, no?

Contributor

kitchen commented Dec 18, 2012

On Mon, Dec 17, 2012 at 07:00:34PM -0800, Jose Diaz-Gonzalez wrote:

Well I just thought about it, and I have the following thoughts:

  • Shouldn't the current retry connection code handle disconnects?
    There is a TransportException that should try a reconnect, if
    thrown. The redis transport does not implement this atm.

it does, and it works fine. The only time there's an issue is when it
can't reconnect, then it is a fatal error.

  • Beaver does not, and never will, daemonize itself. Use something
    else to do that. Therefore, shouldn't the other item just restart
    beaver in the event of a SIGHUP?

beaver doesn't currently remember where it left off in the log file. If
it were to eventually do that, then yes, this is a viable approach.

Not that I am opposed completely to this issue, but I think it should
be handled adequately already, no?

The main reason I'm looking for something like this is because if I take
one of my load balanced redis servers down they'll all reconnect and get
the other one, and I'd like to be able to force them to reconnect again
after the first one comes back up.

With beaver remembering where it left off this becomes a non-issue, just
restart beaver, I agree :)

-Jeremy

Owner

josegonzalez commented Dec 18, 2012

See the open milestone. This will get taken care of extremely soon ;)

Owner

josegonzalez commented Dec 18, 2012

Does it stop trying to reconnect for redis? Again, a TransportException is the way to go, because it will eventually try and reconnect. Redis does not currently throw this exception, but it should somewhat ameliorate the issue.

If you still think this is a noteworthy feature - seems neat - I am not opposed to seeing a PR for it :).

Whenever my redis changes, I just send a restart command to supervisorctl, but I can see this being useful if the old redis is still responding and I just want to switch to another once.

Owner

josegonzalez commented Dec 18, 2012

Should it reparse the config file on SIGHUP?

Contributor

kitchen commented Dec 18, 2012

On Tue, Dec 18, 2012 at 12:42:40AM -0800, Jose Diaz-Gonzalez wrote:

Should it reparse the config file on SIGHUP?

that would be cool but not necessary for this particular issue.

I think restarting would be a simpler solution (code-wise, etc) once
beaver can pick up where it left off.

-Jeremy

Owner

josegonzalez commented Jan 15, 2013

Closed by #90? @kitchen please confirm

Contributor

kitchen commented Jan 15, 2013

On Tue, Jan 15, 2013 at 11:47:52AM -0800, Jose Diaz-Gonzalez wrote:

Closed by #90? @kitchen please confirm

looking.

-Jeremy

Contributor

kitchen commented Jan 15, 2013

On Tue, Jan 15, 2013 at 11:47:52AM -0800, Jose Diaz-Gonzalez wrote:

Closed by #90? @kitchen please confirm

this doesn't look like a signal handler, but the reconnection bits look
good!

-Jeremy

@josegonzalez josegonzalez reopened this Jan 15, 2013

Owner

josegonzalez commented Jan 15, 2013

Well there you go, we should do signal handling still then. I have a firm grasp of how it works too!

Owner

JamieCressey commented Dec 7, 2015

Closing due to inactivity. Happy to see a PR for the signal handling if anyone still has that requirement...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment