Ding-dong, contrib is dead ...

On 2006-09-05, Josh Berkus <josh@agliodbs.com> wrote:

Migrated to pgFoundry:

[...]

userlock (Merlin)

Userlock needs to go into core, not get removed; this was discussed in a
previous "let's clean up contrib/" thread. If it's not going into core
in 8.2 then it needs to stay in contrib.

People then didn't seem receptive to my suggestion that the existing
userlock code was non-copyrightable (on the grounds of being pure
interface), but that difficulty no longer exists now that Abhijit has
posted his clean-room rewrite (look for "otherlock" in -patches).
Perhaps he would be prepared to turn that into a patch against the core...

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

At 2006-09-05 05:47:58 -0000, andrew+nonews@supernews.com wrote:

that difficulty no longer exists now that Abhijit has posted his
clean-room rewrite (look for "otherlock" in -patches). Perhaps he
would be prepared to turn that into a patch against the core...

Absolutely. Just tell me where it should live and I'll post a patch.

-- ams

Josh Berkus <josh@agliodbs.com> writes:

Migrated to pgFoundry:
adddepends (Greg)
dbmirror (Steve Singer)
dbase --> dbf2pg
fulltextindex --> simplefti
mac (LER) --> mac-manufacturer
userlock (Merlin)

Please also kill the following two contrib directories, because despite an
impassioned plea by Robert Treat, they appear to not have any users (at least
nobody responded to posts on PWN or -hackers)
tips
mSQL-interface

Checking my copy of the "hit list", I thought we'd agreed to migrate
contrib/oracle as well.

regards, tom lane

Andrew - Supernews <andrew+nonews@supernews.com> writes:

Userlock needs to go into core, not get removed; this was discussed in a
previous "let's clean up contrib/" thread.

Something like it ought to go into core, but personally I'd opt for
taking the opportunity to redesign the API, which was a bit crufty to
begin with. That would eliminate all question of whether the clean room
was clean enough.

regards, tom lane

At 2006-09-05 10:23:19 -0400, tgl@sss.pgh.pa.us wrote:

Something like it ought to go into core, but personally I'd opt for
taking the opportunity to redesign the API, which was a bit crufty to
begin with.

I'm happy to do the work right away (not that there's much) if someone
suggests a better API. (I don't personally have a need for user-level
locks, and if I did, I'd be happy with just user_lock/user_unlock. So
if anyone has a more specific idea, I'm all ears.)

That would eliminate all question of whether the clean room was
clean enough.

It was really, really clean! Honest! :-)

-- ams

Tom Lane wrote:

Andrew - Supernews <andrew+nonews@supernews.com> writes:

Userlock needs to go into core, not get removed; this was discussed in a
previous "let's clean up contrib/" thread.

Something like it ought to go into core, but personally I'd opt for
taking the opportunity to redesign the API, which was a bit crufty to
begin with. That would eliminate all question of whether the clean room
was clean enough.

It seems odd to remove the module from contrib for 8.2 and then put a
replacement in core for 8.3. I guess we could signal our intentions in
the release notes.

It's a pity we didn't have Abhijit's patch 6 weeks ago.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

It's a pity we didn't have Abhijit's patch 6 weeks ago.

Well, now that we have it, the question is whether we want to do
anything with it. One problem is it lacks documentation.

However, as I said, I'd really rather choose a new API altogether. The
main thing that seems to be lacking is a way to wait for a lock, rather
than having only the equivalent of ConditionalLockAcquire. Also I don't
much like exposing a LOCKMODE directly to user code --- to use
user_lock() or user_unlock() you have to put magic numbers into your SQL
code and hope nobody reassigns the C enum values in future releases.
I'd be inclined to just expose the notions of "share" and "exclusive"
lock and make these separate functions instead of magic numbers.

And then there's the question of what to expose in the way of lock
identifier options. What we've got now is "two int4's or an OID"
which seems a bit random, not to mention that the key space overlaps
in an undocumented fashion. Possibly we could offer OID, int8, or
two int4s, and modify the code to set locktag_field4 to distinguish
these cases so that the key spaces are independent.

