> -----Original Message-----
> From: Bruce Momjian [mailtogman@candle.pha.pa.us]
> Sent: 21 June 2005 18:06
> To: Dave Page
> Cc: PostgreSQL-development; Andreas Pflug
> Subject: Server instrumentation patch
>
>
> OK, let me address this, but you might not like what I have
> to say. ;-)
>
> Basically, Andreas' approach for 8.0 was to develop a patch (without
> posting a proposal or interface), and then argue why pgadmin needs it,
> but without addressing the real concerns about the patch. Saying
> pgadmin needs it just isn't enough to get a patch in. There are the
> issues of security and maintainability that have to be addressed, and
> in the limited time we had to do this in 8.0, it was clear the patch
> should not be applied.

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.

> 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...6/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...7/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.

> 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...6/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.

Unfortunately sometimes people don't respond - either because they don't
care, or because they agree. Either way, *we* cannot force a discussion,
and in this sort of development model we have no choice than to assume
that if discussion of a issue stops and there are no outstanding
queries, concerns or objections, it's because it's everyone is happy for
the result of those discussions to be accepted into the project.

> This needs discussion, not a patch. And because there are so many
> assumptions made in the patch, the patch committers look unreasonable
> asking for X changes to his patch, when in fact he made X
> assumptions in
> the patch and never asked anyone before developing the patch
> about those
> assumptions.

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.

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?

Regards, Dave.

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

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...6/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...7/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...6/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...6/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...5/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...7/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, Pennsylvania 19073

---------------------------(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

Bruce Momjian wrote:

>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...6/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...7/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...6/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...6/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...5/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...7/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.
>
>

I don't think so. As I mentioned, those views are broken. Do you want
them to be in core anyway?

Regards,
Andreas


---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo@postgresql.org)

Andreas Pflug wrote:

>>For the second, please supply a patch that moves _all_ of dbsize into
>>the main server. I think we have agreement on that.
>>
>
> I don't think so. As I mentioned, those views are broken. Do you want them
> to be in core anyway?

Why is e.g. this one broken:
int8 database_size(name) - Return the size of the database in
bytes (by name)

It should do the same as the one with the oid except that it will resolve
the name first, no? If not it should be fixed, not dropped. I understand
you'd like to have those functions for the GUI frontends, but what about
psql users? For many people it will be hard work to type the subquery to get
the database oid.

I vote for all (possibly corrected) functions to be moved into core.

Best Regards,
Michael Paesold


---------------------------(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

Michael Paesold wrote:

> Andreas Pflug wrote:
>
>>> For the second, please supply a patch that moves _all_ of dbsize into
>>> the main server. I think we have agreement on that.
>>>
>>
>> I don't think so. As I mentioned, those views are broken. Do you want
>> them to be in core anyway?
>
>
> Why is e.g. this one broken:
> int8 database_size(name) - Return the size of the database in
> bytes (by name)
>
> It should do the same as the one with the oid except that it will
> resolve the name first, no? If not it should be fixed, not dropped. I
> understand you'd like to have those functions for the GUI frontends,
> but what about psql users? For many people it will be hard work to
> type the subquery to get the database oid.
>
> I vote for all (possibly corrected) functions to be moved into core.

You're correct about the functions, but I was talking about the views.
"WHERE name = $1" won't respect the schema, contrary to the current doc.

Regards,
Andreas


---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

Andreas Pflug wrote:

> Michael Paesold wrote:
>
>> Andreas Pflug wrote:
>>
>>>> For the second, please supply a patch that moves _all_ of dbsize into
>>>> the main server. I think we have agreement on that.
>>>>
>>>
>>> I don't think so. As I mentioned, those views are broken. Do you want
>>> them to be in core anyway?
>>
>>
>> Why is e.g. this one broken:
>> int8 database_size(name) - Return the size of the database in
>> bytes (by name)
>>
>> It should do the same as the one with the oid except that it will resolve
>> the name first, no? If not it should be fixed, not dropped. I understand
>> you'd like to have those functions for the GUI frontends, but what about
>> psql users? For many people it will be hard work to type the subquery to
>> get the database oid.
>>
>> I vote for all (possibly corrected) functions to be moved into core.
>
> You're correct about the functions, but I was talking about the views.
> "WHERE name = $1" won't respect the schema, contrary to the current doc.

Oh, I am sorry for not reading carefully enough. Didn't know there were
views at all. So if they are broken and cannot be fixed, well...

Best Regards,
Michael Paesold


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

http://archives.postgresql.org