On Mon, Dec 30, 2019 at 3:43 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Sun, Dec 29, 2019 at 1:34 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > I have observed some more issues > > > > 1. Currently, In ReorderBufferCommit, it is always expected that > > whenever we get REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM, we must > > have already got REORDER_BUFFER_CHANGE_INTERNAL_SPEC_INSERT and in > > SPEC_CONFIRM we send the tuple we got in SPECT_INSERT. But, now those > > two messages can be in different streams. So we need to find a way to > > handle this. Maybe once we get SPEC_INSERT then we can remember the > > tuple and then if we get the SPECT_CONFIRM in the next stream we can > > send that tuple? > > > > Your suggestion makes sense to me. So, we can try it. Sure. > > > 2. During commit time in DecodeCommit we check whether we need to skip > > the changes of the transaction or not by calling > > SnapBuildXactNeedsSkip but since now we support streaming so it's > > possible that before we decode the commit WAL, we might have already > > sent the changes to the output plugin even though we could have > > skipped those changes. So my question is instead of checking at the > > commit time can't we check before adding to ReorderBuffer itself > > > > I think if we can do that then the same will be true for current code > irrespective of this patch. I think it is possible that we can't take > that decision while decoding because we haven't assembled a consistent > snapshot yet. I think we might be able to do that while we try to > stream the changes. I think we need to take care of all the > conditions during streaming (when the logical_decoding_workmem limit > is reached) as we do in DecodeCommit. This needs a bit more study. I agree.
-- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com