On Tue, Apr 27, 2021 at 1:18 PM vignesh C <vignes...@gmail.com> wrote: > > On Tue, Apr 27, 2021 at 9:43 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Tue, Apr 27, 2021 at 9:17 AM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > > > > On Tue, Apr 27, 2021 at 11:31 AM vignesh C <vignes...@gmail.com> wrote: > > > > > > > > > > And I think there is > > > > > > also a risk to increase shared memory when we want to add other > > > > > > statistics in the future. > > > > > > > > > > > > > > > > Yeah, so do you think it is not a good idea to store stats in > > > > > ReplicationSlot? Actually storing them in a slot makes it easier to > > > > > send them during ReplicationSlotRelease which is quite helpful if the > > > > > replication is interrupted due to some reason. Or the other idea was > > > > > that we send stats every time we stream or spill changes. > > > > > > > > We use around 64 bytes of shared memory to store the statistics > > > > information per slot, I'm not sure if this is a lot of memory. If this > > > > memory is fine, then I felt the approach to store stats seems fine. If > > > > that memory is too much then we could use the other approach to update > > > > stats when we stream or spill the changes as suggested by Amit. > > > > > > I agree that makes it easier to send slot stats during > > > ReplicationSlotRelease() but I'd prefer to avoid storing data that > > > doesn't need to be shared in the shared buffer if possible. > > > > > > > Sounds reasonable and we might add some stats in the future so that > > will further increase the usage of shared memory. > > > > > And those > > > counters are not used by physical slots at all. If sending slot stats > > > every time we stream or spill changes doesn't affect the system much, > > > I think it's better than having slot stats in the shared memory. > > > > > > > As the minimum size of logical_decoding_work_mem is 64KB, so in the > > worst case, we will send stats after decoding that many changes. I > > don't think it would impact too much considering that we need to spill > > or stream those many changes. If it concerns any users they can > > always increase logical_decoding_work_mem. The default value is 64MB > > at which point, I don't think it will matter sending the stats. > > Sounds good to me, I will rebase my previous patch and send a patch for this.
+1. Thanks! Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/
