Concurrent psql patch

On May 11, 2007, at 10:55 AM, Gregory Stark wrote:

Also, if anyone has any better ideas for names than \cswitch and
\cnowait
now's the time. I had intended them only as placeholders because I
couldn't
think of anything better but it doesn't sound like anyone else has
any better
ideas either. If not then we're going to be stuck with them. More
or less,
it's explicitly described as an "experimental" feature in the docs
so I
suppose we could always change them later.

I don't see how we could make the names shorter without moving away
from a backslash command (which I'm guessing would be painful).

Assuming we're stuck with a backslash command \cs[witch] and \cn
[owait] seem to be about as good as we could get.
--
Jim Nasby jim@nasby.net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

"Jim Nasby" <decibel@decibel.org> writes:

I don't see how we could make the names shorter without moving away from a
backslash command (which I'm guessing would be painful).

Assuming we're stuck with a backslash command \cs[witch] and \cn
[owait] seem to be about as good as we could get.

I don't have \cs or \cn set up as abbreviations.

I was originally thinking \c1, \c2, ... for \cswitch and \c& for \cnowait. I'm
not sure if going for cryptic short commands is better or worse here.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

On Sun, May 13, 2007 at 02:39:45PM +0100, Gregory Stark wrote:

"Jim Nasby" <decibel@decibel.org> writes:

I don't see how we could make the names shorter without moving
away from a backslash command (which I'm guessing would be
painful).

Assuming we're stuck with a backslash command \cs[witch] and \cn
[owait] seem to be about as good as we could get.

I don't have \cs or \cn set up as abbreviations.

I was originally thinking \c1, \c2, ... for \cswitch and \c& for
\cnowait. I'm not sure if going for cryptic short commands is better
or worse here.

+1 for \c1, \c2, etc.

What's the reasoning behind \c&? Does it "send things into the
background" the way & does in the shell?

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
phone: +1 415 235 3778 AIM: dfetter666
Skype: davidfetter

Remember to vote!
Consider donating to PostgreSQL: http://www.postgresql.org/about/donate

"David Fetter" <david@fetter.org> writes:

I was originally thinking \c1, \c2, ... for \cswitch and \c& for
\cnowait. I'm not sure if going for cryptic short commands is better
or worse here.

+1 for \c1, \c2, etc.

What's the reasoning behind \c&? Does it "send things into the
background" the way & does in the shell?

Sort of. It sends the *subsequent* command to the background... And unlike the
shell you can't usefully do anything more in the current session while the
command is in the background, you have to manually switch sessions before
issuing subsequent commands.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

On Sun, May 13, 2007 at 02:39:45PM +0100, Gregory Stark wrote:

"Jim Nasby" <decibel@decibel.org> writes:

I don't see how we could make the names shorter without moving away from a
backslash command (which I'm guessing would be painful).

Assuming we're stuck with a backslash command \cs[witch] and \cn
[owait] seem to be about as good as we could get.

I don't have \cs or \cn set up as abbreviations.

I was originally thinking \c1, \c2, ... for \cswitch and \c& for \cnowait. I'm
not sure if going for cryptic short commands is better or worse here.

Would \c# limit us to 9 concurrent connections? Might want

\cs[witch] [session]

which would switch to the specified session. If none specified, it would
switch back to whatever session was previously active.

\c& sounds fine (as do \c1...\c9). \g& would probably be helpful as well
(send query buffer to server in nowait mode).
--
Jim Nasby decibel@decibel.org
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

"Jim C. Nasby" <decibel@decibel.org> writes:

On Sun, May 13, 2007 at 02:39:45PM +0100, Gregory Stark wrote:

I was originally thinking \c1, \c2, ... for \cswitch and \c& for \cnowait. I'm
not sure if going for cryptic short commands is better or worse here.

\c& sounds fine (as do \c1...\c9). \g& would probably be helpful as well
(send query buffer to server in nowait mode).

Er, I just realized I typed the wrong thing there. It can't be \c& since I do
assign a meaning to that "make a new connection to the same place as this
one".

I meant \&

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Gregory Stark <stark@enterprisedb.com> writes:

"David Fetter" <david@fetter.org> writes:

What's the reasoning behind \c&? Does it "send things into the
background" the way & does in the shell?

Sort of. It sends the *subsequent* command to the background...

That sounds just bizarre. Existing backslash commands that do something
to a SQL command are typed *after* the command they affect (\g for
instance). I don't think you should randomly change that.

regards, tom lane

"Tom Lane" <tgl@sss.pgh.pa.us> writes:

Gregory Stark <stark@enterprisedb.com> writes:

"David Fetter" <david@fetter.org> writes:

What's the reasoning behind \c&? Does it "send things into the
background" the way & does in the shell?

Sort of. It sends the *subsequent* command to the background...

That sounds just bizarre. Existing backslash commands that do something
to a SQL command are typed *after* the command they affect (\g for
instance). I don't think you should randomly change that.

So would you prefer \g& as Jim Nasby suggested? I hadn't even considered that
previously since I'm not accustomed to using \g but it does seem kind of
pretty. I normally use ; but I suppose there's nothing wrong with just
declaring that asynchronous commands must be issued using \g& rather than use
the semicolon to fire them off.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Gregory Stark <stark@enterprisedb.com> writes:

So would you prefer \g& as Jim Nasby suggested? I hadn't even considered that
previously since I'm not accustomed to using \g but it does seem kind of
pretty. I normally use ; but I suppose there's nothing wrong with just
declaring that asynchronous commands must be issued using \g& rather than use
the semicolon to fire them off.

It makes sense to me... but what is the state of the session afterward?
Should this be combined with switching to another connection?

regards, tom lane

"Tom Lane" <tgl@sss.pgh.pa.us> writes:

Gregory Stark <stark@enterprisedb.com> writes:

So would you prefer \g& as Jim Nasby suggested? I hadn't even considered that
previously since I'm not accustomed to using \g but it does seem kind of
pretty. I normally use ; but I suppose there's nothing wrong with just
declaring that asynchronous commands must be issued using \g& rather than use
the semicolon to fire them off.

It makes sense to me... but what is the state of the session afterward?
Should this be combined with switching to another connection?

It's an interesting idea since you'll inevitably have to switch connections.
If you issue a second query it'll forces the session to wait for the results.
(It doesn't seem like there's any point in keeping a queue of pending queries
per session.)

However we do still need a command to switch back anyways so there doesn't
seem to be any advantage in combining the two.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

"Jim C. Nasby" <decibel@decibel.org> writes:

Would \c# limit us to 9 concurrent connections? Might want

\cs[witch] [session]

Hm, we kind of have a choice with \c#. Either we treat it as part of the
command in which case the way to connect to an integer-named database is to
include a space. We could even have it magically connect to a database if the
connection isn't already active.

But these kinds of inconsistent behaviours can be traps for users. It means
"\c1" and "\c 1" do different things even though "\cpostgres" and \c postgres"
do the same thing. And it means "\c1" might connect to a database named "1"
today but switch sessions tomorrow.

Or we treat it as the first argument in which case even "\c 9" switches to
session 9. I would prefer to do that but I fear there may be people with
databases named "9".

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Gregory Stark <stark@enterprisedb.com> writes:

But these kinds of inconsistent behaviours can be traps for users. It means
"\c1" and "\c 1" do different things even though "\cpostgres" and \c postgres"
do the same thing. And it means "\c1" might connect to a database named "1"
today but switch sessions tomorrow.

The real problem here is trying to overload an existing command name
with too many different meanings. You need to pick some other name
besides \c.

If you were willing to think of it as "switch session" instead of "connect",
then \S is available ...

regards, tom lane

On Mon, May 14, 2007 at 12:51:39PM +0100, Gregory Stark wrote:

"Tom Lane" <tgl@sss.pgh.pa.us> writes:

Gregory Stark <stark@enterprisedb.com> writes:

So would you prefer \g& as Jim Nasby suggested? I hadn't even considered that
previously since I'm not accustomed to using \g but it does seem kind of
pretty. I normally use ; but I suppose there's nothing wrong with just
declaring that asynchronous commands must be issued using \g& rather than use
the semicolon to fire them off.

It makes sense to me... but what is the state of the session afterward?
Should this be combined with switching to another connection?

It's an interesting idea since you'll inevitably have to switch connections.
If you issue a second query it'll forces the session to wait for the results.
(It doesn't seem like there's any point in keeping a queue of pending queries
per session.)

However we do still need a command to switch back anyways so there doesn't
seem to be any advantage in combining the two.

I'd thought about this, and the question I came up with was: what
connection should we switch to? First thought was to switch back to
whatever connection we'd been using before this one, but then you'd
quickly have 2 connections tied up... then what?

If someone could come up with a logical session to connect to
automatically that'd be great. In the meantime, what about allowing \g&
accept a connection number to switch to?

Also, I'd really love it if we could also do ';&'... I didn't mention it
before because I'm assuming it's essentially not possible, but I'd like
to be wrong...
--
Jim Nasby decibel@decibel.org
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

On Mon, May 14, 2007 at 11:03:52AM -0400, Tom Lane wrote:

Gregory Stark <stark@enterprisedb.com> writes:

But these kinds of inconsistent behaviours can be traps for users. It means
"\c1" and "\c 1" do different things even though "\cpostgres" and \c postgres"
do the same thing. And it means "\c1" might connect to a database named "1"
today but switch sessions tomorrow.

The real problem here is trying to overload an existing command name
with too many different meanings. You need to pick some other name
besides \c.

If you were willing to think of it as "switch session" instead of "connect",
then \S is available ...

Since this command will be getting used very frequently by anyone using
concurrent connections interactively, it'd be nice if it was lower-case.
It looks like that limits us to j, k, m, n, v, and y. In unix this idea
is about jobs, what about using \j?
--
Jim Nasby decibel@decibel.org
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

"Jim C. Nasby" <decibel@decibel.org> writes:

Since this command will be getting used very frequently by anyone using
concurrent connections interactively, it'd be nice if it was lower-case.
It looks like that limits us to j, k, m, n, v, and y. In unix this idea
is about jobs, what about using \j?

Well currently it's not really terribly interesting to use interactively since
you could always just start a second shell and run a second instance of psql.
I really only have regression tests in mind for it. That's why I don't find it
a problem at all to only extend \g and not semicolon handling.

That said, I think a next step for this for interactive use would be to handle
C-z to "background" the currently running query. So perhaps it does make sense
to keep use cases like that when deciding on command names now.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

On Mon, May 14, 2007 at 06:26:42PM +0100, Gregory Stark wrote:

"Jim C. Nasby" <decibel@decibel.org> writes:

Since this command will be getting used very frequently by anyone using
concurrent connections interactively, it'd be nice if it was lower-case.
It looks like that limits us to j, k, m, n, v, and y. In unix this idea
is about jobs, what about using \j?

Well currently it's not really terribly interesting to use interactively since
you could always just start a second shell and run a second instance of psql.
I really only have regression tests in mind for it. That's why I don't find it
a problem at all to only extend \g and not semicolon handling.

That said, I think a next step for this for interactive use would be to handle
C-z to "background" the currently running query. So perhaps it does make sense
to keep use cases like that when deciding on command names now.

Yeah, I think having the ability to open up another connection within
psql will turn out to be very useful from an interactive standpoint; \c&
(or whatever command we use to duplicate the current connection) is
going to be a lot easier to enter than actually starting up a new psql
in many production environments.
--
Jim Nasby decibel@decibel.org
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

On Mon, May 14, 2007 at 11:55:07AM -0500, Jim C. Nasby wrote:

On Mon, May 14, 2007 at 11:03:52AM -0400, Tom Lane wrote:

Gregory Stark <stark@enterprisedb.com> writes:

But these kinds of inconsistent behaviours can be traps for users. It means
"\c1" and "\c 1" do different things even though "\cpostgres" and \c postgres"
do the same thing. And it means "\c1" might connect to a database named "1"
today but switch sessions tomorrow.

The real problem here is trying to overload an existing command name
with too many different meanings. You need to pick some other name
besides \c.

If you were willing to think of it as "switch session" instead of "connect",
then \S is available ...

Since this command will be getting used very frequently by anyone using
concurrent connections interactively, it'd be nice if it was lower-case.
It looks like that limits us to j, k, m, n, v, and y. In unix this idea
is about jobs, what about using \j?

I suppose there is some reason the bash/csh job control characters:

%-
%+
%1

won't work?

-dg

--
David Gould daveg@sonic.net
If simplicity worked, the world would be overrun with insects.

So based on the feedback and suggestions here this is the interface I suggest:

\connect& - to open a new connection keeping the existing one
\g& - to submit a command asynchronously (like & in the shell)
\S [Sess#] - to _S_witch to a different _S_ession
- if no connection # specified list available _S_essions
\D - _D_isconnect from current session (like ^D in the shell)

This leaves no way to submit an asynchronous command without using \g but I'm
really not too concerned with that. I don't want to start messing with psql's
semicolon parsing behaviour and I'm mainly only concerned with this for
regression tests.

Another thought I had for the future is a \C command to simulate C-c and send
a query cancel. That would let us have regression tests that query
cancellation worked. The tests would presumably have to be written using
pg_sleep() to ensure they ran for long enough but even then there would be no
way to control exactly when the interrupt arrived.

Attached is an updated patch.

I also found and fixed some missing ResetCancelConn()s. I think I got them all
and the behaviour seems correct in practice when cancelling various
combinations of synchronous queries, asynchronous queries, and backslash
commands. The one thing I wonder about is that I'm a bit concerned I may have
introduced an assumption about how many resultsets arrive from a single query.

I'll be offline for a few days but I'll be back Monday.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

Gregory Stark wrote:

So based on the feedback and suggestions here this is the interface I suggest:

\connect& - to open a new connection keeping the existing one
\g& - to submit a command asynchronously (like & in the shell)
\S [Sess#] - to _S_witch to a different _S_ession
- if no connection # specified list available _S_essions
\D - _D_isconnect from current session (like ^D in the shell)

This leaves no way to submit an asynchronous command without using \g but I'm
really not too concerned with that. I don't want to start messing with psql's
semicolon parsing behaviour and I'm mainly only concerned with this for
regression tests.

Another thought I had for the future is a \C command to simulate C-c and send
a query cancel. That would let us have regression tests that query
cancellation worked. The tests would presumably have to be written using
pg_sleep() to ensure they ran for long enough but even then there would be no
way to control exactly when the interrupt arrived.

Attached is an updated patch.

I also found and fixed some missing ResetCancelConn()s. I think I got them all
and the behaviour seems correct in practice when cancelling various
combinations of synchronous queries, asynchronous queries, and backslash
commands. The one thing I wonder about is that I'm a bit concerned I may have
introduced an assumption about how many resultsets arrive from a single query.

I'll be offline for a few days but I'll be back Monday.

[ Attachment, skipping... ]

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Hi Greg,

I looked at the patch briefly. I couldn't spot any issues and it looks good
to me. I've just couple of comments here.

The mvcc regression test files are missing in the patch.

--- 1179,1189 ----
                              dbname, user, password);

/* We can immediately discard the password -- no longer needed */
! if (password)
! {
! memset(password, '\0', strlen(password));
free(password);
+ }

Any reason why we do this ? "password" is anyways freed. I think you
might have left it behind after some debugging exercise.

--- 25,37 ----
  #include "mb/pg_wchar.h"
  #include "mbprint.h"

+ #if 0
+ #include "libpq-int.h" /* For PG_ASYNC */
+ #endif
+

This looks redundant..

Apart from that I really like consistent coding style. For example, to me,
"for (i = 0; i < 10; i++)" looks much better than "for (i=0;i<10; i++)"

This is not comment on your patch and neither I am saying
we should follow a specific coding style (though I wish we could have done
so) because we have already so many different styles. So its best to
stick to the coding style already followed in that particular file. But few
simple rules like having a single space around operators like '<', '+', '='
etc really makes the code more readable. Other examples are using
parenthesis in a right manner to improve code readability.

flag = (pointer == NULL); is more readable than
flag = pointer == NULL;

Thanks,
Pavan

--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com

Pavan Deolasee wrote:

--- 1179,1189 ----
dbname, user, password);

/* We can immediately discard the password -- no longer needed */
! if (password)
! {
! memset(password, '\0', strlen(password));
free(password);
+ }

Any reason why we do this ? "password" is anyways freed. I think you
might have left it behind after some debugging exercise.

I believe it's for security reasons. If that memory page is for example
swapped to disk after freeing the field, the password would be written
to the swapfile. Someone who steals your laptop would be able to recover
it from there. Clearing passwords from memory when they're no longer
needed is a good practice.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Gregory Stark wrote:

Attached is an updated patch.

This patch appears to add a nonexistent test to the regression schedules.

cheers

andrew

"Andrew Dunstan" <andrew@dunslane.net> writes:

Gregory Stark wrote:

Attached is an updated patch.

This patch appears to add a nonexistent test to the regression schedules.

I must have forgotten to cvs add it. Sorry.

Also, I forgot to mention previously there is an unrelated trivial hunk in
here. I noticed we free the password early, presumably for security reasons,
but don't actually overwrite the memory at that point. I added a memset in
there, otherwise the free seems kind of pointless. I normally don't bother
with "security" features like that since they don't really add any security
but as long as it's there it may as well do something vaguely useful.

Am Montag, 21. Mai 2007 15:21 schrieb Gregory Stark:

Also, I forgot to mention previously there is an unrelated trivial hunk in
here. I noticed we free the password early, presumably for security
reasons,

No, to save memory.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

Gregory Stark wrote:

"Andrew Dunstan" <andrew@dunslane.net> writes:

Gregory Stark wrote:

Attached is an updated patch.

This patch appears to add a nonexistent test to the regression schedules.

I must have forgotten to cvs add it. Sorry.

Also, I forgot to mention previously there is an unrelated trivial hunk in
here. I noticed we free the password early, presumably for security reasons,
but don't actually overwrite the memory at that point. I added a memset in
there, otherwise the free seems kind of pointless. I normally don't bother
with "security" features like that since they don't really add any security
but as long as it's there it may as well do something vaguely useful.

[ Attachment, skipping... ]

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Gregory Stark wrote:

"Andrew Dunstan" <andrew@dunslane.net> writes:

Gregory Stark wrote:

Attached is an updated patch.

This patch appears to add a nonexistent test to the regression schedules.

I must have forgotten to cvs add it. Sorry.

Also, I forgot to mention previously there is an unrelated trivial hunk in
here. I noticed we free the password early, presumably for security reasons,
but don't actually overwrite the memory at that point. I added a memset in
there, otherwise the free seems kind of pointless. I normally don't bother
with "security" features like that since they don't really add any security
but as long as it's there it may as well do something vaguely useful.

Greg,

In general this looks quite reasonable. It's a very large patch for a
feature of this size, but luckily it's mostly changes due to the new
pset structure rather than new code.

There are some places that it doesn't use PG style (e.g. no newline
before brace after if) and some comments that need to be fixed (e.g. /*
XXX get result */ )

If you can fix that I'll apply the patch - I especially want to get it
tested widely via the buildfarm. I am leaving town for about 5 or 6 days
on Friday morning, and my availability will be somewhat restricted
during that time, so if this isn't fixed pronto it will possibly have to
wait till my return or till another reviewer takes pity on you :-)

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

There are some places that it doesn't use PG style (e.g. no newline
before brace after if) and some comments that need to be fixed (e.g. /*
XXX get result */ )

