On Fri, Feb 10, 2023 at 5:24 PM Hayato Kuroda (Fujitsu)
wrote:
>
Can't we have this option just as a bool (like shutdown_immediate)?
Why do we want to keep multiple modes?
--
With Regards,
Amit Kapila.
ted in the patch. Also, I noticed that for
CMD_NOTHING, the patch just ignores the action whereas the core code
does append the definition. We should check whether such a difference
is required and if so, then add comments for the same.
--
With Regards,
Amit Kapila.
On Fri, Feb 10, 2023 at 10:11 AM Amit Kapila wrote:
>
> On Fri, Feb 10, 2023 at 6:27 AM Kyotaro Horiguchi
> wrote:
> >
> > At Thu, 9 Feb 2023 13:48:52 +0530, Amit Kapila
> > wrote in
> > > On Thu, Feb 9, 2023 at 10:45 AM Kyotaro Horiguchi
> > > wr
On Fri, Feb 10, 2023 at 6:27 AM Kyotaro Horiguchi
wrote:
>
> At Thu, 9 Feb 2023 13:48:52 +0530, Amit Kapila
> wrote in
> > On Thu, Feb 9, 2023 at 10:45 AM Kyotaro Horiguchi
> > wrote:
> > >
> > > At Wed, 8 Feb 2023 09:03:03 +, "Hayato Kuroda (
On Thu, Feb 9, 2023 at 7:56 PM Robert Haas wrote:
>
> On Mon, Feb 6, 2023 at 11:40 PM Amit Kapila wrote:
> > Use appropriate wait event when sending data in the apply worker.
> >
> > Currently, we reuse WAIT_EVENT_LOGICAL_PARALLEL_APPLY_STATE_CHANGE in the
> > a
On Thu, Feb 9, 2023 at 11:21 AM Amit Kapila wrote:
>
>
> How about renaming ProcessPendingWrites to WaitToSendPendingWrites or
> WalSndWaitToSendPendingWrites?
>
How about renaming WalSndUpdateProgress() to
WalSndUpdateProgressAndSendKeepAlive() or
WalSndUpdateProgressAndKeepAliv
think that will lead to more
spurious feedback messages than we get the benefit from them.
--
With Regards,
Amit Kapila.
by
> * timeout during the delay, when the status interval is greater than
> * zero.
> */
> status_interval_ms = wal_receiver_status_interval * 1000L;
> if (status_interval_ms > 0 && diffms > status_interval_ms)
> {
> ...
>
> ~
>
> I understand in theory, your code is more efficient, but in practice,
> I think the overhead of a single variable assignment every loop
> iteration (which is doing WaitLatch anyway) is of insignificant
> concern, whereas having one assignment is simpler than having two IMO.
>
Yeah, that sounds better to me as well.
--
With Regards,
Amit Kapila.
o syncrep, but to leave the default disabled.
> >
> > I don't really see an alternative to making this depend solely on
> > sync_standbys_defined.
Fair point.
How about renaming ProcessPendingWrites to WaitToSendPendingWrites or
WalSndWaitToSendPendingWrites?
[1] -
https://www.postgresql.org/message-id/OS3PR01MB62755D216245199554DDC8DB9EEA9%40OS3PR01MB6275.jpnprd01.prod.outlook.com
--
With Regards,
Amit Kapila.
On Wed, Feb 8, 2023 at 10:57 AM Andres Freund wrote:
>
> On 2023-02-03 10:13:54 +0530, Amit Kapila wrote:
> > I am planning to push this to HEAD sometime next week (by Wednesday).
> > To backpatch this, we need to fix it in some non-standard way, like
> > without introduci
shutdown_immediate and extend
the logical replication syntax for now. We can later extend physical
replication syntax when we want to expose such an option via physical
replication.
--
With Regards,
Amit Kapila.
On Wed, Feb 8, 2023 at 1:35 AM Andres Freund wrote:
>
> On 2023-02-07 11:49:03 -0800, Andres Freund wrote:
> > On 2023-02-01 11:23:57 +0530, Amit Kapila wrote:
> > > On Tue, Jan 31, 2023 at 6:08 PM Masahiko Sawada
> > > wrote:
> > > >
> > > >
On Wed, Feb 8, 2023 at 1:19 AM Andres Freund wrote:
>
> On 2023-02-01 11:23:57 +0530, Amit Kapila wrote:
> > On Tue, Jan 31, 2023 at 6:08 PM Masahiko Sawada
> > wrote:
> > >
> > > Attached updated patches.
> > >
> >
> > Thanks, Andre
s been put forward, but it seems clear that some people
> are nervous about it.
>
> Maybe a better course would be to continue improving the toolset and get more
> people comfortable with using it locally and then talk about integrating it
> upstream.
>
Yeah, that sounds more reasonable to me as well.
--
With Regards,
Amit Kapila.
EXCLUSIVE));
This change looks odd to me. I think it would be better to pass
'already_locked' as true from the caller.
--
With Regards,
Amit Kapila.
tgresql.conf',
+ 'logical_decoding_work_mem = 64kB');
I think this setting is also not required.
--
With Regards,
Amit Kapila.
On Tue, Feb 7, 2023 at 12:41 PM Masahiko Sawada wrote:
>
> On Fri, Feb 3, 2023 at 6:44 PM Amit Kapila wrote:
>
> > We need to think of a predictable
> > way to test this path which may not be difficult. But I guess it would
> > be better to wait for some fe
On Tue, Feb 7, 2023 at 10:42 AM Peter Smith wrote:
>
> On Tue, Feb 7, 2023 at 4:02 PM Amit Kapila wrote:
> >
> > On Tue, Feb 7, 2023 at 10:07 AM Kyotaro Horiguchi
> > wrote:
> > >
> > > At Tue, 7 Feb 2023 09:10:01 +0530, Amit Kapila
> > > wr
re, if what you said would be an improvement.
> thus we can simply wait for
> min(wal_receiver_status_interval, diffms) then call send_feedback()
> unconditionally.
>
>
> -start_apply(XLogRecPtr origin_startpos)
> +start_apply(void)
>
> -LogicalRepApplyLoop(XLogRecPtr last_received)
> +LogicalRepApplyLoop(void)
>
> Does this patch requires this change?
>
I think this is because the scope of last_received has been changed so
that it can be used to pass in send_feedback() during the delay.
--
With Regards,
Amit Kapila.
On Tue, Feb 7, 2023 at 10:07 AM Kyotaro Horiguchi
wrote:
>
> At Tue, 7 Feb 2023 09:10:01 +0530, Amit Kapila
> wrote in
> > On Tue, Feb 7, 2023 at 6:03 AM Peter Smith wrote:
> > > 5b.
> > > Since there are no translator considerations here why not
ms is outside the valid range for parameter
> \"min_apply_delay\" (%d .. %d)",
> result, 0, PG_INT32_MAX))
>
I see that existing usage in the code matches what the patch had
before this comment. See below and similar usages in the code.
if (start <= 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid value for parameter \"%s\": %d",
"start", start)));
--
With Regards,
Amit Kapila.
On Tue, Feb 7, 2023 at 2:04 AM Andres Freund wrote:
>
> On 2023-02-06 12:23:54 +0530, Amit Kapila wrote:
> > On Mon, Feb 6, 2023 at 10:33 AM Andres Freund wrote:
> > > Smart shutdown is practically unusable. I don't think it makes sense to
> > > tie behavior
ser
> might
> not be able to distinguish what they are waiting for. So It seems we'd better
> to use WAIT_EVENT_MQ_SEND here which will be eaier to distinguish and
> understand. Here is a tiny patch for that.
>
Thanks for noticing this. The patch LGTM. I'll push this in some time.
--
With Regards,
Amit Kapila.
a string to represent the yes/no option.
>
I think it is better to be consistent with the existing force
parameter which is also boolean, otherwise, it will look odd.
--
With Regards,
Amit Kapila.
On Mon, Feb 6, 2023 at 12:36 PM Takamichi Osumi (Fujitsu)
wrote:
>
I have made a couple of changes in the attached: (a) changed a few
error and LOG messages; (a) added/changed comments. See, if these look
good to you then please include them in the next version.
--
With Regards,
Amit Kap
On Mon, Feb 6, 2023 at 10:33 AM Andres Freund wrote:
>
> On February 5, 2023 8:29:19 PM PST, Amit Kapila
> wrote:
> >>
> >> But that seems a too narrow view to me. Imagine you want to decomission
> >> the current primary, and instead start to use the
On Sat, Feb 4, 2023 at 6:31 PM Andres Freund wrote:
>
> On 2023-02-02 11:21:54 +0530, Amit Kapila wrote:
> > The main problem we want to solve here is to avoid shutdown failing in
> > case walreceiver/applyworker is busy waiting for some lock or for some
> > other reason
o share v23 given picking the index sounds
> complementary, not strictly required at this point.
>
I agree that it could be a separate patch. However, do you think we
need some way to disable picking the index scan? This is to avoid
cases where sequence scan could be better or do we think there won't
exist such a case?
--
With Regards,
Amit Kapila.
am not telling that teaching walsenders about shutdown modes is
completely a bad idea but it doesn't seem necessary to allow shutdowns
to complete.
--
With Regards,
Amit Kapila.
specified_opts, SUBOPT_MIN_APPLY_DELAY) &&
> sub->minapplydelay > 0)
>
> and
>
> if (opts.min_apply_delay > 0 &&
> !IsSet(opts.specified_opts, SUBOPT_STREAMING) &&
> sub->stream == LOGICALREP_STREAM_PARALLEL)
> ```
>
Won't just checking if ((opts.streaming == LOGICALREP_STREAM_PARALLEL
&& sub->minapplydelay > 0) || (opts.min_apply_delay > 0 && sub->stream
== LOGICALREP_STREAM_PARALLEL)) be sufficient in that case?
--
With Regards,
Amit Kapila.
On Fri, Feb 3, 2023 at 1:28 PM Masahiko Sawada wrote:
>
> On Fri, Feb 3, 2023 at 12:29 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Friday, February 3, 2023 11:04 AM Amit Kapila
> > wrote:
> > >
> > > On Thu, Feb 2, 2023 at 4:52 AM Peter Smith
&
On Fri, Feb 3, 2023 at 11:12 AM Peter Smith wrote:
>
> On Fri, Feb 3, 2023 at 4:21 PM Amit Kapila wrote:
> >
> > On Fri, Feb 3, 2023 at 6:41 AM Peter Smith wrote:
> > >
> > > On Thu, Feb 2, 2023 at 7:21 PM Ta
t; opts->min_apply_delay && opts->streaming == LOGICALREP_STREAM_PARALLEL)
> ereport(ERROR,
>
Yeah, we can probably write that way but in the error message we are
already using > 0, so the current style used by patch seems good to
me. Also, I think using the way you are sugge
apply worker
+# (1 day 5 minutes). Note that the extra 5 minute is to account for any
+# decoding/network overhead.
Let's try to add tests similar to what we have for
recovery_min_apply_delay unless there is some functionality in this
patch that is not there in the recovery_min_apply_delay feature.
--
With Regards,
Amit Kapila.
On Thu, Feb 2, 2023 at 4:51 PM Amit Kapila wrote:
>
> Thanks for the tests. I also see a reduction in test time variability
> with Vignesh's patch. I think we can release the locks in case the
> origin is concurrently dropped as in the attached patch. I am planning
> to commit this
On Wed, Feb 1, 2023 at 10:04 AM Amit Kapila wrote:
>
> On Tue, Jan 31, 2023 at 8:24 PM Ashutosh Bapat
> wrote:
> >
> > On Tue, Jan 31, 2023 at 5:12 PM Amit Kapila wrote:
> > >
> > > On Tue, Jan 31, 2023 at 5:03 PM Ashutosh Bapat
> > > wrote:
>
On Thu, Feb 2, 2023 at 4:52 AM Peter Smith wrote:
>
> Some minor review comments for v91-0001
>
Pushed this yesterday after addressing your comments!
--
With Regards,
Amit Kapila.
dn't have
> any problem with such a check being imposed.
>
> I regularly run pgindent locally, and if I ever commit without
> indenting, it's either intentional, or because I forgot, so the
> reminder would be useful.
>
> And as someone who runs pgindent regularly, I think this will be a net
> time saver, since I won't have to skip over other unrelated indent
> changes all the time.
>
+1.
--
With Regards,
Amit Kapila.
f the tablesync worker errors out due to some reason during the
second transaction, it can remove the slot and origin after restart by
checking the state. However, it would add another relstate which may
not be the best way to address this problem. Anyway, that can be
accomplished as a separate
On Thu, Feb 2, 2023 at 10:04 AM Kyotaro Horiguchi
wrote:
>
> At Wed, 1 Feb 2023 14:58:14 +0530, Amit Kapila
> wrote in
> > On Wed, Feb 1, 2023 at 2:09 PM Masahiko Sawada
> > wrote:
> > >
> > > Otherwise, we will end up terminating
> > > the WAL
On Thu, Feb 2, 2023 at 10:48 AM Masahiko Sawada wrote:
>
> On Wed, Feb 1, 2023 at 6:28 PM Amit Kapila wrote:
> >
> >
> > > In a case where pq_is_send_pending() doesn't become false
> > > for a long time, (e.g., the network socket buffer got full due to the
d some comments for it.
>
I think the point here is that if the apply worker is ahead of
tablesync worker then to complete the catch-up, tablesync worker needs
to apply additional transactions, and delaying during that time will
cause initial table synchronization completion to take a long ti
On Tue, Jan 31, 2023 at 9:04 AM houzj.f...@fujitsu.com
wrote:
>
> I think your comment makes sense, thanks.
> I updated the patch for the same.
>
The patch looks mostly good to me. I have made a few changes in the
comments and docs, see attached.
--
With Regards,
Amit Kapila.
v9
On Wed, Feb 1, 2023 at 2:09 PM Masahiko Sawada wrote:
>
> On Fri, Jan 20, 2023 at 7:45 PM Amit Kapila wrote:
> >
> > On Tue, Jan 17, 2023 at 2:41 PM Amit Kapila wrote:
> > >
> > > Let me try to summarize the discussion till now. The problem we are
&g
resql.org/message-id/CAA4eK1KDFeh%3DZbvSWPx%3Dir2QOXBxJbH0K8YqifDtG3xJENLR%2Bw%40mail.gmail.com
[2] -
https://www.postgresql.org/message-id/CAD21AoDKJBB6p4X-%2B057Vz44Xyc-zDFbWJ%2Bg9FL6qAF5PC2iFg%40mail.gmail.com
--
With Regards,
Amit Kapila.
On Tue, Jan 31, 2023 at 8:24 PM Ashutosh Bapat
wrote:
>
> On Tue, Jan 31, 2023 at 5:12 PM Amit Kapila wrote:
> >
> > On Tue, Jan 31, 2023 at 5:03 PM Ashutosh Bapat
> > wrote:
> > >
> > > On Tue, Jan 31, 2023 at 4:58 PM Amit Kapila
> > > wrot
s way the 'changes_count' variable name makes
> more sense IMO, otherwise (for current code) maybe it should be called
> something like 'changes_since_last_keepalive'
>
> SUGGESTION
> if (++changes_count % CHANGES_THRESHOLD == 0)
> rb->update_progress_txn(rb, txn, change->lsn);
>
I find the current code in the patch clear and easy to understand.
--
With Regards,
Amit Kapila.
On Wed, Feb 1, 2023 at 8:13 AM Kyotaro Horiguchi
wrote:
>
> At Tue, 31 Jan 2023 15:12:14 +0530, Amit Kapila
> wrote in
> > On Tue, Jan 31, 2023 at 1:40 PM Kyotaro Horiguchi
> > wrote:
> > >
> > > Hi, Kuroda-san, Thanks for the detailed study.
> &g
On Tue, Jan 31, 2023 at 5:03 PM Ashutosh Bapat
wrote:
>
> On Tue, Jan 31, 2023 at 4:58 PM Amit Kapila wrote:
>
> > Thanks, the patch looks good to me. I have slightly adjusted one of
> > the comments and ran pgindent. See attached. As mentioned in the
> > commit mess
d one of
the comments and ran pgindent. See attached. As mentioned in the
commit message, we shouldn't backpatch this as this requires a new
callback and moreover, users can increase the wal_sender_timeout and
wal_receiver_timeout to avoid this problem. What do you think?
--
With Regards,
eed to
probably change the return value of defGetMinApplyDelay() to int32.
--
With Regards,
Amit Kapila.
ating the value of
replication_slot_xmin.
+*/
+
--
With Regards,
Amit Kapila.
https://www.postgresql.org/message-id/CAA4eK1Kt67RdW0WTR-LTxasj3pyukPCYhfA0arDUNnsz2wh03A%40mail.gmail.com
--
With Regards,
Amit Kapila.
trade-offs for the
> above, and I'd be open to work on that if there is a clear winner. For me (3)
> is a decent solution for the problem.
>
>From the discussion above it is not very clear that adding maintenance
costs in this area is worth it even though that can give better
results as far as this feature is concerned.
--
With Regards,
Amit Kapila.
a good idea to remove 'already_locked' parameter,
especially in back branches as this is an exposed API.
--
With Regards,
Amit Kapila.
On Mon, Jan 30, 2023 at 11:34 AM Amit Kapila wrote:
>
> I have reproduced it manually. For this, I had to manually make the
> debugger call ReplicationSlotsComputeRequiredXmin(false) via path
> SnapBuildProcessRunningXacts()->LogicalIncreaseXminForSlot()->LogicalConfi
On Thu, Dec 8, 2022 at 8:17 AM Masahiko Sawada wrote:
>
> On Mon, Nov 21, 2022 at 4:31 PM Amit Kapila wrote:
>
> One idea to fix this issue is that in
> ReplicationSlotsComputeRequiredXmin(), we compute the minimum xmin
> while holding both ProcArrayLock and Replicat
On Mon, Jan 30, 2023 at 12:38 PM Kyotaro Horiguchi
wrote:
>
> At Mon, 30 Jan 2023 11:56:33 +0530, Amit Kapila
> wrote in
> > >
> > > The GUC is not stored in a catalog, but.. oh... it is multiplied by
> > > 1000.
> >
> > Which part of the
hat is not processed by the plugin. So, in
such cases, the downstream can timeout. To avoid that we try to send a
keepalive message if required. Trying to send a keepalive message
after every change has some overhead, but testing showed there is no
noticeable overhead if we do it after every ~100 changes."
--
With Regards,
Amit Kapila.
On Mon, Jan 30, 2023 at 9:20 AM vignesh C wrote:
>
> On Sat, 28 Jan 2023 at 11:26, Amit Kapila wrote:
> >
> > One thing that looks a bit odd is that we will anyway have a similar
> > check in replorigin_drop_guts() which is a static function and called
>
On Mon, Jan 30, 2023 at 9:43 AM Kyotaro Horiguchi
wrote:
>
> At Mon, 30 Jan 2023 08:51:05 +0530, Amit Kapila
> wrote in
> > On Mon, Jan 30, 2023 at 8:32 AM Kyotaro Horiguchi
> > wrote:
> > >
> > > At Sat, 28 Jan 2023 04:28:29 +, &q
On Mon, Jan 30, 2023 at 5:40 AM Peter Smith wrote:
>
> Patch v88-0001 LGTM.
>
Pushed.
--
With Regards,
Amit Kapila.
On Mon, Jan 30, 2023 at 10:27 AM Amit Kapila wrote:
>
> On Sun, Jan 29, 2023 at 9:15 PM Masahiko Sawada wrote:
> >
> > On Sat, Jan 28, 2023 at 11:54 PM Hayato Kuroda (Fujitsu)
> > wrote:
> > >
> > > Dear Amit, Sawada-san,
> > >
> > >
ication slot with
> > NOEXPORT_SNAPSHOT option. I think in this case, CreateInitDecodingContext()
> > is
> > called with need_full_snapshot = false, and slot->effective_xmin is not
> > updated.
>
> Right. This is how we create a slot used by an apply worker.
>
I was thinking about how that led to this problem because
GetOldestSafeDecodingTransactionId() ignores InvalidTransactionId. It
seems that is possible when both builder->xmin and
replication_slot_catalog_xmin precede replication_slot_catalog_xmin.
Do you see any different reason for it?
--
With Regards,
Amit Kapila.
On Mon, Jan 30, 2023 at 8:32 AM Kyotaro Horiguchi
wrote:
>
> At Sat, 28 Jan 2023 04:28:29 +, "Takamichi Osumi (Fujitsu)"
> wrote in
> > On Friday, January 27, 2023 8:00 PM Amit Kapila
> > wrote:
> > > So, you have changed min_apply_de
On Sat, Jan 28, 2023 at 9:36 AM houzj.f...@fujitsu.com
wrote:
>
> On Friday, January 27, 2023 8:16 PM Amit Kapila
> >
> > On Fri, Jan 27, 2023 at 3:45 PM vignesh C wrote:
> > >
> > > On Mon, 23 Jan 2023 at 10:52, Amit Kapila
> > wrote:
> > >
mp();
+ if (wakeup[LRW_WAKEUP_SYNC_START] < now)
+ wakeup[LRW_WAKEUP_SYNC_START] =
TimestampTzPlusSeconds(wakeup[LRW_WAKEUP_SYNC_START], 1);
Do we see unnecessary wakeups without this, or delay in sync?
BTW, do we need to do something about wakeups in
wait_for_relation_state_change()?
--
With Regards,
Amit Kapila.
On Fri, Jan 27, 2023 at 3:45 PM vignesh C wrote:
>
> On Mon, 23 Jan 2023 at 10:52, Amit Kapila wrote:
> >
> > IIRC, this is done to prevent concurrent drops of origin drop say by
> > exposed API pg_replication_origin_drop(). See the discussion in [1]
> > related to
On Fri, Jan 27, 2023 at 5:18 PM houzj.f...@fujitsu.com
wrote:
>
> On Wednesday, January 25, 2023 7:26 PM Amit Kapila
> >
> > On Tue, Jan 24, 2023 at 8:15 AM wangw.f...@fujitsu.com
> > wrote:
> > >
> > > Attach the new patch.
> > >
> &g
imestampDifferenceMilliseconds(GetCurrentTimestamp(),
+ TimestampTzPlusMilliseconds(finish_ts, MySubscription->minapplydelay));
The above code appears a bit unreadable. Can we store the result of
TimestampTzPlusMilliseconds() in a separate variable say "TimestampTz
delayUntil;"?
--
With Regards,
Amit Kapila.
is processing
running_xacts via SnapBuildProcessRunningXacts()? Isn't that walsender
slot's effective_xmin have a non-zero value? If not, then why?
--
With Regards,
Amit Kapila.
comments for the callback.
Additionally, I think we should have a test case to show we don't time
out because of not processing non-transactional messages. See
pgoutput_message for cases where it doesn't process the message.
--
With Regards,
Amit Kapila.
v7-0001-Fix-the-logical-replication-timeo
servers."
[1] - https://www.postgresql.org/docs/devel/runtime-config-replication.html
--
With Regards,
Amit Kapila.
of
> > send_feedback to "has_unprocessed_change".
> >
> > At Tue, 24 Jan 2023 12:27:58 +0530, Amit Kapila
> > wrote in
> > > > send_feedback():
> > > > +* If the subscriber side apply is delayed (because of
> > time-delayed
> &
On Wed, Jan 25, 2023 at 10:05 AM Amit Kapila wrote:
>
> On Wed, Jan 25, 2023 at 3:15 AM Peter Smith wrote:
> >
> > 1.
> > @@ -210,7 +210,7 @@ int logical_decoding_work_mem;
> > static const Size max_changes_in_memory = 4096; /* XXX for restore only */
> &
nclude/replication/reorderbuffer.h
>
> 3.
> @@ -18,14 +18,14 @@
> #include "utils/timestamp.h"
>
> extern PGDLLIMPORT int logical_decoding_work_mem;
> -extern PGDLLIMPORT int logical_decoding_mode;
> +extern PGDLLIMPORT int logical_replication_mode;
>
> Probably here should also be a comment to say "/* GUC variables */"
>
Okay, we can do this.
--
With Regards,
Amit Kapila.
hat invalidation has been
processed; (c) print rel states and relids from table_states_not_ready
in process_syncing_tables_for_apply() to see if t2 has ever appeared
in that list.
--
With Regards,
Amit Kapila.
On Tue, Jan 24, 2023 at 12:44 PM Peter Smith wrote:
>
> On Tue, Jan 24, 2023 at 5:58 PM Amit Kapila wrote:
> >
> > On Tue, Jan 24, 2023 at 8:15 AM Kyotaro Horiguchi
> > wrote:
> > >
> > > > Attached the updated patch v19.
> > >
> > &g
d.
>
I think this is true without this patch, so why that has not been
followed in the first place? One comment, I see in this regard is as
below:
/* It's legal to not pass a recvpos */
if (recvpos < last_recvpos)
recvpos = last_recvpos;
--
With Regards,
Amit Kapila.
to be in reverse order as it
doesn't make much sense to me to first say that we are skipping
changes and then say the transaction is delayed. What do you think?
--
With Regards,
Amit Kapila.
On Tue, Jan 24, 2023 at 6:17 AM Kyotaro Horiguchi
wrote:
>
> At Mon, 23 Jan 2023 17:36:13 +0530, Amit Kapila
> wrote in
> > On Sun, Jan 22, 2023 at 6:12 PM Takamichi Osumi (Fujitsu)
> > wrote:
> > >
> > >
> > > Attached the updated pat
make it move forward.
[1] -
https://www.postgresql.org/message-id/TYAPR01MB586668E50FC2447AD7F92491F5E89%40TYAPR01MB5866.jpnprd01.prod.outlook.com
--
With Regards,
Amit Kapila.
On Tue, Jan 24, 2023 at 3:46 AM Peter Smith wrote:
>
> On Mon, Jan 23, 2023 at 9:44 PM Amit Kapila wrote:
> >
> >
> > > 6. defGetMinApplyDelay
> > >
> ...
> > >
> > > 6b.
> > > I thought this function should be implemented as st
and subscriber but not really sure
whether that will simplify the usage.
[1] -
https://www.postgresql.org/message-id/CAJpy0uDzddK_ZUsB2qBJUbW_ZODYGoUHTaS5pVcYE2tzATCVXQ%40mail.gmail.com
--
With Regards,
Amit Kapila.
eature? Currently,
it doesn't care whether the changes of a particular xact are skipped
or not. I think that might be okay because anyway the purpose of this
feature is to make subscriber lag from publishers. What do you think?
I feel we can add some comments to indicate the same.
--
With Regards,
Amit Kapila.
est?
>
> Should there be an additional test to check redundant parameter
> setting -- eg. "... WITH (min_apply_delay=123, min_apply_delay=456)"
>
I don't think that will be of much help. We don't seem to have other
tests for subscription parameters.
--
With Regards,
Amit Kapila.
On Thu, Jan 19, 2023 at 11:24 PM Zheng Li wrote:
>
> On Thu, Jan 19, 2023 at 2:05 AM Amit Kapila wrote:
> >
> > >
> > > Foreign Tables can also be considered replicated with DDL replication
> > > because we
> > > don't even need to replicate the
rocess_syncing_tables_for_sync() before the start of the second
transaction which leads to this change. See the report and discussion
about that race condition in the email [1].
[1] -
https://www.postgresql.org/message-id/CAD21AoAw0Oofi4kiDpJBOwpYyBBBkJj=sluon4gd2gjuakg...@mail.gmail.com
--
With Regards,
Amit Kapila.
ops of origin drop say by
exposed API pg_replication_origin_drop(). See the discussion in [1]
related to it. If we want we can optimize it so that we can acquire
the lock on the specific origin as mentioned in comments
replorigin_drop_by_name() but it was not clear that this operation
would be freq
ltiple times (it is already checked inside
OutputPluginUpdateProgress). So, I would prefer a neat code here.
--
With Regards,
Amit Kapila.
on
> > subscriber and standby nodes.
>
> Using the existing parameter makes sense to me. But if we use
> logical_decoding_mode also on the subscriber, as Shveta Malik also
> suggested, probably it's better to rename it so as not to confuse. For
> example, logical_replication_mo
On Tue, Jan 17, 2023 at 2:41 PM Amit Kapila wrote:
>
> Let me try to summarize the discussion till now. The problem we are
> trying to solve here is to allow a shutdown to complete when walsender
> is not able to send the entire WAL. Currently, in such cases, the
> shutdown fa
On Fri, Jan 20, 2023 at 8:03 AM wangw.f...@fujitsu.com
wrote:
>
> On Thurs, Jan 19, 2023 at 19:18 PM Amit Kapila
> wrote:
> > On Wed, Jan 11, 2023 at 4:20 PM wangw.f...@fujitsu.com
> > wrote:
> > >
> > > When I was reading the "Logical Decod
e.g.
> BEFORE
> ctx->update_progress(ctx, ctx->write_location, ctx->write_xid, false);
> AFTER
> OutputPluginUpdateProgress(ctx, false);
>
We already check whether ctx->update_progress is defined or not which
is the only extra job done by OutputPluginUpdateProgress but probably
we can consolidate the checks and directly invoke
OutputPluginUpdateProgress.
--
With Regards,
Amit Kapila.
can specify the reason for not
allowing this combination in the comments.
--
With Regards,
Amit Kapila.
f you agree this can be 0002
> patch.
>
Do we have any similar stats for recovery_min_apply_delay? If not, I
suggest let's postpone this to see if users really need such a
parameter.
--
With Regards,
Amit Kapila.
On Thu, Jan 19, 2023 at 4:13 PM Ashutosh Bapat
wrote:
>
> On Wed, Jan 18, 2023 at 6:00 PM Amit Kapila wrote:
>
> > + */
> > + ReorderBufferUpdateProgressCB update_progress;
> >
> > Are you suggesting changing the name of the above variable? If so, how
&
On Thu, Jan 19, 2023 at 4:47 PM Amit Kapila wrote:
>
> On Wed, Jan 11, 2023 at 4:20 PM wangw.f...@fujitsu.com
> wrote:
> >
> > When I was reading the "Logical Decoding Output Plugins" chapter in pg-doc
> > [1],
> > I think in the su
e these to be more reader friendly. So I tried to write
> a patch for these and attach it.
>
> Any thoughts?
>
This looks mostly good to me. I have made minor adjustments in the
attached. Do those make sense to you?
--
With Regards,
Amit Kapila.
v2-0001-Improve-the-description-of-Output-Plugin-Callback.patch
Description: Binary data
On Thu, Jan 19, 2023 at 8:39 AM Zheng Li wrote:
>
> On Wed, Jan 18, 2023 at 6:27 AM Amit Kapila wrote:
> >
> > On Sat, Jan 7, 2023 at 8:58 PM Zheng Li wrote:
> > >
>
> Foreign Tables can also be considered replicated with DDL replication because
> we
>
1301 - 1400 of 6017 matches
Mail list logo
