Re: Freeze the inserted tuples during CTAS?

2021-03-15 Thread Paul Guo
>   to set visibility map bits on materialized views. I'll start a new
>thread to discuss that.

Thanks. Also I withdrew the patch.



Re: Freeze the inserted tuples during CTAS?

2021-03-10 Thread Masahiko Sawada
On Wed, Mar 10, 2021 at 3:57 PM Paul Guo  wrote:
>
> > On Mar 3, 2021, at 1:35 PM, Masahiko Sawada  wrote:
> >> On Sun, Feb 21, 2021 at 4:46 PM Paul Guo  wrote:
> >> Attached is the v2 version that fixes a test failure due to plan change 
> >> (bitmap index scan -> index only scan).
>
> > I think this is a good idea.
>
> > BTW, how much does this patch affect the CTAS performance? I expect
> > it's negligible but If there is much performance degradation due to
> > populating visibility map, it might be better to provide a way to
> > disable it.
>
> Yes,  this is a good suggestion. I did a quick test yesterday.
>
> Configuration: shared_buffers = 1280M and the test system memory is 7G.
>
> Test queries:
>   checkpoint;
>   \timing
>   create table t1 (a, b, c, d) as select i,i,i,i from 
> generate_series(1,2000) i;
>   \timing
>   select pg_size_pretty(pg_relation_size('t1'));
>
> Here are the running time:
>
> HEAD   : Time: 10299.268 ms (00:10.299)  + 1537.876 ms (00:01.538)
> Patch  : Time: 12257.044 ms (00:12.257)  + 14.247 ms
>
> The table size is 800+MB so the table should be all in the buffer. I was 
> surprised
> to see the patch increases the CTAS time by 19.x%, and also it is not better 
> than
> "CTAS+VACUUM" on HEAD version. In theory the visibility map buffer change 
> should
> not affect that much. I looked at related code again (heap_insert()). I 
> believe
> the overhead could decrease along with some discussed CTAS optimization
> solutions (multi-insert, or raw-insert, etc).
>
> I tested 'copy' also. The COPY FREEZE does not involve much overhead than COPY
> according to the experiement results as below. COPY uses multi-insert. Seems 
> there is
> no other difference than CTAS when writing a new table.
>
> COPY TO + VACUUM
> Time: 8826.995 ms (00:08.827) + 1599.260 ms (00:01.599)
> COPY TO FREEZE + VACUUM
> Time: 8836.107 ms (00:08.836) + 13.581 ms
>
> So maybe think about doing freeze in CTAS after optimizing the CTAS 
> performance
> later?

Thank you for testing. That's interesting.

I've also done some benchmarks for CTAS (2GB table creation) and got
similar results:

Patched : 44 sec
HEAD : 34 sec

Since CREATE MATERIALIZED VIEW is also internally treated as CTAS, I
got similar results even for CREATE MATVIEW.

After investigation, it seems to me that the cause of performance
degradation is that heap_insert() set PD_ALL_VISIBLE when inserting a
tuple for the first time on the page (L2133 in heapam.c). This
requires every subsequent heap_insert() to pin a visibility map buffer
(see RelationGetBufferForTuple()). This problem doesn't exist in
heap_multi_insert() since it reads vm buffer once to fill the heap
page with tuples.

Given such relatively big performance degradation, it seems to me that
we should do some optimization for heap_insert() first. Otherwise, we
will end up imposing those costs on all users.

> By the way, ‘REFRESH MatView’ does freeze by default. Matview is quite 
> similar to CTAS.
> I did test it also and the conclusion is similar to that of CTAS. Not sure 
> why FREEZE was
> enabled though, maybe I missed something?

I’m not sure the reason why setting visibility map and PD_ALL_VISIBLE
during REFRESH MATVIEW is enabled by default. By commit 7db0cd2145 and
39b66a91b, heap_insert and heap_multi_insert() sets visibility map
bits if HEAP_INSERT_FROZEN is specified. Looking at the commit
messages, those changes seem to be intended to COPY FREEZE but they
indeed affect REFRESH MATVIEW as well. But I could not find discussion
and mention about REFRESH MATVIEW both in those threads and commit
messages.