Of course you both realize pgindent will take care of the former ;-).
But yes, bogus comments are worth the trouble to fix now.

regards, tom lane

"Andrew Dunstan" <andrew@dunslane.net> writes:

Greg,

In general this looks quite reasonable. It's a very large patch for a feature
of this size, but luckily it's mostly changes due to the new pset structure
rather than new code.

Really? I expected there would be a few issues raised. For one about the need
to use PG_ASYNC from libpq-int.h.

Perhaps what I should do it split it into two patches, one which just adds
\connect& and \S (switch connection) which will be the larger patch because it
has to break out the connection structure like you mention. And a second which
does the asynchronous response handling which is where there may be some
questions about how to handle things.

There are some places that it doesn't use PG style (e.g. no newline before
brace after if) and some comments that need to be fixed (e.g. /* XXX get result
*/ )

Ah, but it's not just the comment, if I put an XXX in it's definitely because
there's something I'm not certain about. In this case now that I look at it
again I think it's usually ok to ignore the result but in a session with
ON_ERROR_STOP set (such as one running a script) we would want to exit I
think.

Another XXX is next to the include of libpq-int.h and a third one is where I
have the pg_sleep loop instead of a select/poll loop. It occurs to me now that
that loop should check cancel_pressed too.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Gregory Stark wrote:

