Hi,
I'll be responding to a bunch of long review emails in this thread
point by point separately, but just picking out a couple of points
here that jumped out at me:
On Wed, Aug 7, 2019 at 9:18 AM Andres Freund <andres@anarazel.de> wrote:
> > + {
> > + /*
> > + * For the "shared" category, we only discard when the
> > + * rm_undo_status callback tells us we can.
> > + */
>
> Is there a description as to what the rm_status callback is intended to
> do? It currently is mandatory, is that intended? Why does this only
> apply to shared records? And why just for SHARED, not for any of the others?
Yeah, I will respond to this. After recent discussions with Robert
the whole UNDO_SHARED concept looks a bit shaky, and there's a better
way trying to get out -- more on that soon.
> > See
> > + * DiscardWorkerMain.
>
> Hm. This actually reminds me of a complaint I have about this. ISTM that
> the logic for discarding itself should be separate from the discard
> worker. I'd just add that, and a UDF to invoke it, in a separate commit.
That's not a bad idea -- I have a 'pg_force_discard()' SP which I'll
include in my next patchset, which is a bit raw, which I'm planning to
make a bit smarter -- it might make sense to use the same code path
for that.
--
Thomas Munro
https://enterprisedb.com