One reason that might justify such behavior would be materialized
views are read-only. Since visibility map bits never be cleared, even
if it costs here is some cost to set visibility map during the refresh
setting VM bits and PD_ALL_VISIBLE at creation might win. On the other
hand, a table created by CTAS is read-write. The user might not want
to pay a cost during creating a table if the table is updated
frequently after creation. Not sure. Being said that, I think this
performance degradation of REFRESH MATVIEW could be a problem. There
is no way to avoid the degradation and we also can rely on autovacuum
to set visibility map bits on materialized views. I'll start a new
thread to discuss that.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/




Re: Freeze the inserted tuples during CTAS?

2021-03-09 Thread Paul Guo
> On Mar 3, 2021, at 1:35 PM, Masahiko Sawada  wrote:
>> On Sun, Feb 21, 2021 at 4:46 PM Paul Guo  wrote:
>> Attached is the v2 version that fixes a test failure due to plan change 
>> (bitmap index scan -> index only scan).

> I think this is a good idea.

> BTW, how much does this patch affect the CTAS performance? I expect
> it's negligible but If there is much performance degradation due to
> populating visibility map, it might be better to provide a way to
> disable it.

Yes,  this is a good suggestion. I did a quick test yesterday.

Configuration: shared_buffers = 1280M and the test system memory is 7G.

Test queries:
  checkpoint;
  \timing
  create table t1 (a, b, c, d) as select i,i,i,i from 
generate_series(1,2000) i;
  \timing
  select pg_size_pretty(pg_relation_size('t1'));

Here are the running time:

HEAD   : Time: 10299.268 ms (00:10.299)  + 1537.876 ms (00:01.538)  
   
Patch  : Time: 12257.044 ms (00:12.257)  + 14.247 ms
 

The table size is 800+MB so the table should be all in the buffer. I was 
surprised
to see the patch increases the CTAS time by 19.x%, and also it is not better 
than
"CTAS+VACUUM" on HEAD version. In theory the visibility map buffer change should
not affect that much. I looked at related code again (heap_insert()). I believe
the overhead could decrease along with some discussed CTAS optimization
solutions (multi-insert, or raw-insert, etc).

I tested 'copy' also. The COPY FREEZE does not involve much overhead than COPY
according to the experiement results as below. COPY uses multi-insert. Seems 
there is
no other difference than CTAS when writing a new table.

COPY TO + VACUUM
Time: 8826.995 ms (00:08.827) + 1599.260 ms (00:01.599)
COPY TO FREEZE + VACUUM
Time: 8836.107 ms (00:08.836) + 13.581 ms

So maybe think about doing freeze in CTAS after optimizing the CTAS performance
later?

By the way, ‘REFRESH MatView’ does freeze by default. Matview is quite similar 
to CTAS.
I did test it also and the conclusion is similar to that of CTAS. Not sure why 
FREEZE was
enabled though, maybe I missed something?



Re: Freeze the inserted tuples during CTAS?

2021-03-02 Thread Masahiko Sawada
On Sun, Feb 21, 2021 at 4:46 PM Paul Guo  wrote:
>
> Attached is the v2 version that fixes a test failure due to plan change 
> (bitmap index scan -> index only scan).

I think this is a good idea.

BTW, how much does this patch affect the CTAS performance? I expect
it's negligible but If there is much performance degradation due to
populating visibility map, it might be better to provide a way to
disable it.

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/




Re: Freeze the inserted tuples during CTAS?

2021-02-20 Thread Paul Guo
Attached is the v2 version that fixes a test failure due to plan change (bitmap 
index scan -> index only scan).


v2-0001-Freeze-the-tuples-during-CTAS.patch
Description: v2-0001-Freeze-the-tuples-during-CTAS.patch


Re: Freeze the inserted tuples during CTAS?

2021-02-18 Thread Paul Guo


On Jan 27, 2021, at 5:33 PM, Darafei Komяpa Praliaskouski 
mailto:m...@komzpa.net>> wrote:

Hi,