I expected there would be a few issues raised. For one about the need
to use PG_ASYNC from libpq-int.h.

Hmm, yes. Maybe we need to export that. I also see:

+ #if 0
+ #include "libpq-int.h" /* For PG_ASYNC */
+ #endif
+

which needs to disappear.

If we're going to include libpq-int.h maybe we need to put it in
common.h. Is there a reason that our own client programs shouldn't use
our own library internals?

Perhaps what I should do it split it into two patches, one which just adds
\connect& and \S (switch connection) which will be the larger patch because it
has to break out the connection structure like you mention. And a second which
does the asynchronous response handling which is where there may be some
questions about how to handle things.

I don't think that's necessary.

There are some places that it doesn't use PG style (e.g. no newline before
brace after if) and some comments that need to be fixed (e.g. /* XXX get result
*/ )

Ah, but it's not just the comment, if I put an XXX in it's definitely because
there's something I'm not certain about. In this case now that I look at it
again I think it's usually ok to ignore the result but in a session with
ON_ERROR_STOP set (such as one running a script) we would want to exit I
think.

Seems fair.

Another XXX is next to the include of libpq-int.h and a third one is where I
have the pg_sleep loop instead of a select/poll loop. It occurs to me now that
that loop should check cancel_pressed too.

Are you talking about polling the connection? Using select/poll except
on a socket is forbidden, because it breaks on Windows.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

If we're going to include libpq-int.h maybe we need to put it in
common.h. Is there a reason that our own client programs shouldn't use
our own library internals?

Seems like a really bad idea to me. I know I've cursed mysql more than
once for doing the equivalent. Also, if psql needs more than is
currently exported as official API, why wouldn't other programs need it
too? If there's more needed here, let's see an official API change,
not a hack.

regards, tom lane

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

If we're going to include libpq-int.h maybe we need to put it in
common.h. Is there a reason that our own client programs shouldn't use
our own library internals?

Seems like a really bad idea to me. I know I've cursed mysql more than
once for doing the equivalent. Also, if psql needs more than is
currently exported as official API, why wouldn't other programs need it
too? If there's more needed here, let's see an official API change,
not a hack.

Well, I guess the obvious API would be something like:

PGAsyncStatusType PQasyncStatus(const PGconn *conn);

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

Tom Lane wrote:

If there's more needed here, let's see an official API change,
not a hack.

Well, I guess the obvious API would be something like:
PGAsyncStatusType PQasyncStatus(const PGconn *conn);

That would mean exposing PGAsyncStatusType, which doesn't seem like an
amazingly good idea either. What is it that the patch actually wants
to do?

regards, tom lane

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Tom Lane wrote:

If there's more needed here, let's see an official API change,
not a hack.

Well, I guess the obvious API would be something like:
PGAsyncStatusType PQasyncStatus(const PGconn *conn);

That would mean exposing PGAsyncStatusType, which doesn't seem like an
amazingly good idea either. What is it that the patch actually wants
to do?

The relevant snippet in command.c says:

/* If we have async_delay set then we need to allow up to that many
* milliseconds for any responses to come in on *either* connection.
* This ensures that results are printed in a relatively deterministic
* order for regression tests. Note that we only have to allow for n
* milliseconds total between the two connections so we don't reset the
* timer for the second wait.
*
* XXX this should maybe be changed to a select/poll loop instead
*/
if (pset.async_delay > 0 && pset.c->db)
{
GETTIMEOFDAY(&start);
do {
if (pset.c->db->asyncStatus != PGASYNC_BUSY)
{
break;
}
if (CheckQueryResults())
{
ReadQueryResults();
break;
}
pg_usleep(10000);
GETTIMEOFDAY(&now);
} while (DIFF_MSEC(&now, &start) < pset.async_delay);
}

pset.c = &cset[slot-1];
printf(_("[%d] You are now connected to database \"%s\"\n"), slot,
PQdb(pset.c->db));

if (pset.async_delay > 0 && pset.c->db)
{
do {
if (pset.c->db->asyncStatus != PGASYNC_BUSY)
break;
if (CheckQueryResults()) {
ReadQueryResults();
break;
}
pg_usleep(10000);
GETTIMEOFDAY(&now);
} while (DIFF_MSEC(&now, &start) < pset.async_delay);
}

and in mainloop.c it's used like this:

if (pset.c->db && pset.c->db->asyncStatus != PGASYNC_IDLE &&
CheckQueryResults()) {
ReadQueryResults();
/* If we processed a query cancellation cancel_pressed may
still be
* set and will interrupt the processing of the next command
unless
* we start the main loop over to reset it. */
if (cancel_pressed)
break;
}

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

