pgsql: Added the Skytools extended transaction ID module to contrib as

wieck@postgresql.org (Jan Wieck) writes:

Added the Skytools extended transaction ID module to contrib as discussed
on CORE previously.

Seems to have some portability issues ... you can not assume that
strtoull exists, and you shouldn't be assuming that it's appropriate
in the first place.

regards, tom lane

On Sun, Oct 07, 2007 at 11:32:19PM +0000, Jan Wieck wrote:

Log Message:
-----------
Added the Skytools extended transaction ID module to contrib as discussed
on CORE previously.

This module offers transaction ID's containing the original XID and the
transaction epoch as a bigint value to the user level. It also provides
a special txid_snapshot data type that contains an entire transactions
visibility snapshot information, which is useful to determine if a
particular txid was visible to a transaction or not.

The module has been tested by porting Slony-I from using its original
xxid data type.

Jan

A couple of questions on this. I'm not objecting to the tech stuff itself,
but on procedure:

1) Why was this added without any discussion, or even notification, on
-hackers before it was done? Last I checked, -core claim not to deal with
such technicalities, but defer to -hackers (or other lists as needed). I certainly
trust -core to make the right call no these things, but it's not the
procedure that we claim to have.

If that procedure is changing (I've been getting a sneaky feeling that
it's tilting a bit in that direction before this one), that's fine, but it
should be communicated to the community so everybody knows how it works.

2) How can this go in *months* after feature freeze, and even after we
tagged and bundled beta1? This makes such discission even more important,
IMHO.

3) I thought the general agreement was to cut down on contrib, and move
things either to pgfoundry or to core. Why are we adding more? I'm sure
there's motivation for this as discussed on -core, but the rest of us would
like to know that as well... Like why we're not trying to make it a real
feature, if it's something that's important enough to break the rules as in
#2 above.

Or I could've missed the discussion on -hackers that actually took place -
in that case, just discard this message. but the only one I recall is
someone asking for pl/proxy to go in.

//Magnus

Magnus Hagander wrote:

Or I could've missed the discussion on -hackers that actually took place -
in that case, just discard this message. but the only one I recall is
someone asking for pl/proxy to go in.

There was some discussion (subject: Provide 8-byte transaction IDs to
user level), but that itself happened after feature freeze and didn't
seem too conclusive.

I share your other concerns. This process certainly seems to have been
less than transparent.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

I share your other concerns. This process certainly seems to have been
less than transparent.

FWIW, Jan asked -core about a week ago whether it'd be okay to add this
code post-beta, and we concluded it would be okay on the grounds that
(1) we've always been laxer about contrib than the core code,
(2) we'd already granted similar permission to Oleg and Teodor to add
some example tsearch dictionaries to contrib post-beta.
But I was expecting some -hackers discussion and/or a proposed patch
on -patches next. I agree that Jan skipped a step.

regards, tom lane

On 10/8/07, Magnus Hagander <magnus@hagander.net> wrote:

On Sun, Oct 07, 2007 at 11:32:19PM +0000, Jan Wieck wrote:

Log Message:
-----------
Added the Skytools extended transaction ID module to contrib as discussed
on CORE previously.

To explain the situation, the public discussion about the current
submission happened here:

http://lists.pgfoundry.org/pipermail/skytools-users/2007-September/000245.html

and here:

http://lists.slony.info/pipermail/slony1-hackers/2007-September/000057.html

And ofcourse, the original submission was at 2006-07 to _8.2_:

http://archives.postgresql.org/pgsql-patches/2006-07/msg00157.php

It was rejected then mostly on 3 reasons (from my POV):

- it was messy and contained unnecesary cruft.
- it was submitted to core not /contrib
- slony was not interested in it at that moment

Now as you can read from recent disussion we had, we found out
that it would be *really* *really* cool if it would appear
in 8.3... Talk about last moment...

Because of the bad timing it would have been -core call anyway
whether it gets in or not so Jan asked -core directly. That's
my explanation about what happened, obviously Jan and Tom have
their own opinion.