I confirm that my analytic workflows often do the CTAS and VACUUM of the 
relation right after, before the index creation, to mark stuff as all-visible 
for IOS to work. Freezing and marking as visible will help.

Thanks for letting me know there is such a real case in production environment.
I attached the short patch. If no more other concerns, I will log the patch on 
commitfest.


-Paul


On Wed, Jan 27, 2021 at 12:29 PM Paul Guo 
mailto:gu...@vmware.com>> wrote:
Here is the simple patch,

diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index dce882012e..0391699423 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -552,7 +552,7 @@ intorel_startup(DestReceiver *self, int operation, 
TupleDesc typeinfo)
myState->rel = intoRelationDesc;
myState->reladdr = intoRelationAddr;
myState->output_cid = GetCurrentCommandId(true);
-   myState->ti_options = TABLE_INSERT_SKIP_FSM;
+   myState->ti_options = TABLE_INSERT_SKIP_FSM | TABLE_INSERT_FROZEN;

MatView code already does this and COPY does this if specified. I’m not sure how
does the community think about this. Actually personally I expect more about the
all-visible setting due to TABLE_INSERT_FROZEN since I could easier use index 
only scan
if we create an index and table use CTAS, else people have to use index only 
scan
after vacuum. If people do not expect freeze could we at least introduce a 
option to
specify the visibility during inserting?

Regards,
Paul




v1-0001-Freeze-the-tuples-during-CTAS.patch
Description: v1-0001-Freeze-the-tuples-during-CTAS.patch


Re: Freeze the inserted tuples during CTAS?

2021-01-27 Thread Komяpa
Hi,

I confirm that my analytic workflows often do the CTAS and VACUUM of the
relation right after, before the index creation, to mark stuff as
all-visible for IOS to work. Freezing and marking as visible will help.

On Wed, Jan 27, 2021 at 12:29 PM Paul Guo  wrote:

> Here is the simple patch,
>
> diff --git a/src/backend/commands/createas.c
> b/src/backend/commands/createas.c
> index dce882012e..0391699423 100644
> --- a/src/backend/commands/createas.c
> +++ b/src/backend/commands/createas.c
> @@ -552,7 +552,7 @@ intorel_startup(DestReceiver *self, int operation,
> TupleDesc typeinfo)
> myState->rel = intoRelationDesc;
> myState->reladdr = intoRelationAddr;
> myState->output_cid = GetCurrentCommandId(true);
> -   myState->ti_options = TABLE_INSERT_SKIP_FSM;
> +   myState->ti_options = TABLE_INSERT_SKIP_FSM | TABLE_INSERT_FROZEN;
>
> MatView code already does this and COPY does this if specified. I’m not
> sure how
> does the community think about this. Actually personally I expect more
> about the
> all-visible setting due to TABLE_INSERT_FROZEN since I could easier use
> index only scan
> if we create an index and table use CTAS, else people have to use index
> only scan
> after vacuum. If people do not expect freeze could we at least introduce a
> option to
> specify the visibility during inserting?
>
> Regards,
> Paul



-- 
Darafei "Komяpa" Praliaskouski
OSM BY Team - http://openstreetmap.by/


Freeze the inserted tuples during CTAS?

2021-01-27 Thread Paul Guo
Here is the simple patch,

diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index dce882012e..0391699423 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -552,7 +552,7 @@ intorel_startup(DestReceiver *self, int operation, 
TupleDesc typeinfo)
myState->rel = intoRelationDesc;
myState->reladdr = intoRelationAddr;
myState->output_cid = GetCurrentCommandId(true);
-   myState->ti_options = TABLE_INSERT_SKIP_FSM;
+   myState->ti_options = TABLE_INSERT_SKIP_FSM | TABLE_INSERT_FROZEN;

MatView code already does this and COPY does this if specified. I’m not sure how
does the community think about this. Actually personally I expect more about the
all-visible setting due to TABLE_INSERT_FROZEN since I could easier use index 
only scan
if we create an index and table use CTAS, else people have to use index only 
scan
after vacuum. If people do not expect freeze could we at least introduce a 
option to
specify the visibility during inserting?

Regards,
Paul