Re: [Patch] Temporary tables that do not bloat pg_catalog (a.k.a fast temp tables) - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: [Patch] Temporary tables that do not bloat pg_catalog (a.k.a fast temp tables)
Date
Msg-id 26c3bc60-6613-e7ab-517a-53136f01fb6c@2ndquadrant.com
Whole thread Raw
In response to Re: [Patch] Temporary tables that do not bloat pg_catalog (a.k.a fast temp tables)  (Aleksander Alekseev <a.alekseev@postgrespro.ru>)
Responses Re: [Patch] Temporary tables that do not bloat pg_catalog (a.k.a fast temp tables)  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On 08/01/2016 11:45 AM, Aleksander Alekseev wrote:
> Hello.
>
> Thanks everyone for great comments!
>
>> Well, jokes aside, that's a pretty lousy excuse for not writing any
>> docs
>
> I think maybe I put it in a wrong way. There are currently a lot of
> comments in a code, more then enough to understand how this feature
> works. What I meant is that this is not a final version of a patch and
> a few paragraphs are yet to be written. At least it's how I see it. If
> you believe that some parts of the code are currently hard to understand
> and some comments could be improved, please name it and I will be happy
> to fix it.

I don't think there's "a lot of comments in the code", not even
remotely. At least not in the files I looked into - heapam, indexam,
xact etc. There are a few comments in general, and most of them only
comment obvious facts, like "ignore in-memory tuples" right before a
trivial if statement.

What is needed is an overview of the approach, so that the reviewers can
read that first, instead of assembling the knowledge from pieces
scattered over comments in many pieces. But I see the fasttab.c contains
this:

/* TODO TODO comment the general idea - in-memory tuples and indexes,
hooks principle, FasttabSnapshots, etc */

The other thing that needs to happen is you need to modify comments in
front of some of the modified methods - e.g. the comments may need a
paragraph "But when the table is fast temporary, what happens is ..."

>
>> IMHO if this patch gets in, we should use it as the only temp table
>> implementation (Or can you think of cases where keeping rows in
>> pg_class has advantages?). That'd also eliminate the need for FAST
>> keyword in the CREATE TABLE command.
>
>> Probably has zero value to have slow and fast temp tables (from
>> catalogue cost perspective). So the FAST implementation should be used
>> everywhere.
>
> If there are no objections I see no reason no to do it in a next
> version of a patch.

I believe there will be a lot of discussion about this.

>
>> I'm getting failures in the regression suite
>
> I've run regression suite like 10 times in a row in different
> environments with different build flags but didn't manage to reproduce
> it. Also our DBAs are testing this feature for weeks now on real-world
> applications and they didn't report anything like this. Could you
> please describe how to reproduce this issue?
>

Nothing special:

$ ./configure --prefix=/home/user/pg-temporary --enable-debug \
   --enable-cassert

$ make -s clean && make -s -j4 install

$ export PATH=/home/user/pg-temporary/bin:$PATH

$ pg_ctl -D ~/tmp/data-temporary init

$ pg_ctl -D ~/tmp/data-temporary -l ~/temporary.log start

$ make installcheck

I get the failures every time - regression diff attached. The first
failure in "rolenames" is expected, because of clash with existing user
name. The remaining two failures are not.

I only get the failure for "installcheck" but not "check" for some reason.

regards

--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment

pgsql-hackers by date:

Previous
From: Etsuro Fujita
Date:
Subject: Confusing docs about GetForeignUpperPaths in fdwhandler.sgml
Next
From: Hannu Krosing
Date:
Subject: Re: Why we lost Uber as a user