--
marko

Marko Kreen wrote:

Now as you can read from recent disussion we had, we found out
that it would be *really* *really* cool if it would appear
in 8.3... Talk about last moment...

That discussion didn't happen on any list I read, so to me it just came
as a bolt from the blue.

Surely there should at the very least have been a patch posted, core
approval or not.

cheers

andrew

Marko Kreen wrote:

On 10/8/07, Magnus Hagander <magnus@hagander.net> wrote:

On Sun, Oct 07, 2007 at 11:32:19PM +0000, Jan Wieck wrote:

Log Message:
-----------
Added the Skytools extended transaction ID module to contrib as discussed
on CORE previously.

To explain the situation, the public discussion about the current
submission happened here:

http://lists.pgfoundry.org/pipermail/skytools-users/2007-September/000245.html

and here:

http://lists.slony.info/pipermail/slony1-hackers/2007-September/000057.html

Ok. That certainly explains it - it did sound weird to have that go in
without any public discussion at all - but none of those lists are
pgsql-hackers ;-)

And ofcourse, the original submission was at 2006-07 to _8.2_:

http://archives.postgresql.org/pgsql-patches/2006-07/msg00157.php

Ah. I only searched for this year, since I only considered submissions
for 8.3. But still, it wasn't AFAIK on any of the patch lists etc.

It was rejected then mostly on 3 reasons (from my POV):

- it was messy and contained unnecesary cruft.
- it was submitted to core not /contrib
- slony was not interested in it at that moment

Now as you can read from recent disussion we had, we found out
that it would be *really* *really* cool if it would appear
in 8.3... Talk about last moment...

Well, if it's really really cool to have, why do we put it in /contrib?
If it's that cool, it should be in core, no?

That's not just making comments, I really *do* think that it should be
in core if it's interesting enough to be added to contrib at this time.

Because of the bad timing it would have been -core call anyway
whether it gets in or not so Jan asked -core directly. That's
my explanation about what happened, obviously Jan and Tom have
their own opinion.

Right. I can see your point, but it's my understanding that -hackers is
really the ones supposed to decide on this.

//Magnus

Magnus Hagander <magnus@hagander.net> writes:

Marko Kreen wrote:

Because of the bad timing it would have been -core call anyway
whether it gets in or not so Jan asked -core directly. That's
my explanation about what happened, obviously Jan and Tom have
their own opinion.

Right. I can see your point, but it's my understanding that -hackers is
really the ones supposed to decide on this.

It would ultimately have been core's decision, but the discussion should
have happened on -hackers. There was no reason for it to be private.

regards, tom lane

On 10/8/2007 1:34 PM, Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

Marko Kreen wrote:

Because of the bad timing it would have been -core call anyway
whether it gets in or not so Jan asked -core directly. That's
my explanation about what happened, obviously Jan and Tom have
their own opinion.

Right. I can see your point, but it's my understanding that -hackers is
really the ones supposed to decide on this.

It would ultimately have been core's decision, but the discussion should
have happened on -hackers. There was no reason for it to be private.

That blame certainly belongs to me and I apologize for jumping that and
adding it to contrib without any -hackers discussion.

It is definitely a timing issue since I write this very email from JFK,
boarding a flight to Hong Kong in less than an hour and will be mostly
offline for the rest of the week.

I agree with the technical issue Tom brought up. Slony itself doesn't
rely on strtoull() either and this slipped through. I will see that I
fix that by using Slony's int64 scanning code. I can work on it during
the flight and commit the fix when I arrive in the hotel.

