Re: Server instrumentation patch

Dave Page wrote:
> The reason it happen that way was because we already had the code as a
> contrib-style module for pgAdmin. It was posted because we recognised
> that it was becoming a PITA for pgAdmin users to compile a new
> server-side component and the functions seemed like they would be useful
> to other tools similar to pgAdmin.
> 
> Yes, this is not the normal way to proprose new features, but I'm sure
> you appreciate that as picture speaks a thousand words, posting the
> *existing* code with minor changes to properly integrate it shows
> exactly what is being proposed, both in functional and impelmentation
> detail.

Sure.

> > Now, in 8.1, the same thing has happened.  Two weeks before feature
> > freeze, with no discussion, the patch appears, and makes no 
> > reference to
> > concerns raised during the 8.0 discussion.
> 
> OK, first it was the 10th of June which is a little more than two weeks,
> however, Andreas clearly did reference previous discussions on the
> subject - see his message
> http://archives.postgresql.org/pgsql-patches/2005-06/msg00226.php in
> which he points out that 2 functions are from the logger suprocess patch
> from 07/2004, that the file related stuff is based on discussions
> starting at
> http://archives.postgresql.org/pgsql-patches/2004-07/msg00287.php,
> including comments from yourself!!
> 
> > pg_terminate_backend is even
> > in the patch, and there is no mention or attempt to address 
> > concerns we
> > had in 8.0.
> 
> No. I cannot argue with that, and for that reason I suggested that
> Andreas repost the patch without that function so it can be properly
> discussed and implemented in a safe way in the future. I'm sure you have
> see the reposted patch.

OK.

> > The move of dbsize into the backend is similar.  He moves the parts of
> > dbsize the pgadmin needs into the backend, but makes no mention or
> > change to /contrib/dbsize to adjust it to the movement of the code. He
> > has since posted and updated version that fixes this, I think, but
> > again, we have to discuss how this is to be done --- do we 
> > move all the
> > dbsize functions into the backend, some, or none?  Do the other dbsize
> > functions stay in /contrib or get deleted?
> 
> Well as far as I can see, Andreas did respond to all queries about it,
> and then posted his updated patch after it became apparent noone else
> was going to discuss the issue further -
> http://archives.postgresql.org/pgsql-patches/2005-06/msg00309.php. From
> what I can see, no-one has argued or disagreed with his opinion given a
> few days to do so, therefore there was nothing further to discuss. 

Well, I see Marc replying that dbsize should be moved completely from
contrib:
http://archives.postgresql.org/pgsql-patches/2005-06/msg00409.php

The current version of the patch only moves those functions he wants. 
Marc says he wants them all moved, and I agree.

> With the exception of the now removed pg_terminate_backend, I am unaware
> of any issues that are outstanding. If the committers have issues they
> *must* raise them for *any* submitted patch otherwise developers will
> lose faith in the process when their hard work gets ignored.

Well, from the May, 2005 discussion URL you posted, I see a clear reply
to the idea of adding the I/O functions to the backend:
http://archives.postgresql.org/pgsql-hackers/2005-05/msg00874.php

Now, you can agree or disagree that there are issues with the I/O
functions, but the concern was raised in May, and not addressed at all,
either via email or the patch.

> Now, to try to get this ball rolling again - do the committers or anyone
> else have any outstanding issues with the instrumentation or dbsize
> patches that haven't been answered in public discussion and addressed in
> the patches already?

OK, agreed, how can we move forward?  The patch has three parts:
o  file I/Oo  move dbsize from contribo  backend terminate

For the first, we need to re-discuss this on hackers.  I found this as
the conclusion from July of 2004 as it relates to the I/O functions:
http://archives.postgresql.org/pgsql-patches/2004-07/msg00561.php

However, the TODO items still exist so we can discuss it and hopefully
resolve it by feature freeze.

For the second, please supply a patch that moves _all_ of dbsize into
the main server.  I think we have agreement on that.

For backend terminate, I agree with Tom that we have to get SIGTERM
working, and then the function can just send a SIGTERM signal.  Unless
it is working 100%, we will not add a terminate function to SQL.  I will
post separately on this topic.

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073