Re: Fixing findDependentObjects()'s dependency on scan order (regressions in DROP diagnostic messages)

From: Peter Geoghegan <pg(at)bowt(dot)ie>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fixing findDependentObjects()'s dependency on scan order (regressions in DROP diagnostic messages)
Date: 2019-01-18 01:09:20
Message-ID: CAH2-WzkMFfJb0_qCZQ2vxrF2fW3xEio0fG-MBT7ky8-9a13XKQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jan 17, 2019 at 4:40 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Yeah, that's the policy we've followed so far, but I remain concerned
> about its effects on the regression tests. There are a lot of places
> where we print full DROP CASCADE output because "it hasn't failed yet".
> I fear every one of those is a gotcha that's waiting to bite us.

Right. I don't want to have to play whack-a-mole with this later on,
and risk suppressing the wrong thing.

> Also, is index scan order really guaranteed for equal-keyed items?
> It isn't today, and I didn't think your patches were going to make
> it so.

The idea is that you'd have an extra column, so they wouldn't be
equal-keyed (the keys that the scan was interested in would be
duplicated, but we could still rely on the ordering). Are you
suggesting that there'd be a stability issue regardless?

My patch does guarantee order for equal-keyed items, since heap TID is
treated as just another attribute, at least as far as the nbtree key
space is concerned. The big unknown is how exactly that works out when
the regression tests are run in parallel, on a busy system, since heap
TID isn't just another column outside of nbtree. I think that this
will probably cause test flappiness if we don't nail it down now. That
was certainly my experience before I nailed it down in my own
tentative way. My sense is that without addressing this issue, we're
going to be sensitive to concurrent heap TID recycling by VACUUM in a
way that might become an annoyance. I see no reason to not go fix it
once and for all, since the vast majority of all problems are around
the two pg_depend indexes.

> We're going to stick all these items into an ObjectAddress array anyway,
> so at worst it'd be 2X growth, most likely a lot less since we'd only
> be sorting one level of dependency at a time.

It sounds like we don't have a good reason to not just sort them
explicitly, then. I'm happy to go that way. I mostly just wanted to be
sure that you were aware that we could add a tie-breaker column
without any storage overhead.

> > As I've pointed out a couple of times already, we can add a 4 byte
> > tie-breaker column to both pg_depend indexes without increasing the
> > size of the on-disk representation, since the extra space is already
> > lost to alignment (we could even add a new 4 byte column to the table
> > without any storage overhead, if that happened to make sense).
>
> Meh ... where do you get the 4-byte value from?

In the kludgey patch that I posted, the 4-byte value is manufactured
artificially within a backend in descending order. That may have a
slight advantage over object oid, even after the pg_depend correctness
issues are addressed. A fixed order within a backend or originating
transaction seems like it might avoid a few remaining instability
issues. Not sure. I seem to recall there being some disagreement
between you and Andres on this very point (is object oid a perfectly
stable tie-breaker in practice?) on a similar thread from 2017.

--
Peter Geoghegan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2019-01-18 01:15:23 Re: Fixing findDependentObjects()'s dependency on scan order (regressions in DROP diagnostic messages)
Previous Message Tom Lane 2019-01-18 01:08:07 Re: pgsql: Restrict the use of temporary namespace in two-phase transaction