To Magnus: It certainly would have been cool to have this in core, but
two weeks ago we didn't know if we can get the code into shape for that
before BETA (as it is right now I would say it still isn't). So we shot
for the next best target, which was contrib, where post BETA changes
aren't as critical.

Jan

--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #

On 10/8/07, Magnus Hagander <magnus@hagander.net> wrote:

Marko Kreen wrote:

On 10/8/07, Magnus Hagander <magnus@hagander.net> wrote:

On Sun, Oct 07, 2007 at 11:32:19PM +0000, Jan Wieck wrote:

Log Message:
-----------
Added the Skytools extended transaction ID module to contrib as discussed
on CORE previously.

To explain the situation, the public discussion about the current
submission happened here:

http://lists.pgfoundry.org/pipermail/skytools-users/2007-September/000245.html

and here:

http://lists.slony.info/pipermail/slony1-hackers/2007-September/000057.html

Ok. That certainly explains it - it did sound weird to have that go in
without any public discussion at all - but none of those lists are
pgsql-hackers ;-)

Yes, sorry about that. Just the discussion started very
hypetetically, with us probing each other opinion, and there
was nothing to discuss with -hackers.

When we saw a concrete plan for the module, then it was too late
to do a regular -hackers submission, due to the beta timeline
we needed -core opinion immidiately.

Now, after -core gave a nod, then yes, the patch should have been
to -hackers with a notice that it is on fast-path.

[btw, this is me guessing Jan's thinking, but I would have
acted same way.]

Now as you can read from recent disussion we had, we found out
that it would be *really* *really* cool if it would appear
in 8.3... Talk about last moment...

Well, if it's really really cool to have, why do we put it in /contrib?
If it's that cool, it should be in core, no?

Main cool thing came from fact that this is the last moment
Slony could do such big conversion of it's codebase.

That's not just making comments, I really *do* think that it should be
in core if it's interesting enough to be added to contrib at this time.

Yes, it is cool enough to be in core and I think that's the goal.

But asking for it to be accepted to core a week before beta and
couple months after feature freeze would be asking bit too much,
don't you think? ;)

--
marko

Jan Wieck wrote:

On 10/8/2007 1:34 PM, Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

Marko Kreen wrote:

Because of the bad timing it would have been -core call anyway
whether it gets in or not so Jan asked -core directly. That's
my explanation about what happened, obviously Jan and Tom have
their own opinion.

Right. I can see your point, but it's my understanding that -hackers is
really the ones supposed to decide on this.

It would ultimately have been core's decision, but the discussion should
have happened on -hackers. There was no reason for it to be private.

That blame certainly belongs to me and I apologize for jumping that and
adding it to contrib without any -hackers discussion.

It is definitely a timing issue since I write this very email from JFK,
boarding a flight to Hong Kong in less than an hour and will be mostly
offline for the rest of the week.

I agree with the technical issue Tom brought up. Slony itself doesn't
rely on strtoull() either and this slipped through. I will see that I
fix that by using Slony's int64 scanning code. I can work on it during
the flight and commit the fix when I arrive in the hotel.

Good. Win32 is pretty much dead right now, so we can't proceed with
things like testing the buildfarm with msvc/ecpg.

To Magnus: It certainly would have been cool to have this in core, but
two weeks ago we didn't know if we can get the code into shape for that
before BETA (as it is right now I would say it still isn't). So we shot
for the next best target, which was contrib, where post BETA changes
aren't as critical.

Right. My thought is still that if it isn't good enough for core, it
shouldn't be in contrib. If it *is* good enough, and we want it, we
should accept that it came in long after freeze and put it in core
anyway. If it *isn't*, then it should be on pgfoundry and be moved into
core when it's ready - for 8.4 or so.

The whole contrib thing confuses a lot of users. Is it included, or
isn't it? IMHO, that distinction need to be clear, and I thought we
were working (if not actively then at least passively) to "retire"
contrib, moving things either to core or to pgFoundry. Adding yet
another important feature that's "just in contrib" is making things
worse, not better.
IMHO, of course ;-)

//Magnus

Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

Marko Kreen wrote:

Because of the bad timing it would have been -core call anyway
whether it gets in or not so Jan asked -core directly. That's
my explanation about what happened, obviously Jan and Tom have
their own opinion.

Right. I can see your point, but it's my understanding that -hackers is
really the ones supposed to decide on this.

It would ultimately have been core's decision, but the discussion should
have happened on -hackers. There was no reason for it to be private.

