At Fri, 9 Jun 2023 21:28:23 -0700, Andres Freund <and...@anarazel.de> wrote in > Hi, > > On 2023-05-22 13:36:42 +0900, Kyotaro Horiguchi wrote: > > At Sun, 21 May 2023 15:14:23 -0700, Andres Freund <and...@anarazel.de> > > wrote in > > > Hi, > > > > > > I don't really feel confident we're going to get the timeout approach > > > solid > > > enough for something going into the tree around beta 1. > > > > > > How about this, imo a lot simpler, approach: We flush stats whenever > > > replaying > > > a XLOG_RUNNING_XACTS record. Unless the primary is idle, it will log > > > those at > > > a regular interval. If the primary is idle, we don't need to flush stats > > > in > > > the startup process, because we'll not have done any io. > > > > > > We only log XLOG_RUNNING_XACTS when wal_level >= replica, so stats > > > wouldn't > > > get regularly flushed if wal_level = minimal - but in that case the stats > > > are > > > also not accessible, so that's not a problem. > > > > I concur with the three aspects, interval regularity, reliability and > > about the case of the minimal wal_level. While I can't confidently > > claim it is the best, its simplicilty gives us a solid reason to > > proceed with it. If necessary, we can switch to alternative timing > > sources in the future without causing major disruptions for users, I > > believe. > > > > > It's not the prettiest solution, but I think the simplicity is worth a > > > lot. > > > > > > Greetings, > > > > +1. > > Attached a patch implementing this approach. > > It's imo always a separate bug that we were using > GetCurrentTransactionStopTimestamp() when force was passed in - that timestamp > can be quite out of date in some cases. But I don't immediately see a > noticeable consequence, so ...
Thanks! I think that the reason is that we only pass true only when we're in a sort of idle time. Addition to that XLOG_RUNNING_XACTS comes so infrequently to cause noticeable degradation. If it causes sufficiently frequent updates, we will be satisfied with it. The patch is quite simple and looks good to me. (At least way better than always using GetCurrentTimestamp():) regards. -- Kyotaro Horiguchi NTT Open Source Software Center
