Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly

From: Steve Singer <steve(at)ssinger(dot)info>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Alexey Kondratov <kondratov(dot)aleksey(at)gmail(dot)com>
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
Date: 2019-11-17 00:53:27
Message-ID: 157395200750.29912.1178609357962324139.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, failed
Spec compliant: not tested
Documentation: tested, failed

* I had to replace heap_open/close with table_open/close to get the
patch to compile against master

In the documentation

+ <para>
+ This specifies a tablespace, where all rebuilt indexes will be created.
+ Can be used only with <literal>REINDEX INDEX</literal> and
+ <literal>REINDEX TABLE</literal>, since the system indexes are not
+ movable, but <literal>SCHEMA</literal>, <literal>DATABASE</literal> or
+ <literal>SYSTEM</literal> very likely will has one.
+ </para>

I found the "SCHEMA,DATABASE or SYSTEM very likely will has one." portion confusing and would be inclined to remove it or somehow reword it.

Consider the following

-------------
create index foo_bar_idx on foo(bar) tablespace pg_default;
CREATE INDEX
reindex=# \d foo
Table "public.foo"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
id | integer | | not null |
bar | text | | |
Indexes:
"foo_pkey" PRIMARY KEY, btree (id)
"foo_bar_idx" btree (bar)

reindex=# reindex index foo_bar_idx tablespace tst1;
REINDEX
reindex=# reindex index foo_bar_idx tablespace pg_default;
REINDEX
reindex=# \d foo
Table "public.foo"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
id | integer | | not null |
bar | text | | |
Indexes:
"foo_pkey" PRIMARY KEY, btree (id)
"foo_bar_idx" btree (bar), tablespace "pg_default"
--------

It is a bit strange that it says "pg_default" as the tablespace. If I do
this with a alter table to the table, moving the table back to pg_default
makes it look as it did before.

Otherwise the first patch seems fine.

With the second patch(for NOWAIT) I did the following

T1: begin;
T1: insert into foo select generate_series(1,1000);
T2: reindex index foo_bar_idx set tablespace tst1 nowait;

T2 is waiting for a lock. This isn't what I would expect.

The new status of this patch is: Waiting on Author

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2019-11-17 01:59:01 Re: could not stat promote trigger file leads to shutdown
Previous Message Daniel Gustafsson 2019-11-16 23:01:08 Re: Ought to use heap_multi_insert() for pg_attribute/depend insertions?