if (pset.c->db->asyncStatus != PGASYNC_BUSY)
{
break;
}

There already is a defined API for this, namely PQisBusy().

In any case, I rather concur with the XXX comment: busy-waiting like
this sucks. The correct way to do this is to get the socket numbers for
the connections (via PQsocket), wait for any of them to be read-ready
according to select() (or for the timeout to elapse, assuming that we
think that behavior is good), then cycle through PQconsumeInput() and
PQisBusy() on each connection. See
http://www.postgresql.org/docs/8.2/static/libpq-async.html

regards, tom lane

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

if (pset.c->db->asyncStatus != PGASYNC_BUSY)
{
break;
}

There already is a defined API for this, namely PQisBusy().

In any case, I rather concur with the XXX comment: busy-waiting like
this sucks. The correct way to do this is to get the socket numbers for
the connections (via PQsocket), wait for any of them to be read-ready
according to select() (or for the timeout to elapse, assuming that we
think that behavior is good), then cycle through PQconsumeInput() and
PQisBusy() on each connection. See
http://www.postgresql.org/docs/8.2/static/libpq-async.html

In that case I guess Greg has some work to do :-) . Looks like there
are about five such calls in toto, so it's not a huge tragedy.

cheers

andrew

"Andrew Dunstan" <andrew@dunslane.net> writes:

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

if (pset.c->db->asyncStatus != PGASYNC_BUSY)
{
break;
}

There already is a defined API for this, namely PQisBusy().

Oh, now I remember why I'm including libpq-int.h. It's not for PGASYNC_BUSY as
above. The case above can indeed be fixed using PQIsBusy() (and select/poll
looping).

The missing interface is for PGASYNC_IDLE. The problem is that I didn't really
want to have psql go through the trouble to check all the connections for
waiting output if it didn't have any queries pending.

This could be fixed by having psql track which connections are waiting for
query results. It's a bit annoying to have two state bits that hold the same
data at two different levels of abstraction though.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

"Tom Lane" <tgl@sss.pgh.pa.us> writes:

Andrew Dunstan <andrew@dunslane.net> writes:

if (pset.c->db->asyncStatus != PGASYNC_BUSY)
{
break;
}

There already is a defined API for this, namely PQisBusy().

In any case, I rather concur with the XXX comment: busy-waiting like
this sucks. The correct way to do this is to get the socket numbers for
the connections (via PQsocket), wait for any of them to be read-ready
according to select() (or for the timeout to elapse, assuming that we
think that behavior is good), then cycle through PQconsumeInput() and
PQisBusy() on each connection. See
http://www.postgresql.org/docs/8.2/static/libpq-async.html

Huh, so it turns out we already have code that does exactly this in
pqSocketPoll and pqSocketCheck. Except that they have too little resolution
because they work with time_t which means we would have to wait at least 1-2
seconds.

And pqSocketCheck keeps looping when it gets an EINTR which doesn't seem like
the right thing for psql to do.

It would be nice to use these functions though because:

a) They get the SSL case right in that that they check the SSL buffer before
calling select/poll.

b) They use poll if available and fall back to select

c) they would keep the select/poll system code out of psql where there's none
of it currently.

So would I be better off adding a PQSocketPollms() which works in milliseconds
instead of seconds? Or should I just copy all this code into psql?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Gregory Stark wrote:

"Andrew Dunstan" <andrew@dunslane.net> writes:

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

if (pset.c->db->asyncStatus != PGASYNC_BUSY)
{
break;
}

There already is a defined API for this, namely PQisBusy().

Oh, now I remember why I'm including libpq-int.h. It's not for PGASYNC_BUSY as
above. The case above can indeed be fixed using PQIsBusy() (and select/poll
looping).

The missing interface is for PGASYNC_IDLE. The problem is that I didn't really
want to have psql go through the trouble to check all the connections for
waiting output if it didn't have any queries pending.

This could be fixed by having psql track which connections are waiting for
query results. It's a bit annoying to have two state bits that hold the same
data at two different levels of abstraction though.

Unless you have a better solution I think we'd better do that, though.

I notice that the header is also included in command.c even though it
doesn't use PGASYNC_IDLE.

Are you going to make these fixes?

cheers

andrew

"Andrew Dunstan" <andrew@dunslane.net> writes:

Unless you have a better solution I think we'd better do that, though.

I notice that the header is also included in command.c even though it doesn't
use PGASYNC_IDLE.

Are you going to make these fixes?

Yes, sorry, I started to already but got distracted by some tests I've been running.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Author unresponsive to request for updated patch.

This patch has been saved for the 8.4 release:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Gregory Stark wrote:

"Andrew Dunstan" <andrew@dunslane.net> writes:

Unless you have a better solution I think we'd better do that, though.

I notice that the header is also included in command.c even though it doesn't
use PGASYNC_IDLE.

Are you going to make these fixes?

Yes, sorry, I started to already but got distracted by some tests I've been running.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

[ redirecting to -hackers since -patches isn't the place for general
discussion of feature specifications ]

Gregory Stark <stark@enterprisedb.com> writes:

So based on the feedback and suggestions here this is the interface I suggest:

