mojolicious / mojo-pg

Mojolicious :heart: PostgreSQL
https://metacpan.org/release/Mojo-Pg
Artistic License 2.0
101 stars 46 forks source link

Mojo::Pg::PubSub needs better connection management #50

Closed kraih closed 5 years ago

kraih commented 5 years ago

Mojo::Pg::PubSub needs to be fixed to properly maintain a persistent connection, even if the PostgreSQL server has been unavailable for some time. The current reconnect code does not work properly in a few cases and we get regular complaints about it on IRC.

kraih commented 5 years ago

If we can't find a proper solution, i propose the removal of Mojo::Pg::PubSub for now, since it's not good enough for a Mojolicious spin-off project.

kwakwaversal commented 5 years ago

I'd be sad to see this removed. I'm using it in a few projects to provide real-time events. Leveraging the NOTIFY/LISTEN functions in combination with websockets is one of the things that makes using Mojo with PostgreSQL so great.

If I can reproduce some of the cases where the reconnect doesn't work I might be able to have a look at it. The way I deal with the db connection going away is code that looks like this.

my $notify = {};
$notify->{channel_foo} = sub { ... };
$notify->{channel_bar} = sub { ... };

my $pg = Mojo::Pg->new("...");
my $cb;
$cb = sub {
  my ($pg, $dbh) = @_;
  for my $chan (keys %{$notify}) {
    $pg->pubsub->unlisten($chan => $notify->{$chan})
      ->listen($chan => $notify->{$chan});
  }
  $pg->once(connection => $cb);
};
$pg->once(connection => $cb);

Then ping the db connection every second so that it raises the relevant event to resubscribe the subscriptions.

kwakwaversal commented 5 years ago

Thinking about it, the ping isn't a standard ping, it's eval'd to silently ignore an exception.

    unless (eval { $pg->db->ping; 1; }) {
      say 'Notification database not pinging...';
    }
kraih commented 5 years ago

Unfortunately there has been no proposal so far. That means we will have to deprecate Mojo::Pg::PubSub.

jberger commented 5 years ago

My apologies for not opening the PR on that branch. I consider it to be a proposal and now that I've opened a PR I welcome comment on it.

kraih commented 5 years ago

This might be resolved now with #51.