Re: PGStream synchronization

From: Maciek Sakrejda <msakrejda(at)truviso(dot)com>
To: Oliver Jowett <oliver(at)opencloud(dot)com>
Cc: pgsql-jdbc(at)postgresql(dot)org
Subject: Re: PGStream synchronization
Date: 2009-08-26 09:01:51
Message-ID: 895e58dd0908260201g59762012ka2349eada052ab6d@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-jdbc

> Right, so long as you get the atomicity correct I think that will work.
> This will probably require more finegrained synchronization since
> currently there's effectively one big lock that's held for the entire
> duration of the query, which doesn't work so well here if you want
> close() to interrupt any ongoing operation in a timely fashion.

I wasn't quite sure why this would impact regular (non-COPY) queries,
but after poking around in the source some more, I see what you are
saying. We don't really have a good message-level synchronization
point other than synchronizing on QueryExecutorImpl (i.e., its own
synchronized methods), but that will lead to blocking indefinitely on
close(). This is good enough for most communication since all the
relevant methods in QueryExecutorImpl are synchronized anyway, but
stop() gets around this to avoid blocking on all the other connection
activity before shutdown.

I'm a little wary of introducing message-level synchronization since
(a) it touches a *lot* of core code and (b) there might be a
non-negligible performance impact.

I'm not really sure if there's another clean approach, though. I'm
attaching a patch that takes another dirty approach, but other than
the fact that it seems to work, I don't really like it at all. The
basic idea is to cancel whatever you're doing before closing the
connection: if there's an active copy, have the executor send a
CopyFail; otherwise, have the ProtocolConnectionImpl request a query
cancellation. Then close the connection without fear of blocking
(although this should probably be synchronized with QueryExecutorImpl
since, theoretically, we could have close() trigger a cancel and then
another thread could start another query before we actually send the
Terminate message on PGStream).

However, this seems like a hack. QUERY CANCELED is not really the
right error for when a connection is shutting down while your query is
running. It should really be some sort of CONNECTION EXCEPTION, and we
should just run into that automatically when we send the Terminate
message, but for that, we're back to protocol message-level
synchronization.

If we do want to synchronize at the message level, is there a
reasonable way to gauge the performance impact of this change? I.e.,
is there a standard pgsql-jdbc performance suite for measuring
relative performance changes?

--
Maciek Sakrejda | Software Engineer | Truviso
(650) 242-3500 Main
(650) 242-3501 F
msakrejda(at)truviso(dot)com
www.truviso.com

Attachment Content-Type Size
close.diff text/x-patch 1.5 KB

In response to

Responses

Browse pgsql-jdbc by date

  From Date Subject
Next Message Mario Splivalo 2009-08-26 16:28:44 Inserting 'large' amounts of data
Previous Message Oliver Jowett 2009-08-25 22:53:46 Re: PGStream synchronization