Hmm. I thought that -core doesn't decide on things like these, they just
"vote" on -hackers and have no special powers (other than being very
respected community members that we all listen to, of course).

I seem to recall hearing all the time (most often from people on core,
but I'm certainly one of the people who relay that information further)
that core specifically *doesn't* decide on things like that (being
direct technical issues, or just the talk about the name-change that's
been flooding -advocacy), but that core are only there for "dealing with
companies that don't want to deal in public", and for making decisions
"if -hackers can't agree", security sensitive stuff, and things like that.

It may be that it just was like that before, and isn't anymore, and my
information is outdated. I don't mind, really, because I certainly trust
-core to make good decisions. But if that's so, then at least I have to
change what I tell people that ask...

//Magnus

"Magnus Hagander" <magnus@hagander.net> writes:

The whole contrib thing confuses a lot of users. Is it included, or
isn't it? IMHO, that distinction need to be clear, and I thought we
were working (if not actively then at least passively) to "retire"
contrib, moving things either to core or to pgFoundry. Adding yet
another important feature that's "just in contrib" is making things
worse, not better.
IMHO, of course ;-)

I disagree with this principle. I think contrib has a role to play and there
are modules which belong in contrib for now and forever. The key distinction
isn't code quality -- I think that's an effect rather than a cause.

The key distinction is the intended audience. Contrib is for things which are
integral parts of the system and as such would be harder to maintain in
pgfoundry, but have a very limited audience, especially things that a typical
admin wouldn't necessarily want in his install for security or safety reasons.

So pageinspect, adminpack, pg_buffercache, pg_standby, etc, these are all
things which are tightly tied to the system. They often need to be modified
when making unrelated changes to the core and can't be maintained as a
separate add-on by a different group of maintainers. But they're not
appropriate to install by default because they have a limited audience.

Some of the modules in there are legacy modules which today would probably be
done as pgfoundry modules. The GIST data types, earthdistance, isn, etc. We
did actively prune out a lot of modules like that which were poorly maintained
and bitrotting. The ones which remain are in reasonably good shape and have
wide enough user-bases that moving them to pgfoundry would cause more upgrade
pain than it would help.

But that doesn't mean we're phasing contrib out entirely. The question remains
whether txid is more like pageinspect/pg_standby/etc or more like
earthdistance/isn. It does sound like the whole point of it is to provide an
interface to core that pgfoundry modules can use, so presumably it's dealing
with the nitty gritty stuff that pgfoundry modules would have trouble
maintaining. Also, we only want *one* official such module. I think pushing it
to pgfoundry doesn't make any sense.

Does it make sense to put it in core? it has a limited audience in that only
skype and slony users need it. On the other hand there's not much reason an
admin wouldn't want it installed I don't think.

What happens if we put it in core now and then the replication folk add more
to the interface and include things that not all admins would want installed?
And is the interface mature enough that users should be building applications
depending on this interface directly?

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

On Monday 08 October 2007 16:29, Magnus Hagander wrote:

The whole contrib thing confuses a lot of users. Is it included, or
isn't it? IMHO, that distinction need to be clear, and I thought we
were working (if not actively then at least passively) to "retire"
contrib, moving things either to core or to pgFoundry. Adding yet
another important feature that's "just in contrib" is making things
worse, not better.
IMHO, of course ;-)

+1. I felt the same way about pg_standby, which would have been far more
accessible for 8.2 users had it lived on pg_foundry.

--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

Tom Lane wrote:

(1) we've always been laxer about contrib than the core code,

While that appears to be true, I think

(a) there is no technical reason allowing us to do that, and
(b) most people don't seem to like it.

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

On Mon, 08 Oct 2007 17:38:48 -0400
Robert Treat <xzilla@users.sourceforge.net> wrote:

On Monday 08 October 2007 16:29, Magnus Hagander wrote:

