On Fri, Jul 31, 2020 at 2:51 PM James Coleman <jtc...@gmail.com> wrote: > > On Thu, Jul 16, 2020 at 7:34 PM David Johnston > <david.g.johns...@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 time will not return on any one > > table until all have completed", with back-patching. I do not believe the > > new paragraph is necessary though. I'd suggest trying to weave it into the > > existing paragraph ending "Even then, however, the index may not be > > immediately usable for queries: in the worst case, it cannot be used as > > long as transactions exist that predate the start of the index build." > > Adding "Notably, " in front of the existing sentence fragment above and > > tacking it onto 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.
Álvaro's patch confused the current state of this thread, so I'm reattaching (rebased) v2 as v3. James
v3-0002-Document-vacuum-on-one-table-depending-on-concurr.patch
Description: Binary data
v3-0001-Document-concurrent-indexes-waiting-on-each-other.patch
Description: Binary data
