On Fri, Nov 13, 2020 at 12:11:21PM -0600, Justin Pryzby wrote:
> On Fri, Nov 13, 2020 at 01:39:31PM -0300, Alvaro Herrera wrote:
> > On 2020-Nov-13, Justin Pryzby wrote:
> >
> > > I saw a bunch of these in my logs:
> > >
> > > log_time | 2020-10-25 22:59:45.619-07
> > > database |
> > > left | could not open relation with OID 292103095
> > > left | processing work entry for relation
> > > "ts.child.alarms_202010_alarm_clear_time_idx"
> > >
> > > Those happen following a REINDEX job on that index.
> > >
> > > I think that should be more like an INFO message, since that's what
> > > vacuum does
> > > (vacuum_open_relation), and a queued work item is even more likely to hit
> > > a
> > > dropped relation.
> >
> > Ah, interesting. Yeah, I agree this is a bug. I think it can be fixed
> > by using try_relation_open() on the index; if that returns NULL, discard
> > the work item.
> >
> > Does this patch solve the problem?
>
> Your patch didn't actually say "try_relation_open", so didn't work.
> But it does works if I do that, and close the table.
That patch broke the case that a non-index is passed, which I addressed here.
I wondered if the function should return NULL in those cases, but it seems to
be "impossible".
>From ef5685253a04cf720574f21b711e177e13299ba3 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Fri, 13 Nov 2020 13:39:31 -0300
Subject: [PATCH v3] Avoid errors in brin summarization..
..which can happen if an index is reindexed concurrently
---
src/backend/access/brin/brin.c | 22 +++++++++++++++-------
src/test/regress/expected/brin.out | 6 +++++-
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 1f72562c60..46e5269260 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -887,16 +887,24 @@ brin_summarize_range(PG_FUNCTION_ARGS)
/*
* We must lock table before index to avoid deadlocks. However, if the
* passed indexoid isn't an index then IndexGetRelation() will fail.
- * Rather than emitting a not-very-helpful error message, postpone
- * complaining, expecting that the is-it-an-index test below will fail.
+ * Rather than emitting a not-very-helpful error message, prepare to
+ * return without doing anything. This allows autovacuum work-items to be
+ * silently discarded rather than uselessly accumulating error messages in
+ * the server log.
*/
heapoid = IndexGetRelation(indexoid, true);
- if (OidIsValid(heapoid))
- heapRel = table_open(heapoid, ShareUpdateExclusiveLock);
- else
- heapRel = NULL;
+ heapRel = OidIsValid(heapoid) ?
+ table_open(heapoid, ShareUpdateExclusiveLock) : NULL;
- indexRel = index_open(indexoid, ShareUpdateExclusiveLock);
+ if (heapRel == NULL)
+ PG_RETURN_INT32(0);
+
+ indexRel = try_relation_open(indexoid, ShareUpdateExclusiveLock);
+ if (indexRel == NULL)
+ {
+ table_close(heapRel, ShareUpdateExclusiveLock);
+ PG_RETURN_INT32(0);
+ }
/* Must be a BRIN index */
if (indexRel->rd_rel->relkind != RELKIND_INDEX ||
diff --git a/src/test/regress/expected/brin.out b/src/test/regress/expected/brin.out
index e53d6e4885..51dff7295d 100644
--- a/src/test/regress/expected/brin.out
+++ b/src/test/regress/expected/brin.out
@@ -405,7 +405,11 @@ UPDATE brintest SET int8col = int8col * int4col;
UPDATE brintest SET textcol = '' WHERE textcol IS NOT NULL;
-- Tests for brin_summarize_new_values
SELECT brin_summarize_new_values('brintest'); -- error, not an index
-ERROR: "brintest" is not an index
+ brin_summarize_new_values
+---------------------------
+ 0
+(1 row)
+
SELECT brin_summarize_new_values('tenk1_unique1'); -- error, not a BRIN index
ERROR: "tenk1_unique1" is not a BRIN index
SELECT brin_summarize_new_values('brinidx'); -- ok, no change expected
--
2.17.0
