m6w6 / ext-pq

PostgreSQL client library (libpq) binding
BSD 2-Clause "Simplified" License
39 stars 7 forks source link

Queue for async queries #10

Closed ParkFramework closed 8 years ago

ParkFramework commented 8 years ago

libpq

PQsendQuery cannot be called again (on the same connection) until PQgetResult has returned a null pointer, indicating that the command is done.

http://www.postgresql.org/docs/current/static/libpq-async.html

We need not wait for the result to send a lot of independent inquiries.

$db->execAsync($sql_1);
$db->execAsync($sql_2);
$db->execAsync($sql_3);

Even if one of the queries return error, other two must be fulfilled regardless. I think where to do it right, in user app or in module php-pq? Thank.

Not to propose merge queries :)

$db->execAsync("$sql_1; $sql_2; $sql_3");
m6w6 commented 8 years ago

Yeah, while checking the test cases for issue #9 I noticed that behavior, too.

Let me think a few days about whether and how to implement that queue.

ParkFramework commented 8 years ago

I'm think, new async API without hell callbacks, used coroutine - yield Generator. Tomorrow writing prototype new async API.

m6w6 commented 8 years ago

without hell callbacks

:question: :smiley:

ParkFramework commented 8 years ago

Yes, callbacks are many shortcomings, there are no exceptions, the code is divided into many parts, I do not like this node.js :) http://callbackhell.com/

ParkFramework commented 8 years ago

Let me think a few days about whether and how to implement that queue.

Perhaps a good solution is to use a pool free connections (sockets) in one instance pq\Connection?

pq\Statement - always used only single connection pq\Transaction - always used only single connection

Connection::execAsync - used any free connections from pool. This only way to make multi thread executing queries in PostgreSQL.

m6w6 commented 8 years ago

Complex logic should be implemented in userland, because it's easier to implement, maintain and iterate over.

DaveRandom commented 8 years ago

TL;DR I'm -1 on this being an issue with the extension, it's an issue with the consuming application.


I'm not sure that the extension is the right place to implement a queue like this - I'd much rather have some kind of LogicException when I try to execute a second command against a busy connection.

Queues and pools can be and frequently are implemented in userland, if the extension queues things as well that gets way harder to track, whereas an exception helps me find bugs in my queuing/pooling logic. There is no such thing as a universal queue or a universal pool that serves everyone's use case (someone might want a priority queue, someone might want some kind of advanced management of channel listeners, etc etc) and because of this I'd rather have nothing and keep the extension as a thin-ish wrapper over libpq with a nicer API, to serve as a primitive for building app logic around.

If the extension did provide this functionality, it should be in the form of decorator classes and not bolted on to the side of pq\Connection etc.

IMHO, YMMV, slippery when wet, <insert other disclaimers here>

ParkFramework commented 8 years ago

Complex logic should be implemented in userland, because it's easier to implement, maintain and iterate over.

Well, I agree.

m6w6 commented 8 years ago

Currently a pq\Exception\RuntimeException is raised:

$ php -r '$c=new pq\Connection; $c->execAsync(""); $c->execAsync("");'

Fatal error: Uncaught pq\Exception\RuntimeException: Failed to execute query (another command is already in progress)

So marking it wontfix for now, please re-open if wanted.

docteurklein commented 6 years ago

I'm having a similar problem on a userland-impl of a connection pool: https://github.com/docteurklein/react-pq/commit/4e61634816f058c1ff5c60de5c937fb04bb348e2

would you have an idea why ?

DaveRandom commented 6 years ago

@docteurklein come and talk to me in Room 11, github issues is not a great place for such a conversation

m6w6 commented 6 years ago

So, was the problem resolved for @docteurklein?

DaveRandom commented 6 years ago

Not as yet. no further discussion has taken place.

amphp/postgres has a working Pool implementation though, so I remain confident that this is a problem best solved in userland, at least in the context of async - in general there are too many external factors that need to be accounted for when dealing with connection management to create a useful API in the extension.

That said, it's possible that a synchronous Pool might be a useful thing, i.e. some kind of API that is conceptually similar to curl_multi_*(), whereby the top-level API is blocking but userland can pass in multiple "actions" to execute concurrently. I don't personally have a use-case, but I can imagine that they might exist.

docteurklein commented 6 years ago

oh sorry, I didn't see the notice in time :| I might not be fully available so an async form of messaging would work for me (even if harder to communicate). Thanks for the links BTW!

docteurklein commented 6 years ago

wow, crazy amount of work on amp/psql! nice job!