On Thu, Jul 16, 2020 at 7:34 PM David Johnston
<david.g.johnston@gmail.com> wrote:
>
> The following review has been posted through the commitfest application:
> make installcheck-world: not tested
> Implements feature: not tested
> Spec compliant: not tested
> Documentation: tested, passed
>
> James,
>
> I'm on board with the point of pointing out explicitly the "concurrent index builds on multiple tables at the same
timewill not return on any one table until all have completed", with back-patching. I do not believe the new paragraph
isnecessary though. I'd suggest trying to weave it into the existing paragraph ending "Even then, however, the index
maynot be immediately usable for queries: in the worst case, it cannot be used as long as transactions exist that
predatethe start of the index build." Adding "Notably, " in front of the existing sentence fragment above and tacking
itonto the end probably suffices.
I'm not sure "the index may not be immediately usable for queries" is
really accurate/sufficient: it seems to imply the CREATE INDEX has
returned but for some reason the index isn't yet valid. The issue I'm
trying to describe here is that the CREATE INDEX query itself will not
return until all preceding queries have completed *including*
concurrent index creations on unrelated tables.
> I don't actually don't whether this is true behavior though. Is it something our tests do, or could, demonstrate?
It'd take tests that exercise parallelism, but it's pretty simple to
demonstrate (but you do have to catch the first index build in a scan
phase, so you either need lots of data or a hack). Here's an example
that uses a bit of a hack to simulate a slow scan phase:
Setup:
create table items(i int);
create table others(i int);
create function slow_expr() returns text as $$ select pg_sleep(15);
select '5'; $$ language sql immutable;
insert into items(i) values (1), (2);
insert into others(i) values (1), (2);
Then the following in order:
1. In session A: create index concurrently on items((i::text || slow_expr()));
2. In session B (at the same time): create index concurrently on others(i);
You'll notice that the 2nd command, which should be practically
instantaneous, waits on the first ~30s scan phase of (1) before it
returns. The same is true if after (2) completes you immediately run
it again -- it waits on the second ~30s scan phase of (1).
That does reveal a bit of complexity though that that the current
patch doesn't address, which is that this can be phase dependent (and
that complexity gets a lot more non-obvious when there's real live
activity (particularly long-running transactions) in the system as
well.
I've attached a new patch series with two items:
1. A simpler (and I believe more correct) doc changes for "cic blocks
cic on other tables".
2. A patch to document that all index builds can prevent tuples from
being vacuumed away on other tables.
If it's preferable we could commit the first and discuss the second
separately, but since that limitation was also discussed up-thread, I
decided to include them both here for now.
James