I have no opinions about function names, except that I'd suggest
choosing names based around "advisory lock" instead of "user lock".
Advisory locks are a standard concept and so that terminology
already tells people something ...

regards, tom lane

Andrew,

It seems odd to remove the module from contrib for 8.2 and then put a
replacement in core for 8.3. I guess we could signal our intentions in
the release notes.

The current code is GPL. It *has* to be removed.

--Josh

Tom,

Checking my copy of the "hit list", I thought we'd agreed to migrate
contrib/oracle as well.

Hmmm ... somehow that got dropped out of discussions early on, without
any reason why. See the "more nuclear options" thread; oracle is
nowhere on it.

Will only take me 30 min to migrate, but we need to give people time to
object ... and I need to check whether there's even any code in there
that superceded orasysviews and orafce and oralink.

Hey, everyone, if you have a reason for contrib/oracle not to migrate
out to pgFoundry, speak up now!

Or if you want to take the new project over, speak up too.

--Josh Berkus

Josh Berkus <josh@agliodbs.com> writes:

Checking my copy of the "hit list", I thought we'd agreed to migrate
contrib/oracle as well.

Hmmm ... somehow that got dropped out of discussions early on, without
any reason why.

Actually ... never mind that, it seems to have been done already.
Sorry for the noise.

regards, tom lane

On 9/5/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

It's a pity we didn't have Abhijit's patch 6 weeks ago.

Well, now that we have it, the question is whether we want to do
anything with it. One problem is it lacks documentation.

yes, userlocks have to be documented, in particular the dangers of
lock exhaustion. also a tie-in to the recently upgraded pg_locks view
would be nice.

However, as I said, I'd really rather choose a new API altogether. The
main thing that seems to be lacking is a way to wait for a lock, rather
than having only the equivalent of ConditionalLockAcquire. Also I don't
much like exposing a LOCKMODE directly to user code --- to use
user_lock() or user_unlock() you have to put magic numbers into your SQL
code and hope nobody reassigns the C enum values in future releases.
I'd be inclined to just expose the notions of "share" and "exclusive"
lock and make these separate functions instead of magic numbers.

I agree 100%.

And then there's the question of what to expose in the way of lock
identifier options. What we've got now is "two int4's or an OID"
which seems a bit random, not to mention that the key space overlaps
in an undocumented fashion. Possibly we could offer OID, int8, or
two int4s, and modify the code to set locktag_field4 to distinguish
these cases so that the key spaces are independent.

right, this is some legacy cruft, in fact I raised this to your
attention which was perhaps part of the inspiration to upgrade
pg_locks.

I have no opinions about function names, except that I'd suggest
choosing names based around "advisory lock" instead of "user lock".
Advisory locks are a standard concept and so that terminology
already tells people something ...

Agreement here also.

As to the point of userlocks being in core, they are in fact already
in core, and have been several years, the name 'userlock' having been
taken from the in source documentation. The userlock contrib module
is nothing besides some wrappers for exisiting functions built into
the backend. Removing userlock from contrib just removes a convenient
mechanism to use them.

I also agree with Andrew that pgfoundry is not a appropriate place for
userlocks. They should be properly documented with a cleaned up api.
I have no objection from them being removed from contrib in the short
term due to the gpl issue, although I am not sure how you can
copyright a function wrapper.

merlin

"Merlin Moncure" <mmoncure@gmail.com> writes:

I also agree with Andrew that pgfoundry is not a appropriate place for
userlocks. They should be properly documented with a cleaned up api.
I have no objection from them being removed from contrib in the short
term due to the gpl issue, although I am not sure how you can
copyright a function wrapper.

Right, I see the pgfoundry project as just a backwards-compatibility
thing for anyone who doesn't want to change their code. I'm happy to
put some cleaned-up functions into core right now (ie, for 8.2) if
someone will do the legwork to define and implement them.

After further thought it occurs to me that having both OID and int8
keys might be a problem, in that it's not too clear which you'd get
from a single-argument call. But we could offer just int8 and two-int4
signatures and rely on promoting OID to int8 if you need a lock on OID.