The whole contrib thing confuses a lot of users. Is it included, or
isn't it? IMHO, that distinction need to be clear, and I thought we
were working (if not actively then at least passively) to "retire"
contrib, moving things either to core or to pgFoundry. Adding yet
another important feature that's "just in contrib" is making things
worse, not better.
IMHO, of course ;-)

+1. I felt the same way about pg_standby, which would have been far
more accessible for 8.2 users had it lived on pg_foundry.

Well pg_standby is certainly an excellent example :). We have had this
similar discussion before, this exact discussion is one of the reasons
Tsearch2 got pushed into core.

Contrib is just a dead zone for the user populous. Most people consider
it unsupported *stuff* with no particular purpose (regardless of the
real meaning).

If you look at contrib in other projects they are always user
contributed modules that are cool, but your mileage may vary.

Personally, I would like to see contrib completely removed.

Sincerely,

Joshua D. Drake

--

=== The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997 http://www.commandprompt.com/
UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

On Mon, 08 Oct 2007 22:32:55 +0200
Magnus Hagander <magnus@hagander.net> wrote:

Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

Marko Kreen wrote:

Because of the bad timing it would have been -core call anyway
whether it gets in or not so Jan asked -core directly. That's
my explanation about what happened, obviously Jan and Tom have
their own opinion.

Right. I can see your point, but it's my understanding that
-hackers is really the ones supposed to decide on this.

It would ultimately have been core's decision, but the discussion
should have happened on -hackers. There was no reason for it to be
private.

Hmm. I thought that -core doesn't decide on things like these, they
just "vote" on -hackers and have no special powers (other than being
very respected community members that we all listen to, of course).

That was my understanding as well.

I quote from Tom Lane on August 30th of this year:

I think you're overstating the amount of power the core committee has.
Core sets release schedules, but beyond that has no more power than any
other committer. The pool of committers is quite a bit bigger than core.

In practice, of course, core has quite a lot of political power because
folk are often willing to follow our lead. But we'd lose that real
quick if we tried to lead in the wrong direction.

Source:

http://people.planetpostgresql.org/xzilla/index.php?/archives/317-PostgreSQL-is-Not-a-Democracy.html

Sincerely,

Joshua D. Drake

--

=== The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997 http://www.commandprompt.com/
UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

Magnus Hagander <magnus@hagander.net> writes:

Right. My thought is still that if it isn't good enough for core, it
shouldn't be in contrib. If it *is* good enough, and we want it, we
should accept that it came in long after freeze and put it in core
anyway. If it *isn't*, then it should be on pgfoundry and be moved into
core when it's ready - for 8.4 or so.

The long and the short of it was that the patch wasn't ready. To put it
in core for 8.3, we'd have either had to delay the beta yet more, or
force initdb post-beta1, neither of which would have flown.

The whole contrib thing confuses a lot of users.

To me, contrib exists mostly as a forcing function to ensure that we
keep the extension-module system working. If we got rid of it entirely,
as some here propose, we'd be more likely to break things that we'd not
find out about until much later (like when some pgfoundry project tried
to update their code, which almost certainly wouldn't be till the next
beta cycle).

Contrib also has a role to play as a repository of code examples that
people can crib from when developing new extension modules. I would
not want to claim that it's all "best practice" code --- a lot of it
definitely isn't --- but it stands a lot better chance of representing
current good practice if it's maintained with the core code than if it's
out on pgfoundry. On pgfoundry, it won't get included in the global-
search-and-replace patches that we do so many of, and it'll most likely
accumulate a lot of cruft from trying to be compatible with multiple
core releases.

So I have no interest in trying to "retire" contrib. I think there's
room for debate about exactly which modules to include, of course.

regards, tom lane

A Dimarts 09 Octubre 2007, Joshua D. Drake va escriure:

Contrib is just a dead zone for the user populous. Most people consider
it unsupported *stuff* with no particular purpose (regardless of the
real meaning).

I think no visible documentation is the reason for this misconception and 8.3
will provide contrib documentation together with core docs. Let's see what
happens then...

--
Albert Cervera i Areny
http://www.NaN-tic.com