Re: [DOC] Document concurrent index builds waiting on each other
David G. Johnston <david.g.johnston@gmail.com>
From: "David G. Johnston" <david.g.johnston@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: James Coleman <jtc331@gmail.com>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>,
Andres Freund <andres@anarazel.de>, Alvaro Herrera <alvherre@2ndquadrant.com>
Date: 2020-10-21T22:25:51Z
Lists: pgsql-hackers
On Wed, Sep 30, 2020 at 2:10 AM Michael Paquier <michael@paquier.xyz> wrote: > On Tue, Sep 08, 2020 at 01:25:21PM -0400, James Coleman wrote: > > Álvaro's patch confused the current state of this thread, so I'm > > reattaching (rebased) v2 as v3. > > + <para> > + <command>CREATE INDEX</command> (including the > <literal>CONCURRENTLY</literal> > + option) commands are included when <command>VACUUM</command> > calculates what > + dead tuples are safe to remove even on tables other than the one being > indexed. > + </para> > FWIW, this is true as well for REINDEX CONCURRENTLY because both use > the same code paths for index builds and validation, with basically > the same waiting phases. But is CREATE INDEX the correct place for > that? Wouldn't it be better to tell about such things on the VACUUM > doc? > > 0001 sounds fine to me. > > v3-0002 needs a rebase over the create_index.sgml page due to the change of the nearby xref to link. Attached as v4-0002 along with the original v3-0001. I resisted the temptation to commit my word-smithing thoughts to the affected paragraph. The word "phase" appearing out of nowhere struck me a bit oddly. "Then finally the" feels like it is missing a couple of commas - or just drop the finally. "then two table scans occur in separate transactions" reads better than "two more transactions" IMO. For 0002 maybe focus on the fact that CREATE INDEX is a global concern even though it only names a single table in any one invocation. As a consequence, while it is running, vacuum cannot bring the system's oldest xid more current than the oldest xid on any index-in-progress table (I don't know exactly how this works). And, rehasing 0001, all concurrent indexing will finish at the same time. In short maybe focus less on procedure and specific waiting states and more on the user-visible consequences. 0001 didn't really clear things up much in that regard. It reads like we are introducing a deadlock situation even though that evidently is not the case. I concur that vacuum's perspective on the create index global reach needs to be addressed there if it is not already. <starts looking at vacuum> I'm a bit confused as to why/whether create index transactions are somehow special in this regard, compared to other transactions. I infer from the existence of 0002 that they somehow are... My conclusion thus far is that with respect to the original complaint: On 2019-09-18 13:51:00 -0400, James Coleman wrote: > In my experience it's not immediately obvious (even after reading the > documentation) the implications of how concurrent index builds manage > transactions with respect to multiple concurrent index builds in > flight at the same time. These two limited scope patches have not materially moved the needle in understanding. They are too technical when the underlying issue is comprehension by non-technical people in terms of how they see their system behave. David J.
Commits
-
Call out vacuum considerations in create index docs
- c285a244f6d3 13.2 landed
- 93c39f987e9c 14.0 landed
- 259b21233032 12.6 landed
-
Document concurrent indexes waiting on each other
- ed9c9b033546 11.11 landed
- d3bd36a63d69 10.16 landed
- b3d33bf598dd 9.6.21 landed
- b2603f16ad75 12.6 landed
- 968a537b432e 9.5.25 landed
- 58ebe967f8a1 14.0 landed
- 3fe0e7c3fa27 13.2 landed
-
snapshot scalability: Don't compute global horizons while building snapshots.
- dc7420c2c927 14.0 cited
-
Remove PROC_IN_ANALYZE and derived flags
- cea3d5589865 14.0 landed
-
Improve performance of get_actual_variable_range with recently-dead tuples.
- 3ca930fc39cc 11.0 cited