So the function list might look like

void pg_advisory_lock(int8) wait
void pg_advisory_lock_shared(int8) wait
bool pg_try_advisory_lock(int8) no wait
bool pg_try_advisory_lock_shared(int8) no wait
bool pg_advisory_unlock(int8) returns T if successful
bool pg_advisory_unlock_shared(int8) returns T if successful

plus all the above taking 2 int4's, plus

void pg_advisory_unlock_all()

Not wedded to these names at all...

regards, tom lane

On 2006-09-05, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

It's a pity we didn't have Abhijit's patch 6 weeks ago.

Well, now that we have it, the question is whether we want to do
anything with it. One problem is it lacks documentation.

However, as I said, I'd really rather choose a new API altogether.

What about existing users?

The main thing that seems to be lacking is a way to wait for a lock,

Is this a feature that people actually want or need?

Certainly exposing the lockmode as a magic number isn't ideal.

And then there's the question of what to expose in the way of lock
identifier options. What we've got now is "two int4's or an OID"
which seems a bit random, not to mention that the key space overlaps
in an undocumented fashion.

It is documented in the original README.user_locks.

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

On 2006-09-05, Josh Berkus <josh@agliodbs.com> wrote:

The current code is GPL. It *has* to be removed.

Which is why Abhijit's version exists - it's intended to be a drop-in,
BSD-licensed replacement for the current code.

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

On 9/5/06, Andrew - Supernews <andrew+nonews@supernews.com> wrote:

On 2006-09-05, Josh Berkus <josh@agliodbs.com> wrote:

The current code is GPL. It *has* to be removed.

Which is why Abhijit's version exists - it's intended to be a drop-in,
BSD-licensed replacement for the current code.

does his patch include documentation? I can help with that if it isn't
done. was it reviewed?

merlin

Merlin,

The current code is GPL. It *has* to be removed.

Which is why Abhijit's version exists - it's intended to be a drop-in,
BSD-licensed replacement for the current code.

does his patch include documentation? I can help with that if it isn't
done. was it reviewed?

No, and no. It's unfortunately too late for Abhijit's patch to make it
into 8.2; it was't submitted until last week, I believe.

So userlocks will be in pgFoundry for the next rev -- frankly, it should
have been for 8.1 but I forgot it. For the 8.3 version, as Tom has
indicated we may want to change the API somewhat anyway, so we'll want the
pgFoundry version for backwards-compat.

--
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

Tom Lane wrote:

Checking my copy of the "hit list", I thought we'd agreed to migrate
contrib/oracle as well.

It has already been removed because it is being actively maintained
elsewhere.

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

On 9/5/06, Josh Berkus <josh@agliodbs.com> wrote:

So userlocks will be in pgFoundry for the next rev -- frankly, it should
have been for 8.1 but I forgot it. For the 8.3 version, as Tom has
indicated we may want to change the API somewhat anyway, so we'll want the
pgFoundry version for backwards-compat.

