Prev: ALTER TABLE...ALTER COLUMN vs inheritance
Next: patch: Add JSON datatype to PostgreSQL (GSoC, WIP)
From: Jaime Casanova on 23 Jul 2010 11:05
On Thu, Jun 10, 2010 at 3:39 PM, Robert Haas <robertmhaas(a)gmail.com> wrote:
>
>
> I believe that this patch will clear away one major obstacle to
> implementing global temporary and unlogged tables: it enables us to be
> sure of cleaning up properly after a crash without relying on catalog
> entries or XLOG.  Based on previous discussions, however, I believe
> there is support for making this change independently of what becomes
> of that project, just for the benefit of having a more robust cleanup
> mechanism.
>

Hi,

i was looking at v3 of this patch...

it looks good, works as advertised, pass regression tests, and all
tests i could think of...
haven't looked the code at much detail but seems ok, to me at least...

--
Jaime Casanova         www.2ndQuadrant.com
Soporte y capacitación de PostgreSQL

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Jaime Casanova on 25 Jul 2010 02:37
On Fri, Jul 23, 2010 at 10:05 AM, Jaime Casanova <jaime(a)2ndquadrant.com> wrote:
> On Thu, Jun 10, 2010 at 3:39 PM, Robert Haas <robertmhaas(a)gmail.com> wrote:
>>
>>
>> I believe that this patch will clear away one major obstacle to
>> implementing global temporary and unlogged tables: it enables us to be
>> sure of cleaning up properly after a crash without relying on catalog
>> entries or XLOG.  Based on previous discussions, however, I believe
>> there is support for making this change independently of what becomes
>> of that project, just for the benefit of having a more robust cleanup
>> mechanism.
>>
>
> Hi,
>
> i was looking at v3 of this patch...
>

Ok, i like what you did in smgrextend, smgrwrite, and others...
changing isTemp for skipFsync is more descriptive

but i have a few questions, maybe is right what you did i only want to
understand it:
- you added this in include/storage/smgr.h, so why is safe to assume
that if the backend != InvalidBackendId it must be a temp relation?

+#define SmgrIsTemp(smgr) \
+ ((smgr)->smgr_rnode.backend != InvalidBackendId)


- you added a question like this "if (rel->rd_backend == MyBackendId)"
in a few places... why are you assuming that? that couldn't be a new
created relation (in current session of course)? is that safe?

--
Jaime Casanova         www.2ndQuadrant.com
Soporte y capacitación de PostgreSQL

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Robert Haas on 25 Jul 2010 17:32
On Sun, Jul 25, 2010 at 2:37 AM, Jaime Casanova <jaime(a)2ndquadrant.com> wrote:
> but i have a few questions, maybe is right what you did i only want to
> understand it:
> - you added this in include/storage/smgr.h, so why is safe to assume
> that if the backend != InvalidBackendId it must be a temp relation?
>
> +#define SmgrIsTemp(smgr) \
> + � ((smgr)->smgr_rnode.backend != InvalidBackendId)

That's pretty much the whole point of the patch. Instead of
identifying relations as simply "temporary" or "not temporary", we
identify as "a temporary relation owned by backend X" or as "not
temporary".

> - you added a question like this "if (rel->rd_backend == MyBackendId)"
> in a few places... why are you assuming that? that couldn't be a new
> created relation (in current session of course)? is that safe?

Again, rd_backend is not the creating backend ID unless the relation
is a temprel.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Jaime Casanova on 29 Jul 2010 02:56
On Sun, Jul 25, 2010 at 4:32 PM, Robert Haas <robertmhaas(a)gmail.com> wrote:
> On Sun, Jul 25, 2010 at 2:37 AM, Jaime Casanova <jaime(a)2ndquadrant.com> wrote:
>> but i have a few questions, maybe is right what you did i only want to
>> understand it:
>> - you added this in include/storage/smgr.h, so why is safe to assume
>> that if the backend != InvalidBackendId it must be a temp relation?
>>
>> +#define SmgrIsTemp(smgr) \
>> +   ((smgr)->smgr_rnode.backend != InvalidBackendId)
>
> That's pretty much the whole point of the patch.  Instead of
> identifying relations as simply "temporary" or "not temporary", we
> identify as "a temporary relation owned by backend X" or as "not
> temporary".
>

Ok, this one seems good enough... i'm marking it as "ready for committer"

--
Jaime Casanova         www.2ndQuadrant.com
Soporte y capacitación de PostgreSQL

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Tom Lane on 5 Aug 2010 19:05
Robert Haas <robertmhaas(a)gmail.com> writes:
> [ BackendRelFileNode patch ]

One thing that I find rather distressing about this is the 25% bloat
in sizeof(SharedInvalidationMessage). Couldn't we avoid that? Is it
really necessary to *ever* send an SI message for a backend-local rel?

I agree that one needs to send relcache inval sometimes for temp rels,
but I don't see why each backend couldn't interpret that as a flush
on its own local version.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

 | 
Pages: 1
Prev: ALTER TABLE...ALTER COLUMN vs inheritance
Next: patch: Add JSON datatype to PostgreSQL (GSoC, WIP)