\connect& - to open a new connection keeping the existing one
\g& - to submit a command asynchronously (like & in the shell)
\S [Sess#] - to _S_witch to a different _S_ession
- if no connection # specified list available _S_essions
\D - _D_isconnect from current session (like ^D in the shell)

This is still the latest API suggestion for concurrent psql, right?

After reflecting on it for awhile it seems to me that the use of
automatically assigned numbers as connection IDs is kind of a wart.
It makes it difficult if not impossible to write context-insensitive
script fragments, and even for interactive use it doesn't seem
especially convenient. How about naming connections with user-assigned
strings, instead, eg

\connect& name [ optional connect params ]
\S name

This would require choosing a name for the default session, maybe "-".
Or you could use "1" if you figured that people really would prefer
numbers as IDs.

I'm not real thrilled with overloading \S with two fundamentally
different behaviors, either. Can't we find a different string to assign
to the listing purpose? Maybe \S without parameter should mean to
switch to the default session.

Another thought I had for the future is a \C command to simulate C-c and send
a query cancel. That would let us have regression tests that query
cancellation worked.

Do we really need a regression test for that? I notice \C is already
taken. In general the space of backslash command names is taken up
densely enough that eating single-letter names for marginal functions
doesn't seem wise. (I also question giving \D a single-letter name.)

But the part of the API that really seems like a wart is

+       <varlistentry>
+         <term><varname>ASYNC_DELAY</varname></term>
+         <listitem>
+         <para>
+         Wait up to this period of time (in milliseconds) for output prior to
+         any connection switch. If no asynchronous command is pending or if any
+         output arrives <application>psql</> may not wait the full specified
+         time.
+         </para>

There is no way to select a correct value for ASYNC_DELAY --- any value
you might pick could be too small if the machine is heavily loaded.
In any case for safety's sake you'd need to pick values much larger than
(you think) are really needed, which is not cool for something we hope to
use in regression tests. Those of us who routinely run the tests many
times a day will scream pretty loudly if they start spending most of
their time waiting --- and the prospect of random failures on heavily
loaded buildfarm members is not appetizing either.

What seems possibly more useful is to reintroduce \cwait (or hopefully
some better name) and give it the semantics of "wait for a response from
any active connection; switch to the first one to respond, printing its
name, and print its result".

This would lead to code like, say,

\c& conn1
\c& conn2
...
\S conn1
CREATE INDEX ... \g&
\S conn2
CREATE INDEX ... \g&
...
\cwait
\cwait

The number of \cwaits you need is exactly equal to the number of
async commands you've issued. For regression testing purposes
you'd need to design the script to ensure that only one of the
connections is expected to respond next, but that seems necessary
anyway --- and you don't need any extra checks to catch the case
that you get an unexpected early response from another one.

Hmm, this still seems a bit notation-heavy, doesn't it? What if \g&
takes an arg indicating which connection to issue the command on:

\c& conn1
\c& conn2
...
CREATE INDEX ... \g& conn1
CREATE INDEX ... \g& conn2
...
\cwait
\cwait

Not totally sure about that one, but issuing a command on a background
connection seems appealing for scripting purposes. It eliminates the
risk that the query response comes back before you manage to switch away
from the connection; which would be bad because it would mess up your
count of how many cwait's you need. It seems a bit more analogous to
the use of & in shell scripts, too, where you implicitly fork away from
the async command. (Maybe c& shouldn't make the new connection
foreground either?)

regards, tom lane

I wrote:

What seems possibly more useful is to reintroduce \cwait (or hopefully
some better name) and give it the semantics of "wait for a response from
any active connection; switch to the first one to respond, printing its
name, and print its result".

It strikes me that with these semantics, \cwait is a lot like a thread
join operation, so we could call it \join or \j.

regards, tom lane

Tom Lane wrote:

I wrote:

What seems possibly more useful is to reintroduce \cwait (or hopefully
some better name) and give it the semantics of "wait for a response from
any active connection; switch to the first one to respond, printing its
name, and print its result".

It strikes me that with these semantics, \cwait is a lot like a thread
join operation, so we could call it \join or \j.

FWIW on POSIX shell there's something similar called "wait".

http://www.opengroup.org/onlinepubs/009695399/utilities/wait.html

Perhaps we should define the operator after these semantics -- these
guys have probably hashed up a good interface. Basically it means we
would have a "\cwait [n ...]" command meaning "wait for the connection
'n' to return".

If we do that, we can then have multiple commands in flight on
regression tests, and wait for them in whatever deterministic order we
choose, regardless of which one finishes execution first.

However, the no-operands version of POSIX wait means "wait for all
commands" instead of "wait for any command". Perhaps we could have
"\cwait -" as meaning "wait for any command".

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre@commandprompt.com> writes:

Tom Lane wrote:

It strikes me that with these semantics, \cwait is a lot like a thread
join operation, so we could call it \join or \j.

FWIW on POSIX shell there's something similar called "wait".
http://www.opengroup.org/onlinepubs/009695399/utilities/wait.html
Perhaps we should define the operator after these semantics -- these
guys have probably hashed up a good interface. Basically it means we
would have a "\cwait [n ...]" command meaning "wait for the connection
'n' to return".

I was thinking about this some more while out running an errand, and
came to the same conclusion that "\cwait connID" would be a good thing
to have.

However, the no-operands version of POSIX wait means "wait for all
commands" instead of "wait for any command". Perhaps we could have
"\cwait -" as meaning "wait for any command".

That would require prohibiting "-" as a connection ID, but maybe that's
an OK price for acting like a known standard.

Another thought that came to me while driving around is that it seems
bogus to offer a prompt when attached to a connection that can't
actually accept a command right now. I know that psql can get into that
state after a connection dies, but it's still a wart, and not really
something we should design into normal operations. Furthermore, I don't
see the reason for switching to a connection with an active async
command unless you are desiring to wait for that command's result.
So I'm thinking we could unify \S with \cwait. This leads to the
following proposals:

sql-command \g& connID

If connID is idle and not the current connection, issue
sql-command on it, but do *not* switch to that connection.
(There are various possibilities on what to do in the
corner cases where it's busy or the current connection.
If it's busy, we could throw error, or do a forced \join
before issuing the command. If it's the current connection,
my inclination is to treat this exactly like \g, ie wait
for the result.)
Also, if connID is not a known ID, we could automatically
create it as a clone of the current connection; which'd
eliminate the need for explicit \connect& in many cases.
(OTOH that might be too vulnerable to typos.)

\join connID

Switch to connection connID. If it is busy, wait for
command completion and print the result before offering
a new command prompt.

\join (or \join - as per Alvaro)

Wait for any currently busy connection's command to finish,
then \join to it. Error if there is no busy connection.

While there's still a possible use for \D (disconnect) in this
scheme, I'm not sure how interesting it is. In any case disconnecting
the active session is a bogus behavior; you should only be able
to disconnect a non-active, idle one.

regards, tom lane

This has been saved for the next commit-fest:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Gregory Stark wrote:

"Andrew Dunstan" <andrew@dunslane.net> writes:

Gregory Stark wrote:

Attached is an updated patch.

This patch appears to add a nonexistent test to the regression schedules.

I must have forgotten to cvs add it. Sorry.

Also, I forgot to mention previously there is an unrelated trivial hunk in
here. I noticed we free the password early, presumably for security reasons,
but don't actually overwrite the memory at that point. I added a memset in
there, otherwise the free seems kind of pointless. I normally don't bother
with "security" features like that since they don't really add any security
but as long as it's there it may as well do something vaguely useful.

[ Attachment, skipping... ]

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:

This has been saved for the next commit-fest:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold

Er, why "saved"? Until there's a new patch submission there's not going
to be more work to do on this in the next fest.

I think maybe you need to think a bit harder about the distinction
between your TODO-items-in-waiting list and the commit fest queue.
I was willing to wade through a pile of TODO-items-in-waiting this
time, because I pressed you to make the list available before having
sorted through it; but I'm not going to be pleased to see those same
threads in the fest queue next time, unless someone's done some actual
work in between.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

This has been saved for the next commit-fest:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold

Er, why "saved"? Until there's a new patch submission there's not going
to be more work to do on this in the next fest.

I think maybe you need to think a bit harder about the distinction
between your TODO-items-in-waiting list and the commit fest queue.
I was willing to wade through a pile of TODO-items-in-waiting this
time, because I pressed you to make the list available before having
sorted through it; but I'm not going to be pleased to see those same
threads in the fest queue next time, unless someone's done some actual
work in between.

It is in the next fest so I will remember to ask if people have done any
work on them --- if not they are either deleted or moved to the next
commit fest.

Are you suggesting we just delete the threads and let them die if they
don't submit a new version?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:

Are you suggesting we just delete the threads and let them die if they
don't submit a new version?

I am suggesting that they are not material for another commit fest until
some new work has been done. (And the appearance of that new work would
trigger an entry being made in the pending-commit-fest list.)

I've surely got no objection to you hanging on to such threads in your
personal almost-TODO list, and prodding people when appropriate. But
the commit fest queue is not for that purpose.

regards, tom lane

Bruce Momjian wrote:

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

This has been saved for the next commit-fest:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold

Er, why "saved"? Until there's a new patch submission there's not going
to be more work to do on this in the next fest.

I think maybe you need to think a bit harder about the distinction
between your TODO-items-in-waiting list and the commit fest queue.
I was willing to wade through a pile of TODO-items-in-waiting this
time, because I pressed you to make the list available before having
sorted through it; but I'm not going to be pleased to see those same
threads in the fest queue next time, unless someone's done some actual
work in between.

It is in the next fest so I will remember to ask if people have done any
work on them --- if not they are either deleted or moved to the next
commit fest.

Are you suggesting we just delete the threads and let them die if they
don't submit a new version?

My understanding was that all items in a commit-fest have one of these
three dispositions:

. committed
. rejected
. referred back to author for more work

We're really only interested in the third one here, and so, yes, the
ball should be in the author's court, not yours. I don't see any reason
for you to move items from one queue to another like that. It just looks
like it's making work.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

My understanding was that all items in a commit-fest have one of these
three dispositions:

. committed
. rejected
. referred back to author for more work

Right. But Bruce's personal queue has got a different lifecycle:
items get removed when they are resolved by a committed patch, or
by being rejected as not wanted, or by being summarized on the public
TODO list. For what he's doing that's a very good definition ---
things don't get forgotten just because nothing has happened lately.
But it's becoming clearer to me that the commit-fest queue has to be
a separate animal. We used Bruce's queue as the base this time around,
because we had no other timely-available source of the raw data.
Seems like it's time to split them, though.

If we do split them then there is going to be some added effort to
maintain the commit fest queue. Bruce has made it pretty clear
that he doesn't want to put in any extra cycles here. So someone
else has to step up to the plate if this is going to work.
Any volunteers out there?

regards, tom lane

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

My understanding was that all items in a commit-fest have one of these
three dispositions:

. committed
. rejected
. referred back to author for more work

Right. But Bruce's personal queue has got a different lifecycle:
items get removed when they are resolved by a committed patch, or
by being rejected as not wanted, or by being summarized on the public
TODO list. For what he's doing that's a very good definition ---
things don't get forgotten just because nothing has happened lately.
But it's becoming clearer to me that the commit-fest queue has to be
a separate animal. We used Bruce's queue as the base this time around,
because we had no other timely-available source of the raw data.
Seems like it's time to split them, though.

Right, if the patch author stops working on it, but it is a feature we
want, the thread goes on the TODO list (or we complete the patch), so
yes, it is a different life-cycle.

If we do split them then there is going to be some added effort to
maintain the commit fest queue. Bruce has made it pretty clear
that he doesn't want to put in any extra cycles here. So someone
else has to step up to the plate if this is going to work.
Any volunteers out there?

I assumed the wiki was going to be the official patch list from now on
and my web pages were just going to be a public display of things I was
tracking.

Frankly, I haven't been putting anything on the queue for the next
commit fest now except stuff that was already in-process for this commit
fest. The ideas is that we can commit stuff that has appeared since the
commit fest started before the next commit fest starts. I also moved
the emails to the next commit fest queue because that preserves the
comments made too.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Andrew Dunstan wrote:

Are you suggesting we just delete the threads and let them die if they
don't submit a new version?

My understanding was that all items in a commit-fest have one of these
three dispositions:

. committed
. rejected
. referred back to author for more work

We're really only interested in the third one here, and so, yes, the
ball should be in the author's court, not yours. I don't see any reason
for you to move items from one queue to another like that. It just looks
like it's making work.

True. I could move the emails back to my private mailbox and just track
them there too. I moved them so it would be visible we were waiting for
some people.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:

\connect& name [ optional connect params ]
\S name

This would require choosing a name for the default session, maybe "-".
Or you could use "1" if you figured that people really would prefer
numbers as IDs.

+1 with name as a string, when an empty string is passed a numerical
sequence is used as default.

I'm not real thrilled with overloading \S with two fundamentally
different behaviors, either. Can't we find a different string to assign
to the listing purpose? Maybe \S without parameter should mean to
switch to the default session.

I think it seems fine. Fits with \h and \d behaviour.

Hmm, this still seems a bit notation-heavy, doesn't it? What if \g&
takes an arg indicating which connection to issue the command on:

\c& conn1
\c& conn2
...
CREATE INDEX ... \g& conn1
CREATE INDEX ... \g& conn2
...
\cwait
\cwait

+1 on the \g& but I would reverse the syntax -

\g& conn1 CERATE INDEX...;

Not totally sure about that one, but issuing a command on a background
connection seems appealing for scripting purposes. It eliminates the
risk that the query response comes back before you manage to switch away
from the connection; which would be bad because it would mess up your
count of how many cwait's you need. It seems a bit more analogous to
the use of & in shell scripts, too, where you implicitly fork away from
the async command. (Maybe c& shouldn't make the new connection
foreground either?)

\c& for a new foreground connection
\cb& for a new background connection?

--

Shane Ambler
pgSQL (at) Sheeky (dot) Biz

Get Sheeky @ http://Sheeky.Biz

Tom Lane wrote:

\join connID

Switch to connection connID. If it is busy, wait for
command completion and print the result before offering
a new command prompt.

When switching to a conn we also need a non-destructive way out if it is
busy.

\join (or \join - as per Alvaro)

Wait for any currently busy connection's command to finish,
then \join to it. Error if there is no busy connection.

So what you suggest is that if you have 10 busy conns running \join will
send you to the next conn to return a result?

On that - listing the current conns could be useful to have some status
info with the list to indicate idle or running what command.

--

Shane Ambler
pgSQL (at) Sheeky (dot) Biz

Get Sheeky @ http://Sheeky.Biz

Shane Ambler <pgsql@Sheeky.Biz> writes:

+1 on the \g& but I would reverse the syntax -
\g& conn1 CERATE INDEX...;

No, not good. If the command requires multiple lines then this creates
an action-at-a-distance behavior. Thought experiment: what would you
expect here:

\g& conn1
CREATE INDEX z (<oops, made a mistake>
\r
CREATE INDEX q ...;

And whichever behavior you'd "expect", how would you get the other
one when you needed it? Hidden state sucks.

(Yeah, this argument probably appeals to people who like RPN calculators
more than those who don't...)

psql's established behavior is that \g is issued after the command
it affects, and we should not change that.

regards, tom lane

Shane Ambler <pgsql@Sheeky.Biz> writes:

When switching to a conn we also need a non-destructive way out if it is
busy.

Uh, why? Why would you switch to a connection at all, if you didn't
want its result?

This is a pretty fundamental issue, and insisting that you want that
behavior will make both the user's mental model and the implementation
a whole lot more complex. I'm not going to accept unsupported arguments
that it might be a nice thing to have.

So what you suggest is that if you have 10 busy conns running \join will
send you to the next conn to return a result?

Right.

On that - listing the current conns could be useful to have some status
info with the list to indicate idle or running what command.

Sure, some status-inquiry commands could be added without fundamentally
affecting anything.

regards, tom lane

"Tom Lane" <tgl@sss.pgh.pa.us> writes:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Tom Lane wrote:

It strikes me that with these semantics, \cwait is a lot like a thread
join operation, so we could call it \join or \j.

FWIW on POSIX shell there's something similar called "wait".
http://www.opengroup.org/onlinepubs/009695399/utilities/wait.html
Perhaps we should define the operator after these semantics -- these
guys have probably hashed up a good interface. Basically it means we
would have a "\cwait [n ...]" command meaning "wait for the connection
'n' to return".

I was thinking about this some more while out running an errand, and
came to the same conclusion that "\cwait connID" would be a good thing
to have.

I threw out cwait because it seemed to me that to write any kind of reliable
regression test you would end up having to put a cwait with a timeout on every
connection switch.

Consider a simple regression test to test that update locks out concurrent
updaters:

1 begin;
1 update t where i=1
UPDATE 1
<switch to connection 2>
2 begin;
2 update t where i=1
<switch to connection 2>
2 commit;
COMMIT
<switch to connection 1>
UPDATE 1

So here what you really want to test is that the second update blocks. If we
don't wait at all we might very well miss the UPDATE message because we just
flew past it too fast. In fact IIRC that's exactly what I saw.

While there's still a possible use for \D (disconnect) in this
scheme, I'm not sure how interesting it is. In any case disconnecting
the active session is a bogus behavior; you should only be able
to disconnect a non-active, idle one.

Unless you're specifically trying to test that things get cleaned up properly
when the session rolls back... But yeah, I only put it in for the sake of
completeness at the time.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training!

Tom Lane wrote:

Shane Ambler <pgsql@Sheeky.Biz> writes:

When switching to a conn we also need a non-destructive way out if it is
busy.

Uh, why? Why would you switch to a connection at all, if you didn't
want its result?

What if you switch to the wrong connection and it hasn't finished. Do
you then have to wait until you get the results before you can issue
another command? Or will we be able to type commands while we wait for
results?

I am thinking as currently happens - you can't type a command as you are
waiting for a result. So if the connection you switch to is busy but you
want to go to another connection then how do you?

This may tie into an 'auto new connection'. You start psql enter a
command that will take a while then think of something else you can do
as you wait. Do you open another shell and start psql again, or send the
working task to the background and enter another command in a new
connection?

Think jobs in a shell, you can suspend a long running process then send
it to the background to work and go on with something else.

So I am thinking something like C-z that will allow you to switch out of
a task that is waiting for results without having to stop it with C-c.

--

Shane Ambler
pgSQL (at) Sheeky (dot) Biz

Get Sheeky @ http://Sheeky.Biz

Shane Ambler wrote:

Think jobs in a shell, you can suspend a long running process then send
it to the background to work and go on with something else.

So I am thinking something like C-z that will allow you to switch out of
a task that is waiting for results without having to stop it with C-c.

I agree -- we would need to have a mode on which it is "not on any
connection", to which we could switch on C-z. If all connections are
busy, there's no way to create a new one otherwise.

It makes sense if we continue with the shell analogy: the shell prompt
is not any particular task. Either there is a task running in
foreground (in which case we have no prompt, but we can press C-z to
suspend the current task and get a prompt), or there isn't (in which
case we have a prompt.)

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera <alvherre@commandprompt.com> writes:

Shane Ambler wrote:

So I am thinking something like C-z that will allow you to switch out of
a task that is waiting for results without having to stop it with C-c.

I agree -- we would need to have a mode on which it is "not on any
connection", to which we could switch on C-z. If all connections are
busy, there's no way to create a new one otherwise.

That would work okay for interactive use and not at all for scripts,
which makes it kind of a nonstarter. I'm far from convinced that the
case must be handled anyway. If you fat-finger a SQL command the
consequences are likely to be far worse than having to wait a bit,
so why is it so critical to be able to recover from a typo in a \join
argument?

(I'm also unconvinced that there won't be severe implementation
difficulties in supporting a control-Z-like interrupt --- we don't have
any terminal signals left to use AFAIK. And what about Windows?)

It makes sense if we continue with the shell analogy: the shell prompt
is not any particular task. Either there is a task running in
foreground (in which case we have no prompt, but we can press C-z to
suspend the current task and get a prompt), or there isn't (in which
case we have a prompt.)

This is nonsense. When you have a shell prompt, you are connected to a
shell that will take a command right now.

regards, tom lane

On Apr 9, 2008, at 12:27 PM, Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Shane Ambler wrote:

So I am thinking something like C-z that will allow you to switch
out of
a task that is waiting for results without having to stop it with
C-c.

I agree -- we would need to have a mode on which it is "not on any
connection", to which we could switch on C-z. If all connections are
busy, there's no way to create a new one otherwise.

That would work okay for interactive use and not at all for scripts,
which makes it kind of a nonstarter.

I can't see any need to do this in a script, and in fact I don't
think shell scripting supports it. Totally different story for
interactive use. Anyone using *nix is likely to be familiar with how
job control works in shells and expecting psql to work the same way.
We should try and follow the shell standard as much as possible just
so that people don't have to re-train themselves.

I'm far from convinced that the
case must be handled anyway. If you fat-finger a SQL command the
consequences are likely to be far worse than having to wait a bit,
so why is it so critical to be able to recover from a typo in a \join
argument?

I find myself doing this frequently with any long-running command,
but currently it's a PITA because I'd doing it at the shell level and
firing up a new psql: more work than should be necessary, and psql
sometimes gets confused when you resume it from the background in
interactive mode (stops echoing characters, though maybe this has
been fixed).

(I'm also unconvinced that there won't be severe implementation
difficulties in supporting a control-Z-like interrupt --- we don't
have
any terminal signals left to use AFAIK. And what about Windows?)

That might be true. I don't know if we could use ^z anyway; the shell
might have different ideas there.

It makes sense if we continue with the shell analogy: the shell
prompt
is not any particular task. Either there is a task running in
foreground (in which case we have no prompt, but we can press C-z to
suspend the current task and get a prompt), or there isn't (in which
case we have a prompt.)

