search

miyagawa / AnyEvent-Redis

Asynchronous Redis client
http://search.cpan.org/dist/AnyEvent-Redis
41 stars 16 forks source link

re-queue commands and reconnect on read/write errors #19

Closed leedo closed 12 years ago

leedo commented 12 years ago

This fixes problems caused by a redis-server restart. Previously, a server restart would cause the first subsequent command to silently fail. The on_error handler is not invoked until the first read attempt is made. So now the on_error handler checks for commands that were waiting for a read and re-queues them and reconnects.

chloe-zen commented 12 years ago

sigh what's wrong with https://github.com/miyagawa/AnyEvent-Redis/pull/16 ?

leedo commented 12 years ago

As far as I can tell it doesn't re-queue a command after a connection error, which is the specific problem I'm trying to solve here. I don't really expect this to be merged since this module seems pretty unmaintained right now, but though it might be useful for others.

chloe-zen commented 12 years ago

What if the disconnection happens mid-MULTI?

leedo commented 12 years ago

Yeah, originally I wanted to just have it re-attempt non-MULTI commands, but the logic was getting really messy. Maybe it could be done more elegantly on top of your changes.

Here is a commit that updates just the reconnect test, which may be useful for making more progress on this. https://github.com/leedo/AnyEvent-Redis/commit/b7cc8fa924f7bea463206cb0fe494753d93a1f9d

chloe-zen commented 12 years ago

you'll want to move this to dgl's repo. miyagawa has abandoned this.

miyagawa commented 12 years ago

I don't think moving stuff to dgl's repo magically resolves any issue. They have maint bit on my repository as well.

miyagawa commented 12 years ago

@chipdude i just added you as a collaborator on this repo so that you can merge things in. You still need to ping @dgl to cut a release on PAUSE, but we'll see.

dgl commented 12 years ago

FWIW, I've merged chip's changes now, so maybe this could be made to work as you mention?

PS: There seems to be an AnyEvent::Redis::RipeRedis which claims to have reconnection support, has anyone looked at it? It does seem rather silly there's two AnyEvent Redis libraries now...

leedo commented 12 years ago

Thanks! I'll take a look at reworking this to work with chip's changes.