On Monday, January 23, 2023 7:45 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > On Mon, Jan 23, 2023 at 1:36 PM Peter Smith <smithpb2...@gmail.com> > wrote: > > > > Here are my review comments for v19-0001. > > > ... > > > > 5. parse_subscription_options > > > > + /* > > + * The combination of parallel streaming mode and min_apply_delay is > > + not > > + * allowed. The subscriber in the parallel streaming mode applies > > + each > > + * stream on arrival without the time of commit/prepare. So, the > > + * subscriber needs to depend on the arrival time of the stream in > > + this > > + * case, if we apply the time-delayed feature for such transactions. > > + Then > > + * there is a possibility where some unnecessary delay will be added > > + on > > + * the subscriber by network communication break between nodes or > > + other > > + * heavy work load on the publisher. On the other hand, applying the > > + delay > > + * at the end of transaction with parallel apply also can cause > > + issues of > > + * used resource bloat and locks kept in open for a long time. Thus, > > + those > > + * features can't work together. > > + */ > > > > IMO some re-wording might be warranted here. I am not sure quite how > > to do it. Perhaps like below? > > > > SUGGESTION > > > > The combination of parallel streaming mode and min_apply_delay is not > allowed. > > > > Here are some reasons why these features are incompatible: > > a. In the parallel streaming mode the subscriber applies each stream > > on arrival without knowledge of the commit/prepare time. This means we > > cannot calculate the underlying network/decoding lag between publisher > > and subscriber, and so always waiting for the full 'min_apply_delay' > > period might include unnecessary delay. > > b. If we apply the delay at the end of the transaction of the parallel > > apply then that would cause issues related to resource bloat and locks > > being held for a long time. > > > > ~~~ > > > > How about something like: > The combination of parallel streaming mode and min_apply_delay is not > allowed. This is because we start applying the transaction stream as soon as > the first change arrives without knowing the transaction's prepare/commit > time. > This means we cannot calculate the underlying network/decoding lag between > publisher and subscriber, and so always waiting for the full 'min_apply_delay' > period might include unnecessary delay. > > The other possibility is to apply the delay at the end of the parallel apply > transaction but that would cause issues related to resource bloat and locks > being held for a long time. Thank you for providing a good description ! Adopted. The latest patch can be seen in [1].
[1] - https://www.postgresql.org/message-id/TYCPR01MB8373DC1881F382B4703F26E0EDC99%40TYCPR01MB8373.jpnprd01.prod.outlook.com Best Regards, Takamichi Osumi
