Re: [GENERAL] pg_upgrade ?deficiency

On Mon, Nov 25, 2013 at 02:24:25PM -0800, Kevin Grittner wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > Kevin Grittner wrote:
> >> Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> >>
> >>> I am not a fan of backpatching any of this.
> >>
> >> Here's my problem with that.  Here's setup to create what I
> >> don't think is all that weird a setup:
> >>
> >> The cluster is created in the state that was dumped, default
> >> read only flags and all.
> >>
> >> Are you saying that you find current behavior acceptable in back
> >> branches?
> >
> > First, I don't need to see a 300-line pg_dump restore output to
> > know it is a bug.
>
> I was trying to save people the trouble of working up their own
> test to see what happened when running pg_dumpall output from a
> successful run into a freshly initdb'd cluster.  No offense
> intended.

Fine, but were 300 lines of output necessary?

> > Second, what you didn't do is to address the rest of my
> > paragraph:
> >
> >> I am not a fan of backpatching any of this.  We have learned the
> >> fix is more complex than thought,
>
> I didn't realize that anyone thought the pg_dumpall fix would
> amount to something simpler than two printf statements; but even
> so, that seems pretty reasonable to me.

Well, if I say "more complex than thought", it means not just the patch,
but the impact it might have. We are pushing out a replication fix in a
few weeks because of backpatches that people thought were safe.

For example, if we backpatch, we get zero user testing. If we did it
wrong somehow, it is much harder to fix later. For example, what is the
logic that a per-database setting of statement_timeout is reset in
pg_dump, but default_transaction_read_only is reset in pg_dumpall? Can
we know we have thought this all through with zero user testing?

> >> and the risk of breakage and having pg_dump diffs change between
> >> minor releases doesn't seem justified.
>
> You might have missed the part of the thread where we seemed to
> have reached a consensus that pg_dump output would not change at
> all; only pg_dumpall output -- to something capable of being
> restored to a fresh cluster.

Yes, I saw that. My point was that we have had to move around the patch
to fix the problem, meaning the fix was not obvious. The number of
lines is only part of a measure of a patch's riskiness.

> > We have to balance the _one_ user failure report we have received
> > vs. potential breakage.
>
> What breakage mode do you see?

Uh, I said "potential breakage." If I knew of the breakage, I would
have said so.

> > Now, others seem to be fine with a backpatch, so perhaps it is
> > safe.  I am merely pointing out that, with all backpatching, we
> > have to balance the fix against possible breakage and behavioral
> > change.
>
> Sure, but I think a bug in a dump utility which allows it to run
> with apparent success while generating results which don't restore
> is pretty clearly something to fix.  FWIW I don't feel as strongly

pg_dumpall certainly reduces the diff risk.

> about the pg_upgrade aspect of this -- as long as a test run
> reports that the cluster can't be upgraded without changing those
> settings, there is no problem.  Of course it would be nice if it
> could be fixed instead, but that's not as critical in my view.

How are we handling breakage of pg_dump, not pg_dumpall? doc patch?
pg_upgrade probably needs a doc patch too, or a reset like pg_dumpall.
pg_upgrade is more like pg_dumpall, so a code patch seems most logical,
again, assuming we decide that pg_dumpall is the right place for this
reset of default_transaction_read_only.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +