Unix Technical Forum

Re: Concurrent psql patch

This is a discussion on Re: Concurrent psql patch within the Pgsql Patches forums, part of the PostgreSQL category; --> On Mon, May 14, 2007 at 12:51:39PM +0100, Gregory Stark wrote: > "Tom Lane" <tgl@sss.pgh.pa.us> writes: > > > ...


Go Back   Unix Technical Forum > Database Server Software > PostgreSQL > Pgsql Patches

Register FAQ Members List Calendar Search Today's Posts Mark Forums Read
  #11 (permalink)  
Old 04-18-2008, 10:03 AM
Jim C. Nasby
 
Posts: n/a
Default Re: Concurrent psql patch

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)

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #12 (permalink)  
Old 04-18-2008, 10:04 AM
Jim C. Nasby
 
Posts: n/a
Default Re: Concurrent psql patch

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)

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

http://archives.postgresql.org

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #13 (permalink)  
Old 04-18-2008, 10:04 AM
Gregory Stark
 
Posts: n/a
Default Re: Concurrent psql patch


"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


---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #14 (permalink)  
Old 04-18-2008, 10:04 AM
Jim C. Nasby
 
Posts: n/a
Default Re: Concurrent psql patch

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)

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

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #15 (permalink)  
Old 04-18-2008, 10:04 AM
daveg
 
Posts: n/a
Default Re: Concurrent psql patch

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.

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

http://archives.postgresql.org

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #16 (permalink)  
Old 04-18-2008, 10:07 AM
Andrew Dunstan
 
Posts: n/a
Default Re: Concurrent psql patch



Gregory Stark wrote:
> Attached is an updated patch.
>


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

cheers

andrew


---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #17 (permalink)  
Old 04-18-2008, 10:08 AM
Peter Eisentraut
 
Posts: n/a
Default Re: Concurrent psql patch

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/

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #18 (permalink)  
Old 04-18-2008, 10:08 AM
Andrew Dunstan
 
Posts: n/a
Default Re: Concurrent psql patch



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

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #19 (permalink)  
Old 04-18-2008, 10:08 AM
Tom Lane
 
Posts: n/a
Default Re: Concurrent psql patch

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

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

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #20 (permalink)  
Old 04-18-2008, 10:08 AM
Gregory Stark
 
Posts: n/a
Default Re: Concurrent psql patch

"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


---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
Reply


Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are On
Forum Jump


All times are GMT. The time now is 07:17 AM.


Powered by vBulletin® Version 3.6.5
Copyright ©2000 - 2008, Jelsoft Enterprises Ltd.
SEO by vBSEO 3.2.0
www.UnixAdminTalk.com