This is nonsense. When you have a shell prompt, you are connected
to a
shell that will take a command right now.

You're always connected to the shell, but if you background something
in the shell it becomes a stand-alone job that you're not connected
to. You could even think of it as every command you run being a job,
it's just a question of if you're actually connected to it or not.
--
Decibel!, aka Jim C. Nasby, Database Architect decibel@decibel.org
Give your computer some brain candy! www.distributed.net Team #1828

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Shane Ambler wrote:

So I am thinking something like C-z that will allow you to switch out of
a task that is waiting for results without having to stop it with C-c.

I agree -- we would need to have a mode on which it is "not on any
connection", to which we could switch on C-z. If all connections are
busy, there's no way to create a new one otherwise.

That would work okay for interactive use and not at all for scripts,
which makes it kind of a nonstarter. I'm far from convinced that the
case must be handled anyway. If you fat-finger a SQL command the
consequences are likely to be far worse than having to wait a bit,
so why is it so critical to be able to recover from a typo in a \join
argument?

I can see that a non-connected prompt would interfere with a script but
I would think that a prompt should always be linked to a connection. It
may work to get an un-connected prompt made available from C-z which
could be limited to only allow new connections or \join commands which
would also be limited to interactive input.

My first thoughts where that C-z would either drop back to the previous
connection or create a new connection either based on the initial login
or the connection you are C-z'ing out of. This would be the tricky
decider though which may make a limited prompt viable.

