Re: v12.0: segfault in reindex CONCURRENTLY

On Wed, Oct 16, 2019 at 04:11:46PM -0500, Justin Pryzby wrote:
> On Sun, Oct 13, 2019 at 04:18:34PM -0300, Alvaro Herrera wrote:
>> (FWIW I expect the crash is possible not just in reindex but also in
>> CREATE INDEX CONCURRENTLY.)
>
> FWIW, for sake of list archives, and for anyone running v12 hoping to avoid
> crashing, I believe we hit this for DROP INDEX CONCURRENTLY, although I don't
> have the backtrace to prove it.

You may not have a backtrace, but I think that you are right:
WaitForLockers() gets called in index_drop() with progress reporting
enabled. index_drop() would also be taken by REINDEX CONCURRENTLY
through performMultipleDeletions() but we cannot know if it gets used
for REINDEX CONCURRENTLY or for DROP INDEX CONCURRENTLY as it goes
through the central deletion machinery, so we have to mark progress
reporting as true anyway. Maybe that's worth a comment in index_drop
when calling WaitForLockers() because it is not actually that obvious,
say like that:
@@ -2157,7 +2157,10 @@ index_drop(Oid indexId, bool concurrent, bool
concurrent_lock_mode)

/*
* Wait till every transaction that saw the old index state has
- * finished.
+ * finished. Progress reporting is enabled here for REINDEX
+ * CONCURRENTLY, but not for DROP INDEX CONCURRENTLY. Track
+ * the progress through WaitForLockers() anyway, the information
+ * will not show up if using DROP INDEX CONCURRENTLY.
*/
WaitForLockers(heaplocktag, AccessExclusiveLock, true);

Thoughts?
--
Michael