Re: CommitFest 2009-11: Two weeks (and a little) update

From: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: Jaime Casanova <jcasanov(at)systemguards(dot)com(dot)ec>
Cc: Bernd Helmle <bernd(at)oopsware(dot)de>, Greg Smith <greg(at)2ndquadrant(dot)com>, pgsql-rrreviewers(at)postgresql(dot)org
Subject: Re: CommitFest 2009-11: Two weeks (and a little) update
Date: 2009-12-03 02:44:14
Message-ID: 4B1725FE.4000308@ak.jp.nec.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-rrreviewers

Jaime Casanova wrote:
> On Wed, Dec 2, 2009 at 4:13 PM, Bernd Helmle <bernd(at)oopsware(dot)de> wrote:
>>
>> --On 2. Dezember 2009 15:26:19 -0500 Jaime Casanova
>> <jcasanov(at)systemguards(dot)com(dot)ec> wrote:
>>
>>> i have no access to dtrace at all... but now i can take a patch, maybe
>>> another one of those that you mention?
>>
>> Hmm i'm running OS X...i already proposed Greg to take my hands on the
>> Largeobject Access Control Patch, but now it seems better to look at those
>> DTrace things. I originally hesitated to review those since i'm not very
>> familiar with DTrace, but if it helps (and i can improve my skills here),
>> i'm fine with it and you can review the Largeobject patches?
>>
>
> that's fine with me...

Jaime, Thanks for your volunteering.

Since you reviewed this patch at the last commit fest, I think it is not
difficult to check the code. It was just a revised version according to
the comments from Tom Lane.

Greg referenced my previous post which introduced this feature.

http://archives.postgresql.org/message-id/4A9757F6.3010401@ak.jp.nec.com
http://archives.postgresql.org/message-id/4A9B5AD1.3090002@ak.jp.nec.com

Then, this patch is moved to "ready for commiter", but Tom Lane commented
some of his complaints, as follows:

http://archives.postgresql.org/message-id/15563.1254845821@sss.pgh.pa.us
http://archives.postgresql.org/message-id/3224.1255488185@sss.pgh.pa.us

The biggest issue was incompatibility of snapshot on references to
the pg_largeobject_metadata and pg_largeobject system catalogs.

In the previous revision, it always refered the new pg_largeobject_metadata
catalog with SnapshotNow, but inv_api.c may refer the pg_largeobject with
query's snapshot in read-only mode, not always SnapshotNow.
It meaned we could see unvisible changes in the metadata which contains
access privileges, if we apply incompatible snapshot between metadata and
contents.
So, I added pg_largeobject_aclcheck_snapshot() which takes an argument to
give a snapthot to be used to scan pg_largeobeject_metadata. It is invoked
from inv_api.c, and the caller delivers an appropriate snapshot (it may be
SnapshotNow, or not).
Anyway, the point is that we should apply an identical snapshot to scan
both of data(pg_largeobject) and metadata(pg_largeobject_metadata).

The _snapshot() interface seems to you a bit strange, but it has a reason. :-)

In addition, the following points are changed in new revision:
- Syscache support has gone, because it can consume unexpected amount of
local memory when user tries to create massive number of large objects.
- ac_largeobject_*() has gone, because "Reworks for Access Control" patch
was rejected in the lasst commit fest.
- pg_dump support was added.
- \lo_list support multiple server version, not only v8.5 or later.
- documentations and user visible error messages were fixed.
(I surprised at "largeobject" is an incorrect English word. :-)

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

In response to

Browse pgsql-rrreviewers by date

  From Date Subject
Next Message Greg Smith 2009-12-06 15:24:43 Review request: XLogInsert
Previous Message Greg Smith 2009-12-03 00:40:04 Re: CommitFest 2009-11: Two weeks (and a little) update