C-z input detection may also be limited to the wait for query response
loop so that it is only available if the current connection is without a
prompt.

I do think it is useful for more than typo's in the \join command. What
about a slip where you forget to \g& the command. Or you start a query
that seems to be taking too long, background it and look into what is
happening. This would be more helpful to those that ssh into a machine
then run psql from there.

(I'm also unconvinced that there won't be severe implementation
difficulties in supporting a control-Z-like interrupt --- we don't have
any terminal signals left to use AFAIK. And what about Windows?)

That may be so and could be the decider over whether this can be added
or not.

Unless Windows steals the input before psql gets it I don't see there
will be a problem there. Windows may be a factor in deciding which key
to use for this command if it is to be uniform across platforms.

--

Shane Ambler
pgSQL (at) Sheeky (dot) Biz

Get Sheeky @ http://Sheeky.Biz

On Thu, 2008-04-10 at 05:03 +0930, Shane Ambler wrote:

I do think it is useful for more than typo's in the \join command. What
about a slip where you forget to \g& the command. Or you start a query
that seems to be taking too long, background it and look into what is
happening. This would be more helpful to those that ssh into a machine
then run psql from there.

For interactive use in the above mentioned scenario you can use the
'screen' command and start as many psqls as needed ('man screen' to see
what it can do). I would probably always use screen instead of psql's
multisession capability in interactive use. I do want to instantly see
what is currently running, and a psql screen cluttered with multiple
results will not make that easier. Even a list method of what is running
will only help if it actually shows the complete SQL for all running
sessions and that will be a PITA if the SQLs are many and big. Multiple
screens are much better at that.

So from my POV scripting should be the main case for such a feature...
and there it would be welcome if it would be made easy to synchronize
the different sessions.

Cheers,
Csaba.

I find myself doing this frequently with any long-running command,
but currently it's a PITA because I'd doing it at the shell level and
firing up a new psql: more work than should be necessary, and psql
sometimes gets confused when you resume it from the background in
interactive mode (stops echoing characters, though maybe this has
been fixed).

I would recommend trying out the 'screen' utility (see my other post
too). And here you find a nice .screenrc too which will show you a
status bar of your active session, I find it super cool (and it's well
commented if you don't like it as it is):

http://home.insightbb.com/~bmsims1/Scripts/Screenrc.html

The man page has all commands you need, the most used by me:

Ctrl-a Ctrl-c -> open a new session;
Ctrl-a A -> name the session 8will show up with that name in the status
bar, note that the second key is a capital A not a);
Ctrl-a Ctrl-a -> switch to the last viewed session;
Ctrl-a <n> -> switch to the <n>th session, where <n> is a digit 0-9

I usually leave the screen sessions running end detach only the
terminal, and then I can connect again to the already set up sessions
using "screen -R". It's a real time saver.

It has many more facilities, and creating a new psql session is just
Ctrl-a Ctrl-c and then type in psql... and you're good to go... I don't
think you can beat that by a large margin with psql-intern commands (you
still need to type in something extra), and you do have added benefits
of clearly separated workflows and a nice overview of it.

Cheers,
Csaba.

"Csaba Nagy" <nagy@ecircle-ag.com> writes:

For interactive use in the above mentioned scenario you can use the
'screen' command and start as many psqls as needed

Sure, or you could just start multiple xterms or emacs shell buffers
(my preferred setup).

But I'm sure there are people who would prefer C-z too.

So from my POV scripting should be the main case for such a feature...
and there it would be welcome if it would be made easy to synchronize
the different sessions.

I think it's the main case, that's why I didn't implement C-z at all. But I
think we should keep it as a design consideration and not preclude it in the
future.

Hm. I had a thought though. Perhaps C-z should just immediately start a new
connection. That would perhaps maintain the shell metaphor the way Tom was
thinking where you're always at a usable prompt. That might suck if you're at
a password-authenticated connection.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!

So, Greg, after all this feedback, are you going to rework the patch?

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

"Alvaro Herrera" <alvherre@commandprompt.com> writes:

So, Greg, after all this feedback, are you going to rework the patch?

I'm a bit busy now but yes, eventually.

I had in mind that it would probably make sense to start over, stealing code
as appropriate. The main thing is that the logic is a bit twisted now since I
originally had it as a prefix command you gave before issuing the sql. As a
postfix command, \g&, the logic could be a bit simpler.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's Slony Replication support!

Gregory Stark <stark@enterprisedb.com> writes:

"Csaba Nagy" <nagy@ecircle-ag.com> writes:

For interactive use in the above mentioned scenario you can use the
'screen' command and start as many psqls as needed

Sure, or you could just start multiple xterms or emacs shell buffers
(my preferred setup).

Yeah, that's an awfully good point, and I have to admit I'd generally
prefer multiple xterms too.

But I'm sure there are people who would prefer C-z too.

AFAICT, supporting C-z will add a pretty significant increment of
definitional complexity, implementation complexity, and portability
risks to what otherwise could be a relatively small patch. I don't
want to buy into that just because "some people might use it".

I note also that if we start trapping C-z, it would stop working
for what it works for now, namely suspending psql so you can do
something else in that window.

So, +1 for thinking about this entirely as a scripting feature.

regards, tom lane

