-
Notifications
You must be signed in to change notification settings - Fork 71
Use asynchronous calls to libpq #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
One thought does occur to me, that this could be done inside Perhaps it would make sense to include these reimplementations in a separate module? Maybe we'd use two modules, with an identical interface:
Then |
As a single data point, a huge +1. I just ran into a bug (in my code, no postgresql-simple) which would have been very well served by the ability to call |
Well, Joey has put in a fair bit of effort into researching these issues on Windows, and it turns out that Another alternative would be to make the blocking calls to C interruptible, which would work on most unixes and Windows, but this requires a new-ish version of GHC and Windows Vista or newer. (Sadly, supporting WinXP is still very relevant for a lot of people, potentially even me.) I stubbed out a build approach to reimplementing libpq's blocking calls in postgresql-libpq here: 96c82197b558019de44cc4b5ef728b95de99f0ab There isn't any useful code at this point, this is just a way to cut down on CPP hackery in supporting both unix and windows. Then we just change all the I suppose we could just export the reimplementations from In any case, it really seems that we need no less than three different bindings to the blocking libpq calls, one that implements the blocking in the GHC runtime via non-blocking libpq, one that implements them as interruptible C calls to blocking libpq, and one that implements them as vanilla safe C calls to libpq. Perhaps we could then have some autodetection magic to select which one to actually compile and export from |
Actually, does anybody know if the interruptible FFI calls are "harmless" (other than not actually being interruptible) on older versions of Windows? Then the only downside would be bumping the required version of GHC to 7.2, which probably isn't a big deal for most Haskellers. I imagine requiring Vista would be much more problematic. |
On Sat, Jul 20, 2013 at 10:58 AM, Leon P Smith [email protected]:
Actually, there are a few ways to wait on a socket in Windows. PostgreSQL
For the purposes of postgresql-libpq, calling select() with a timeout of 1 Thus, interruptible queries are actually pretty easy to support on
On Windows Vista and later, interruptible FFI calls 1 are implemented On Linux, it uses pthread_kill with SIGPIPE. If this interrupts libpq Either way, what exactly should interrupting a query with an exception do? |
I meant that you can't simply fix After thinking about it a while, I'm inclined to make assuming the connection died (2) the default/only behavior directly supported by postgresql-libpq. It should be able to attempt to cancel the query and recover the connection (1) from Haskell client code, at least when using non-blocking libpq calls and the GHC IO manager. But as you point out, this might not be possible when using interruptible ffi calls. The interaction with libpq and interruptible calls sounds like a good question for pgsql-hackers or #postgresql. Anybody feel like asking? |
Is there any update on this? We just hit a problem in our test suite where one connection is blocked on |
@ocharles, it sounds like you forgot to link your executable |
That said, this is still a good idea, but no I haven't worked on it. |
Yea, I found that out just after adding the comment (typical!) - I've amended my earlier comment to mention that. |
Heh, it happens. =) There are a few details to hash out to using libpq asynchronously. As I said before, I think the most reasonable option is to assume that the connection is dead on timeout or other async exception, but the question is then how can the connection be cleaned up in a timely fashion, preferably as cleanly as possible? I certainly wouldn't want to leave backend processes running on the server for very long. So, for example, is it ok to call |
I've taken a look at implementing the libpq blocking api in terms of non-blocking calls (without bothering about the Windows issues) to see what would be involved, and I don't think it can be done with full fidelity using only the public API. It's can certainly be done by poking into PGconn and PGresult's contents, but then you've lost compatibility guarantees across different versions of libpq. |
Very interesting, thank you. It's unfortunate that you can't fully implement this without delving into the internals a bit, although perhaps this is acceptable if the benefits are great enough. After all, at least on my machine the I suppose the alternatives are to either let |
I'd really like to see some benchmarks of @rjmac's work. Does anybody have some code that's suitable? I can't say that I have anything suitable at the moment. We really need something database-bound, both in a single-threaded and concurrent setting. |
I used the latest git source for execStart, but it hasn't been modified in a substantial way since 2011 when bidirectional
For what it's worth, I didn't do any benchmarking. I was only curious about whether it could be done, not about performance. |
Ok, I think I fixed this issue (in postgresql-simple, not postgresql-libpq) with commits 45ccff4 and 83d3c38. This takes care of I don't really have a proper benchmark, but I did time the test suite and any difference was easily lost under the timing noise on my laptop. So if there is a performance regression, it's not a huge one. This was an expedient fix, so unfortunately not all users of |
Actually, it turns out that I would guess that sendQuery would normally only block if the query is large enough that it doesn't fit inside the network socket's write buffer. One could probably demonstrate the weakness in the current implementation by establishing a connection, using iptables or another means to quietly break the connection, trying to send a large query, and then trying to interrupt the thread that's sending the query. This is fixable, using |
From what I can tell by inspection, if |
Hmm, thanks rjmac! That is in the documentation of Reviewing your code again more carefully, it doesn't appear that you do this in your code either, at least not yet. I do think your attempt is interesting, and worth further consideration at some point, but ultimately for me it came down to that I needed to be able to time out database heartbeats and connection attempts. (Although I suppose I could have used joeyadams' |
It does do that, actually, though it's hidden among all the other fully-emulate-blocking-mode noise -- it's in |
Oh I see. Nice. =) Well, anyway a conversation on #postgresql did turn up a bug in libpq that may be a good reason for postgresql-simple to avoid going So perhaps it would make sense to |
Also, see #119 as an example of an issue that's arisen from my quick-and-dirty re-implementation of exec. There's definitely a bit of subtlety when dealing with the COPY case that wasn't immediately apparent from the libpq documentation. (And, the fact that this is an issue at all seems rather strange.) |
Ugh, I think I'd prefer to ditch my quick-and-dirty async implementation and just use rjmac's, but the one thing that makes me hesitate is the fact that in order to compile his code, I needed to also install the server-dev header files as |
My main worry about doing things this way is less user-friendliness -- libpq-int.h's dependencies are a package install away on most platforms -- than with binary compatibility. The only really dangerous thing it does (if I recall correctly; it's been a while) is poke at the I wonder if the postgresql people could be convinced to add an accessor for it. |
Well yes, that's a concern of mine too. I guess it's mitigated in my mind a bit by the fact that it doesn't change often. But yes, the consequences of such a change are pretty bad. Perhaps it would be a good idea to check the version with Adding an accessor may be a good idea, as we could be less conservative in the future. However, are there any symbols exported from a vanilla libpq shared object that aren't part of the public interface? I ask because I started playing around with the first steps towards a native haskell implementation of the protocol, using libpq to connect, but I can't imagine the postgres folk would be too enthusiastic about such a symbol, but perhaps it's worth pointing out that exec cannot be faithfully re-implemented using the public interface alone, and finding a solution for that. Regarding user-friendliness, I agree that I don't think it's going to be an issue on most unix platforms, though people will probably be caught a little off-guard and it might take them a few minutes to track down the issue. Windows, on the other hand, seems to be the real problem point, but I'm not familiar enough with the compilation issues there. (But what am I saying? This code isn't at all usable on Windows for the foreseeable future, so it's not an issue on Windows. So... yeah I agree, user-friendliness isn't much of an issue.) |
Ok, if we want to compare versions, the compile-time version appears to be defined in |
On the other hand, going this route would also require recompilation of NixOS may be better suited to this arrangement, and would certainly be more sympathetic as well. |
After a conversation with @joeyadams, I believe postgresql-simple ought to use asynchronous calls to libpq. We would then use
threadWaitRead
andthreadWaitWrite
to schedule asynchronous calls using GHC's IO manager, instead of using blocking C calls and having the OS kernel do the scheduling.The performance impact of such a change is unclear, but the one clear advantage is that
System.Timeout
could then be used withconnect
,query
,execute
, etc.System.Timeout
cannot be used with blocking C calls because GHC's runtime is not in control of the thread and cannot interrupt the thread until the call returns.The text was updated successfully, but these errors were encountered: