On Sun, Mar 13, 2022 at 01:54:10PM +0530, Bharath Rupireddy wrote:
> Thanks for reviewing this. I agree with all of the above suggestions
> and incorporated them in the v2 patch.
Thanks for the new patch.
> Another thing I added in v2 is to not emit snapshot and mapping files
> stats in case of restartpoint as logical decoding isn't supported on
> standbys, so it doesn't make sense to emit the stats there and cause
> server log to grow unnecessarily. Having said that, I added a note
> there to change it whenever logical decoding on standbys is supported.
I think we actually do want to include this information for restartpoints
since some files might be left over from the snapshot that was used to
create the standby. Also, presumably these functions could do some work
during recovery on a primary.
Another problem I see is that this patch depends on the return value of the
lstat() calls that we are trying to remove in 0001 from another thread [0].
I think the size of the removed/sync'd files is somewhat useful for
understanding disk space usage, but I suspect the time spent performing
these tasks is more closely related to the number of files. Do you think
reporting the sizes is worth the extra system call?
[0] https://postgr.es/m/20220215231123.GA2584239%40nathanxps13
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