On Tue, 2008-04-08 at 17:10 -0400, Tom Lane wrote:

What seems possibly more useful is to reintroduce \cwait (or hopefully
some better name) and give it the semantics of "wait for a response from
any active connection; switch to the first one to respond, printing its
name, and print its result".

This would lead to code like, say,

\c& conn1
\c& conn2
...
\S conn1
CREATE INDEX ... \g&
\S conn2
CREATE INDEX ... \g&
...
\cwait
\cwait

The number of \cwaits you need is exactly equal to the number of
async commands you've issued. For regression testing purposes
you'd need to design the script to ensure that only one of the
connections is expected to respond next, but that seems necessary
anyway --- and you don't need any extra checks to catch the case
that you get an unexpected early response from another one.

Hmm, this still seems a bit notation-heavy, doesn't it? What if \g&
takes an arg indicating which connection to issue the command on:

\c& conn1
\c& conn2
...
CREATE INDEX ... \g& conn1
CREATE INDEX ... \g& conn2
...
\cwait
\cwait

Not totally sure about that one, but issuing a command on a background
connection seems appealing for scripting purposes. It eliminates the
risk that the query response comes back before you manage to switch away
from the connection; which would be bad because it would mess up your
count of how many cwait's you need. It seems a bit more analogous to
the use of & in shell scripts, too, where you implicitly fork away from
the async command. (Maybe c& shouldn't make the new connection
foreground either?)

Yes, I think the \g& conn syntax seems useful. Good thinking.

I agree also that the \S syntax has problems and we would wish to avoid
them. I would still like a way to change the default background session.
That will considerably reduce the number of changes people would need to
make to long scripts in order to be able to use this facility.

For example, if we have a script with 100 commands in, we may find that
commands 1-50 and 51-100 are in two groups. Commands 1-50 are each
dependent upon the previous command, as are 51-100. But the two groups
are independent of each other.

If we use the \g& syntax only, we would need to make 100 changes to the
script to send commands to the right session. If we had the capability
to say "use this background session as the default session to send
commands to", then we would be able to add parallelism to the script by
just making 2 changes: one prior to command 1 and one prior to command
51.

The original \S command had that capability, but was designed to
actually change into that session, giving the problems discussed.
Something like \S (don't care what syntax, though) would definitely
simplify scripting, which I think will translate directly into fewer
bugs for users.

I note \b is available... short for "background". Though I really don't
care what we call that command though, just want the capability.

Also, I don't want to have to count cwaits, so I'd like a command to say
"wait for all background sessions that have active statements" and for
that to be the default. For simplicity, \cwait would do this by default.

So this script

\c& conn1
\c& conn2
...
ALTER TABLE ... ADD PRIMARY KEY \g& conn1
ALTER TABLE ... ADD FOREIGN KEY \g& conn1
ALTER TABLE ... ADD FOREIGN KEY \g& conn1
ALTER TABLE ... ADD FOREIGN KEY \g& conn1
...

ALTER TABLE ... ADD PRIMARY KEY \g& conn2
ALTER TABLE ... ADD FOREIGN KEY \g& conn2
ALTER TABLE ... ADD FOREIGN KEY \g& conn2
ALTER TABLE ... ADD FOREIGN KEY \g& conn2
ALTER TABLE ... ADD FOREIGN KEY \g& conn2
...

\cwait
\cwait

would now become

\c& conn1
\c& conn2
...
\b conn1
ALTER TABLE ... ADD PRIMARY KEY ...
ALTER TABLE ... ADD FOREIGN KEY
ALTER TABLE ... ADD FOREIGN KEY
ALTER TABLE ... ADD FOREIGN KEY
...

\b conn2
ALTER TABLE ... ADD PRIMARY KEY ...
ALTER TABLE ... ADD FOREIGN KEY
ALTER TABLE ... ADD FOREIGN KEY
ALTER TABLE ... ADD FOREIGN KEY
ALTER TABLE ... ADD FOREIGN KEY
...

\cwait

Which seems much cleaner.

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com

Greg,

Not sure whether you're working on this or not?

If so, what do you think of the slightly modified syntax I proposed?

I'm fairly keen on getting this patch completed fairly early on in the
8.4 cycle because it allows a new class of concurrent test case. I think
many people will be happy to submit concurrent test cases once the
syntax is known. That seems likely to reveal a few bugs we've not seen
before, especially when we are able to get that into the build farm. It
seems prudent to do that as early as possible so we have time to fix the
many bugs that emerge, some of them port specific.

Would you like any help?

------------------------------------------------------------------------

On Wed, 2008-04-23 at 15:18 +0100, Simon Riggs wrote:

On Tue, 2008-04-08 at 17:10 -0400, Tom Lane wrote:

What seems possibly more useful is to reintroduce \cwait (or hopefully
some better name) and give it the semantics of "wait for a response from
any active connection; switch to the first one to respond, printing its
name, and print its result".

This would lead to code like, say,

\c& conn1
\c& conn2
...
\S conn1
CREATE INDEX ... \g&
\S conn2
CREATE INDEX ... \g&
...
\cwait
\cwait

The number of \cwaits you need is exactly equal to the number of
async commands you've issued. For regression testing purposes
you'd need to design the script to ensure that only one of the
connections is expected to respond next, but that seems necessary
anyway --- and you don't need any extra checks to catch the case
that you get an unexpected early response from another one.

Hmm, this still seems a bit notation-heavy, doesn't it? What if \g&
takes an arg indicating which connection to issue the command on:

\c& conn1
\c& conn2
...
CREATE INDEX ... \g& conn1
CREATE INDEX ... \g& conn2
...
\cwait
\cwait

Not totally sure about that one, but issuing a command on a background
connection seems appealing for scripting purposes. It eliminates the
risk that the query response comes back before you manage to switch away
from the connection; which would be bad because it would mess up your
count of how many cwait's you need. It seems a bit more analogous to
the use of & in shell scripts, too, where you implicitly fork away from
the async command. (Maybe c& shouldn't make the new connection
foreground either?)

Yes, I think the \g& conn syntax seems useful. Good thinking.

I agree also that the \S syntax has problems and we would wish to avoid
them. I would still like a way to change the default background session.
That will considerably reduce the number of changes people would need to
make to long scripts in order to be able to use this facility.

For example, if we have a script with 100 commands in, we may find that
commands 1-50 and 51-100 are in two groups. Commands 1-50 are each
dependent upon the previous command, as are 51-100. But the two groups
are independent of each other.

If we use the \g& syntax only, we would need to make 100 changes to the
script to send commands to the right session. If we had the capability
to say "use this background session as the default session to send
commands to", then we would be able to add parallelism to the script by
just making 2 changes: one prior to command 1 and one prior to command
51.

The original \S command had that capability, but was designed to
actually change into that session, giving the problems discussed.
Something like \S (don't care what syntax, though) would definitely
simplify scripting, which I think will translate directly into fewer
bugs for users.

I note \b is available... short for "background". Though I really don't
care what we call that command though, just want the capability.

Also, I don't want to have to count cwaits, so I'd like a command to say
"wait for all background sessions that have active statements" and for
that to be the default. For simplicity, \cwait would do this by default.

So this script

\c& conn1
\c& conn2
...
ALTER TABLE ... ADD PRIMARY KEY \g& conn1
ALTER TABLE ... ADD FOREIGN KEY \g& conn1
ALTER TABLE ... ADD FOREIGN KEY \g& conn1
ALTER TABLE ... ADD FOREIGN KEY \g& conn1
...

ALTER TABLE ... ADD PRIMARY KEY \g& conn2
ALTER TABLE ... ADD FOREIGN KEY \g& conn2
ALTER TABLE ... ADD FOREIGN KEY \g& conn2
ALTER TABLE ... ADD FOREIGN KEY \g& conn2
ALTER TABLE ... ADD FOREIGN KEY \g& conn2
...

\cwait
\cwait

would now become

\c& conn1
\c& conn2
...
\b conn1
ALTER TABLE ... ADD PRIMARY KEY ...
ALTER TABLE ... ADD FOREIGN KEY
ALTER TABLE ... ADD FOREIGN KEY
ALTER TABLE ... ADD FOREIGN KEY
...

\b conn2
ALTER TABLE ... ADD PRIMARY KEY ...
ALTER TABLE ... ADD FOREIGN KEY
ALTER TABLE ... ADD FOREIGN KEY
ALTER TABLE ... ADD FOREIGN KEY
ALTER TABLE ... ADD FOREIGN KEY
...

\cwait

Which seems much cleaner.

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com