well, I'm confused now. Tom said that cleaned up functions might be
sneaked into 8.2, which is what prompted my question. If that's the
case I'm considering putting something together quickly. It's no big
deal to me either way really. However, it would really be a shame to
drop the contrib module and leave 8.2 without a way of easily use them
(userlocks being, imho, the #1 greatest undiscovered feature in pg).

To be honest, I don't see the need for a backwards-compat version at
all, because all you need to do is copy and paste the code from 8.1.
If advisory functions are promoted in core (8.3 or no), sql wrappers
for compatibility would be trivial to implement. These are one line
wrappers here.

merlin

On 2006-09-05, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Right, I see the pgfoundry project as just a backwards-compatibility
thing for anyone who doesn't want to change their code. I'm happy to
put some cleaned-up functions into core right now (ie, for 8.2) if
someone will do the legwork to define and implement them.

So you're prepared to violate the feature freeze to stick in a new API
that nobody currently wants to _use_, while forcing existing users to
resort to pgfoundry for a module that's been around for several major
releases?

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

Merlin,

well, I'm confused now. Tom said that cleaned up functions might be
sneaked into 8.2, which is what prompted my question.

You're correct, he did. Tom?

If that's the
case I'm considering putting something together quickly. It's no big
deal to me either way really. However, it would really be a shame to
drop the contrib module and leave 8.2 without a way of easily use them
(userlocks being, imho, the #1 greatest undiscovered feature in pg).

Well, all I'm dealing with is that the *existing GPL code* can't stay.
Let me know if Abhijit's version (or something else) gets accepted and
I'll kill the pgFoundry project.

Overall, though, I think we should really wait until 8.3 for core merge and
API improvements. Wasn't Tom just complaining about last-minute features,
and not enough code reviewers?

--
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

Tom Lane wrote:

"Merlin Moncure" <mmoncure@gmail.com> writes:

I also agree with Andrew that pgfoundry is not a appropriate place for
userlocks. They should be properly documented with a cleaned up api.
I have no objection from them being removed from contrib in the short
term due to the gpl issue, although I am not sure how you can
copyright a function wrapper.

Right, I see the pgfoundry project as just a backwards-compatibility
thing for anyone who doesn't want to change their code. I'm happy to
put some cleaned-up functions into core right now (ie, for 8.2) if
someone will do the legwork to define and implement them.

hmm - that is all a nice and such - but is it really a good idea to do
this that late in the release-cycle ?
I think the most "natural" thing would be to replace the existing GPL'd
userlock code with the new one and discuss the API-change one for 8.3
and up ...

Stefan

On 2006-09-05, "Merlin Moncure" <mmoncure@gmail.com> wrote:

On 9/5/06, Andrew - Supernews <andrew+nonews@supernews.com> wrote:

On 2006-09-05, Josh Berkus <josh@agliodbs.com> wrote:

The current code is GPL. It *has* to be removed.

Which is why Abhijit's version exists - it's intended to be a drop-in,
BSD-licensed replacement for the current code.

does his patch include documentation? I can help with that if it isn't
done. was it reviewed?

I don't think it has docs. The existing docs would be fine unless anyone
thinks that those also have copyright problems.

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

On 9/5/06, Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> wrote:

Tom Lane wrote:

"Merlin Moncure" <mmoncure@gmail.com> writes:

I also agree with Andrew that pgfoundry is not a appropriate place for
userlocks. They should be properly documented with a cleaned up api.
I have no objection from them being removed from contrib in the short
term due to the gpl issue, although I am not sure how you can
copyright a function wrapper.

Right, I see the pgfoundry project as just a backwards-compatibility
thing for anyone who doesn't want to change their code. I'm happy to
put some cleaned-up functions into core right now (ie, for 8.2) if
someone will do the legwork to define and implement them.

hmm - that is all a nice and such - but is it really a good idea to do
this that late in the release-cycle ?
I think the most "natural" thing would be to replace the existing GPL'd
userlock code with the new one and discuss the API-change one for 8.3
and up ...

I think that's a reasonable solution, replace the existing (renamed?)
contrib with new wrappers and push core migration/documentation out to
8.3. Then we are talking about one line wrappers here, not a feature
per se...

merlin

Tom Lane wrote:

I'm happy to
put some cleaned-up functions into core right now (ie, for 8.2) if
someone will do the legwork to define and implement them.

OK, who are you and what have you done with the real Tom Lane?

cheers

andrew

Josh Berkus <josh@agliodbs.com> writes:

Merlin,

well, I'm confused now. Tom said that cleaned up functions might be
sneaked into 8.2, which is what prompted my question.

You're correct, he did. Tom?

Well, it's not like we're done with forced initdb's for 8.2, so I don't
particularly see the harm in adding a few more functions. I would be
against writing something large and complicated at this point, but these
functions are trivial (practically one-liners) and I don't think there's
a lot of debate needed about the API. The biggest part of the work
needed is to write the documentation --- but we'd have to do that for
Abhijit's patch too, since the userlocks docs presumably fall under GPL
along with the code.

So basically I don't see the point of investing effort in a
bug-compatible version of userlocks, when we can have something cleaner
and suitable for the long run with not very much more effort.

regards, tom lane

On 2006-09-05, "Merlin Moncure" <mmoncure@gmail.com> wrote:

I have no objection from them being removed from contrib in the short
term due to the gpl issue, although I am not sure how you can
copyright a function wrapper.

I made this point several times in the original discussion (which was a
year and a half or so ago). However, others seemed to disagree, which is
why we now have a replacement version.

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

josh@agliodbs.com (Josh Berkus) writes:

Overall, though, I think we should really wait until 8.3 for core merge and
API improvements. Wasn't Tom just complaining about last-minute features,
and not enough code reviewers?

He may have worked through enough of the backlog that he's ready to
add more to the queue ? ;-)
--
let name="cbbrowne" and tld="cbbrowne.com" in String.concat "@" [name;tld];;
http://linuxdatabases.info/info/languages.html
This program posts news to billions of machines throughout the galaxy.
Your message will cost the net enough to bankrupt your entire planet.
As a result your species will be sold into slavery. Be sure you know
what you are doing. Are you absolutely sure you want to do this? [yn]
y

At 2006-09-05 16:35:49 -0400, tgl@sss.pgh.pa.us wrote:

The biggest part of the work needed is to write the documentation ---
but we'd have to do that for Abhijit's patch too, since the userlocks
docs presumably fall under GPL along with the code.

I'll write the documentation, either for the code as it is, or for any
replacement we decide to use.

I didn't submit documentation (or a Makefile, uninstall_otherlock.sql,
etc.) only because I didn't know if anything was going to be done with
otherlock now. I just wanted to mention the existence of the code.

