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/O
o move dbsize from contrib
o 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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073