On Sun, Apr 1, 2018 at 3:30 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On 31 March 2018 at 14:21, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> I think the vacuum assigns xids only if it needs to truncate some of
>> the pages in the relation which happens towards the end of vacuum.
>> So, it shouldn't hold back the xmin horizon for long.
>
> Yes, that's the reason. I recall VACUUMs giving lots of problems
> during development of Hot Standby.
>
> VACUUM FULL was the thing that needed to be excluded in the past
> because it needed an xid to move rows.
>
> Greg's concern is a good one and his noticing that we hadn't
> specifically excluded VACUUMs is valid, so we should exclude them.
> Well spotted, Greg.
>
> So although this doesn't have the dramatic effect it might have had,
> there is still the possibility of some effect and I think we should
> treat it as a bug.
>
+1. I think you missed to update the comments on top of the modified
function ("Similar to GetSnapshotData but returns more information. We
include all PGXACTs with an assigned TransactionId, even VACUUM
processes." ). It seems last part of the sentence should be omitted
after your patch, otherwise, patch looks good to me.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com