So basically I don't see the point of investing effort in a
bug-compatible version of userlocks, when we can have something
cleaner and suitable for the long run with not very much more
effort.

Fine with me. Two questions:

- Where would the code live, if it were in core?
- Shall I hack up the API you suggested in your earlier message?

-- ams

Abhijit Menon-Sen <ams@oryx.com> writes:

Fine with me. Two questions:

- Where would the code live, if it were in core?

New file under utils/adt/ probably ... it's not a new datatype exactly,
but that's the default place for new SQL-visible functions.

- Shall I hack up the API you suggested in your earlier message?

I'm not wedded to any part of that proposal, but I haven't seen any
objections or better ideas yet ...

regards, tom lane

On 9/5/06, Abhijit Menon-Sen <ams@oryx.com> wrote:

At 2006-09-05 16:35:49 -0400, tgl@sss.pgh.pa.us wrote:

The biggest part of the work needed is to write the documentation ---
but we'd have to do that for Abhijit's patch too, since the userlocks
docs presumably fall under GPL along with the code.

I'll write the documentation, either for the code as it is, or for any
replacement we decide to use.

I didn't submit documentation (or a Makefile, uninstall_otherlock.sql,
etc.) only because I didn't know if anything was going to be done with
otherlock now. I just wanted to mention the existence of the code.

So basically I don't see the point of investing effort in a
bug-compatible version of userlocks, when we can have something
cleaner and suitable for the long run with not very much more
effort.

Fine with me. Two questions:

- Where would the code live, if it were in core?
- Shall I hack up the API you suggested in your earlier message?

are we still moving forward with this? I would love to see this go in for 8.2.

merlin

At 2006-09-07 00:16:38 -0400, mmoncure@gmail.com wrote:

- Where would the code live, if it were in core?
- Shall I hack up the API you suggested in your earlier message?

are we still moving forward with this? I would love to see this go in
for 8.2.

I don't know about its going into 8.2 or not, but I'm writing the code,
and I'll submit a patch tomorrow.

-- ams

Abhijit Menon-Sen <ams@oryx.com> writes:

At 2006-09-05 16:35:49 -0400, tgl@sss.pgh.pa.us wrote:

So basically I don't see the point of investing effort in a
bug-compatible version of userlocks, when we can have something
cleaner and suitable for the long run with not very much more
effort.

Fine with me. Two questions:
- Where would the code live, if it were in core?
- Shall I hack up the API you suggested in your earlier message?

Is this going anywhere? The days grow short ...

regards, tom lane