ered/This check callback ensures the value is
> not overridden by the user/
>
These comments appear mostly repetitive to what is already mentioned
in start_postmaster(). So, I have changed those referred to already
written comments, and slightly adjusted the comments at another place.
See attached. Personally, I don't see the need for a test for this, so
removed the same but can add it back if you or others think so.
--
With Regards,
Amit Kapila.
inhibit_m_s_w_k_s_during_upgrade_6.patch
Description: Binary data
he same.
>
Pushed!
--
With Regards,
Amit Kapila.
[1]. I can take care of this unless we see
some opposition to this idea.
[1] -
https://www.postgresql.org/message-id/20231102.115834.1012152975995247837.horikyota.ntt%40gmail.com
--
With Regards,
Amit Kapila.
On Thu, Nov 9, 2023 at 11:40 AM Kyotaro Horiguchi
wrote:
>
> At Thu, 9 Nov 2023 09:53:07 +0530, Amit Kapila
> wrote in
> > Michael, Horiguchi-San, and others, do you have any thoughts on what
> > is the best way to proceed?
>
> As I previously mentioned, I believe tha
ntal changes be synchronized
till all the new tables are created and synced before step 7?
--
With Regards,
Amit Kapila.
On Tue, Nov 7, 2023 at 4:16 PM Amit Kapila wrote:
>
> On Tue, Nov 7, 2023 at 8:12 AM Michael Paquier wrote:
> >
> > On Tue, Nov 07, 2023 at 07:59:46AM +0530, Amit Kapila wrote:
> > > Do you mean to say that if 'IsBinaryUpgrade' is true then let's n
On Thu, Nov 9, 2023 at 8:11 AM Amit Kapila wrote:
>
> On Wed, Nov 8, 2023 at 8:09 PM Drouvot, Bertrand
> wrote:
> >
> > > Unrelated to above, if there is a user slot on standby with the same
> > > name which the slot-sync worker is trying to create, then shall
he slot sync
mechanism would be stopped. Do you have reasons to prefer giving a
WARNING and skipping creating such slots? I expect this WARNING to
keep getting repeated in LOGs because the consecutive sync tries will
again generate a WARNING.
--
With Regards,
Amit Kapila.
On Wed, Nov 8, 2023 at 12:32 PM Drouvot, Bertrand
wrote:
>
> Hi,
>
> On 11/8/23 4:50 AM, Amit Kapila wrote:
>
> > I think if we want to follow
> > this approach then we need to also monitor these slots for any change
> > in the consecutive cycles and
the whole of
> dbinfos by using pg_malloc0 instead of pg_malloc which will ensure
> that the slot information is set to 0.
>
I would prefer this fix instead of initializing the slot array at
multiple places. I'll push this tomorrow unless someone thinks
otherwise.
--
With Regards,
Amit Kapila.
00839a in check_new_cluster () at check.c:215
> #9 0x55563010 in main (argc=13, argv=0x7fffdf08) at
> pg_upgrade.c:136
>
> This issue occurs because we are accessing uninitialized slot array
> information.
>
Thanks for the report. I'll review your proposed fix.
--
With Regards,
Amit Kapila.
On Tue, Nov 7, 2023 at 7:58 PM Drouvot, Bertrand
wrote:
>
> On 11/7/23 11:55 AM, Amit Kapila wrote:
> >>>
> >>> This is not full proof solution but optimization over first one. Now
> >>> in any sync-cycle, we take 2 attempts for slots-creation (if any sl
hat it shows up in pg_replication_slots before this message is emitted
> (and that
> specially true/worst for non active slots).
>
> Maybe something like "newly locally created slot XXX has been synced..."?
>
> While at it, would that make sense to move
>
> + slot->data.failover = true;
>
> once we stop waiting for this slot? I think that would avoid confusion if one
> query pg_replication_slots while we are still waiting for this slot to be
> synced,
> thoughts? (currently we can see pg_replication_slots.synced_slot set to true
> while we are still waiting).
>
The failover property of the slot is different from whether the slot
has been synced yet, so we can't change the location of marking it but
we can try to improve when to show that slot has been synced.
--
With Regards,
Amit Kapila.
On Tue, Nov 7, 2023 at 8:12 AM Michael Paquier wrote:
>
> On Tue, Nov 07, 2023 at 07:59:46AM +0530, Amit Kapila wrote:
> > Do you mean to say that if 'IsBinaryUpgrade' is true then let's not
> > allow to launch launcher or apply worker? If so, I guess this won
t directory:
> ./build/testrun/pg_upgrade/003_logical_slots/data/delete_old_cluster.sh
>
Thanks for the patch and verification. Pushed the fix.
--
With Regards,
Amit Kapila.
On Sun, Nov 5, 2023 at 5:33 AM Michael Paquier wrote:
>
> On Fri, Nov 03, 2023 at 01:33:26PM +1100, Peter Smith wrote:
> > On Fri, Nov 3, 2023 at 1:11 PM Amit Kapila wrote:
> >> Now, that Michael also committed another similar change in commit
> >> 7021d3b176, it
On Tue, Nov 7, 2023 at 3:56 AM David Rowley wrote:
>
> On Thu, 2 Nov 2023 at 22:42, Amit Kapila wrote:
> > The other two look good to me.
>
> Thanks for looking.
>
> I spent some time trying to see if the performance changes much with
> either of these cases. For the X
On Mon, Nov 6, 2023 at 1:57 PM Amit Kapila wrote:
>
> On Mon, Nov 6, 2023 at 7:01 AM Zhijie Hou (Fujitsu)
> wrote:
> >
+static void
+WalSndGetStandbySlots(List **standby_slots, bool force)
+{
+ if (!MyReplicationSlot->data.failover)
+ return;
+
+ if (standby_slot_names_lis
On Mon, Nov 6, 2023 at 7:01 AM Zhijie Hou (Fujitsu)
wrote:
>
> On Friday, November 3, 2023 7:32 PM Amit Kapila
> >
> > 5.
> > @@ -228,6 +230,28 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo
> > fcinfo, bool confirm, bool bin
> > Na
5 IST [22332] DETAIL: Streaming transactions
committing after 0/1522730, reading WAL from 0/15226F8.
2023-11-06 08:41:58.015 IST [22332] STATEMENT: START_REPLICATION SLOT
"sub1" LOGICAL 0/0 (proto_version '4', origin 'any', publication_names
'"pub1"')
We can get the PID from the log line as for other logs and I don't see
the process name printed anywhere else.
--
With Regards,
Amit Kapila.
hat will make the code look neat and also
avoid allocating this list when failover is not enabled for the slot.
6.
+/* ALTER_REPLICATION_SLOT slot */
+alter_replication_slot:
+ K_ALTER_REPLICATION_SLOT IDENT '(' generic_option_list ')'
I think you need to update the docs for this new command. See existing docs [1].
[1] - https://www.postgresql.org/docs/devel/protocol-replication.html
--
With Regards,
Amit Kapila.
On Thu, Nov 2, 2023 at 2:36 PM Amit Kapila wrote:
>
> On Thu, Nov 2, 2023 at 11:32 AM Michael Paquier wrote:
> >
> > On Thu, Nov 02, 2023 at 02:32:07PM +1100, Peter Smith wrote:
> > > On Thu, Nov 2, 2023 at 2:25 PM Peter Smith wrote:
> > >> Checking this
atch was that here unlike
the publication's slot information, we can't ensure with origin's
remote_lsn that all the WAL is received and applied before allowing
the upgrade. I can't think of any problem at the moment due to this
but still a point worth giving a thought.
--
With Regards,
Amit Kapila.
On Tue, Oct 31, 2023 at 2:25 AM David Rowley wrote:
>
> On Mon, 30 Oct 2023 at 23:48, Amit Kapila wrote:
> >
> > On Fri, Oct 27, 2023 at 3:23 AM David Rowley wrote:
> > > * parallel.c in HandleParallelMessages():
> > > * applyparallelworker.c in HandleParal
TOH, I can't imagine
all possible scenarios. As this setting is invalid or can cause
problems, it seems people favor preventing it. Alvaro also voted in
favor of preventing it, so we are considering to proceed with it
unless more people think otherwise.
--
With Regards,
Amit Kapila.
On Wed, Nov 1, 2023 at 8:33 AM Michael Paquier wrote:
>
> On Fri, Oct 27, 2023 at 05:05:39PM +0530, Amit Kapila wrote:
> > I was analyzing this part and it seems it could be tricky to upgrade
> > in FINISHEDCOPY state. Because the system would expect that subscriber
>
On Wed, Nov 1, 2023 at 12:54 PM Michael Paquier wrote:
>
> On Mon, Oct 30, 2023 at 03:54:39PM +0530, Amit Kapila wrote:
> > On Sat, Oct 28, 2023 at 4:30 PM Michael Paquier wrote:
> >> On Sat, Oct 28, 2023 at 03:45:13PM +0530, Amit Kapila wrote:
> >> > Yes, we nee
ents whether we need to LOG it or not. I guess at some place like
atop LogReplicationSlotAcquire() we should document in a bit more
specific way as to when is this expected to be called.
--
With Regards,
Amit Kapila.
1/postgres/tmp_install/bin/pg_resetwal"
> failed: cannot execute
>
> Failure, exiting
> [16:24:21.144](6.275s) not ok 10 - run of pg_upgrade of old cluster
>
> If the same command is run manually, it succeeds -
>
Can you add some LOGs in pg_resetwal to find out if the command has
performed appropriately?
--
With Regards,
Amit Kapila.
we just remove ERROR in InvalidatePossiblyObsoleteSlot or
> make it an Assert and say do not override max_slot_wal_keep_size in
> docs? Even if someone did override, let the pg_upgrade report the slot
> as invalidated and let the user delete the slot or decide what to do
> with it.
>
The problem is this can happen in the background so it can happen at
the time of shutdown when all the upgrade is complete.
--
With Regards,
Amit Kapila.
hange in 'failover' option; run the
alter_replication_slot command; this needs more analysis as
apply_worker is already doing streaming and changing slot property in
between could be tricky.
--
With Regards,
Amit Kapila.
t; to change the failover flag of the slot,
> > while plugins that do not support this will not set this and the
> > failover flag of the created slot will remain.
> > What do you think?
>
> May be OK, but I came up with a corner case that external plugins have a
> streaming
> option 'failover'. What should be? Has the option been reserved?
>
Sorry, your question is not clear to me. Did you intend to say that
the value of the existing streaming option could be 'failover'?
--
With Regards,
Amit Kapila.
On Mon, Oct 30, 2023 at 7:13 PM Robert Haas wrote:
>
> On Sat, Oct 28, 2023 at 6:15 AM Amit Kapila wrote:
> > > Hmm. So my question is: do we need the cleanup lock on the write
> > > buffer even if there are no tuples, and even if primary bucket and the
> &g
On Mon, Oct 30, 2023 at 2:17 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, October 30, 2023 12:20 PM Amit Kapila
> wrote:
> >
> > On Thu, Oct 26, 2023 at 2:01 PM Zhijie Hou (Fujitsu)
> >
> > wrote:
> > >
> > > On Thursday, October 26, 2023
ROR/NOTICE messages from
parallel workers as you have also noticed. The comment atop
initReadOnlyStringInfo() clearly states that it is used in the
performance-critical path. So, is it worth changing these places? In
the future, this may pose the risk of this API being used
inconsistently.
--
With Regards,
Amit Kapila.
On Sat, Oct 28, 2023 at 4:30 PM Michael Paquier wrote:
>
> On Sat, Oct 28, 2023 at 03:45:13PM +0530, Amit Kapila wrote:
> > Yes, we need it to exclude any concurrent in-progress scans that could
> > return incorrect tuples during bucket squeeze operation.
>
> Thanks. So
d your patch.
--
With Regards,
Amit Kapila.
On Thu, Oct 26, 2023 at 2:01 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Thursday, October 26, 2023 12:42 PM Amit Kapila
> wrote:
> >
> > On Tue, Oct 24, 2023 at 5:27 PM Zhijie Hou (Fujitsu)
> >
> > wrote:
> > >
> > > While re
On Mon, Oct 30, 2023 at 7:58 AM Kyotaro Horiguchi
wrote:
>
> At Fri, 27 Oct 2023 14:57:10 +0530, Amit Kapila
> wrote in
> > On Fri, Oct 27, 2023 at 2:02 PM Alvaro Herrera
> > wrote:
> > >
> > > On 2023-Oct-27, Kyotaro Horiguchi
t; Hmm. So my question is: do we need the cleanup lock on the write
> buffer even if there are no tuples, and even if primary bucket and the
> write bucket are the same?
>
Yes, we need it to exclude any concurrent in-progress scans that could
return incorrect tuples during bucket squeeze operation.
--
With Regards,
Amit Kapila.
On Fri, Oct 27, 2023 at 9:00 PM Drouvot, Bertrand
wrote:
>
> On 10/27/23 10:35 AM, Amit Kapila wrote:
> > On Wed, Oct 25, 2023 at 3:15 PM Drouvot, Bertrand
> >
> > I think even if we provide such an API, we need to have logic to get
> > the slots from the primary
x27;t we at least ensure that replication origins do exist in the
old cluster corresponding to each of the subscriptions? Otherwise,
later the query to get remote_lsn for origin in getSubscriptions()
would fail.
--
With Regards,
Amit Kapila.
n simply (c) document the usage of max_slot_wal_keep_size in the
upgrade. I am not sure whether it is worth complicating the code for
this as the user shouldn't be using such an option during the upgrade.
So, I think doing (a) and (c) could be simpler.
>
> In InvalidatePossiblyObsoleteSlot() we could have
> just an Assert() or elog(PANIC).
>
Yeah, we can change to either of those.
--
With Regards,
Amit Kapila.
that the API call is "hanging" until there is some activity on the
> newly created slot
> (currently there is "waiting for remote slot " message in the logfile as
> mentioned above but
> I'm not sure that's enough)
>
I think even if we provide such an API, we need to have logic to get
the slots from the primary and create them. Say, even if the user used
the APIs, there may still be some new slots that the sync worker needs
to create. I think it might be better to provide a view for users to
view the current state of sync. For example, in the above case, we can
say "waiting for the primary to advance remote LSN" or something like
that.
--
With Regards,
Amit Kapila.
; ```
> > -# Setup a pg_upgrade command. This will be used anywhere.
> > +# Setup a common pg_upgrade command to be used by all the test cases
> > ```
>
> The patch LGTM.
>
Thanks, I'll push it in some time.
--
With Regards,
Amit Kapila.
;invalid_logical_slots.txt");
>
> Or you could do something even shorter, with "invalid_slots.txt".
>
I also thought of it but if we want to keep it that way, we should
slightly adjust the messages like: "The slot \"%s\" is invalid" to
include slot_type. This will contain only logical slots, so the
current one probably seems okay.
--
With Regards,
Amit Kapila.
:
>
> errmsg("\"max_slot_wal_keep_size\" must be set to -1 during the upgrade"),
> errhint("Do not override \"max_slot_wal_keep_size\" using command line
> options."));
>
But OTOH, we don't have a value of user-passed options to ensure that.
So, how about a slightly different message: "This can be caused by
overriding \"max_slot_wal_keep_size\" using command line options." or
something along those lines? I see a somewhat similar message in the
existing code (errhint("This can be caused ...")).
--
With Regards,
Amit Kapila.
so how about:
"replication slot was invalidated when in binary upgrade mode"?
--
With Regards,
Amit Kapila.
st file lives under "pg_upgrade/t" so I felt that
> upgrading is already implied.
>
Agreed. So, how about 003_upgrade_logical_slots.pl or simply
003_upgrade_slots.pl?
> 2. If the node names will be shortened they should still retain *some*
> meaning if possible:
> old_publisher/subscriber/new_publisher --> node1/node2/node3 (means
> nothing without studying the tests)
> old_publisher/subscriber/new_publisher --> alpha/bravo/charlie (means
> nothing without studying the tests)
> How about:
> old_publisher/subscriber/new_publisher --> node_p1/node_s/node_p2
> or similar...
>
Why not simply oldpub/sub/newpub or old_pub/sub/new_pub?
--
With Regards,
Amit Kapila.
; behave similar to the '2PC'
option as per discussion [1].
[1] -
https://www.postgresql.org/message-id/b099ebc2-68fd-4c08-87ce-65fc4cb24121%40gmail.com
--
With Regards,
Amit Kapila.
On Thu, Oct 26, 2023 at 5:38 PM Drouvot, Bertrand
wrote:
>
> On 10/26/23 10:40 AM, Amit Kapila wrote:
> > On Wed, Oct 25, 2023 at 8:49 PM Drouvot, Bertrand
> > wrote:
> >>
> >
> > Good point, I think we should enhance the WalSndWait() logic to
> > a
o break the loop or shutdown the walsender.
>
> Thoughts?
>
Good point, I think we should enhance the WalSndWait() logic to
address this case. Additionally, I think we should ensure that
WalSndWaitForWal() shouldn't wait twice once for wal_flush and a
second time for wal to be replayed by physical standby. It should be
okay to just wait for Wal to be replayed by physical standby when
applicable, otherwise, just wait for Wal to flush as we are doing now.
Does that make sense?
--
With Regards,
Amit Kapila.
x27;t tested it yet. BTW, can
we change the comment: "Output stream start if we haven't yet, but
only for the transactional case." to "Output stream start if we
haven't yet for transactional messages"?
I think we should backpatch this fix. What do you think?
--
With Regards,
Amit Kapila.
On Fri, Oct 20, 2023 at 9:40 AM Dilip Kumar wrote:
>
> On Sat, Oct 14, 2023 at 9:43 AM Amit Kapila wrote:
> >
> > This and other results shared by you look promising. Will there be any
> > improvement in workloads related to clog buffer usage?
>
> I did not un
t;
> I'd find that a useful option.
>
+1.
--
With Regards,
Amit Kapila.
On Wed, Oct 25, 2023 at 1:39 PM Bharath Rupireddy
wrote:
>
> On Wed, Oct 25, 2023 at 11:39 AM Amit Kapila wrote:
> >
> > On Tue, Oct 24, 2023 at 1:20 PM Bharath Rupireddy
> > wrote:
> > >
> > >
> > > I spent some time on the v57 patch an
s in the docs to adjust
the order of various prerequisites.
--
With Regards,
Amit Kapila.
v58-0001-pg_upgrade-Allow-to-replicate-logical-replicatio.patch
Description: Binary data
[1] which is to enable failover even for the main slot after
all tables are in ready state, something similar to what we do for
two_phase.
[1] -
https://www.postgresql.org/message-id/CAA4eK1J6BqO5%3DueFAQO%2BaYyHLaU-oCHrrVFJqHS-i0Ce9aPY2w%40mail.gmail.com
--
With Regards,
Amit Kapila.
de_slot_has_caught_up.
>
I think logical_replication is specific to our pub-sub model but we
can have manually created slots as well. So, it would be better to
name it as binary_upgrade_logical_slot_has_caught_up().
--
With Regards,
Amit Kapila.
num].slot_arr.nslots;
>
> first_time = false;
> }
>
> return slot_count;
> }
>
This may not be a problem but this is also not a function that will be
used frequently. I am not sure if adding such code optimizations is
worth it.
> 7. A typo: s/slotname/slot name. "slot name" looks better in user
> visible messages.
> +pg_log(PG_VERBOSE, "slotname: \"%s\", plugin: \"%s\", two_phase: %s",
>
If we want to follow other parameters then we can even use slot_name.
--
With Regards,
Amit Kapila.
= 0)
+ {
+ rawname = pstrdup(standby_slot_names);
+ SplitIdentifierString(rawname, ',', &standby_slot_names_list);
How does this handle the case where '*' is specified for standby_slot_names?
--
With Regards,
Amit Kapila.
ord_required, streaming, two_phase where true or false indicates
whether that option is enabled or not, I am thinking about whether
enable_failover is an appropriate name for this option. The other
option name that comes to mind is 'failover' where true indicates that
the correspondin
ots' is tested here.
>
I think we don't need the second part of the comment: "Two GUCs ...".
Ideally, we should test each parameter's invalid value but that could
be costly, so I think it is okay to test a few of them.
--
With Regards,
Amit Kapila.
On Fri, Oct 13, 2023 at 11:08 AM Amit Kapila wrote:
>
> On Fri, Oct 13, 2023 at 10:04 AM vignesh C wrote:
> >
> > On Thu, 12 Oct 2023 at 11:10, Amit Kapila wrote:
> > >
> > > On Sun, Oct 8, 2023 at 8:22 AM vignesh C wrote:
> > > >
> &g
On Mon, Oct 16, 2023 at 12:47 PM Michael Paquier wrote:
>
> On Fri, Oct 13, 2023 at 03:20:30PM +0530, Amit Kapila wrote:
> > I would prefer to associate the new parameter 'flush' with
> > non-transactional messages as per the proposed patch.
>
> Check.
&
lize subscriber cluster
+my $subscriber = PostgreSQL::Test::Cluster->new('subscriber');
+$subscriber->init(allows_streaming => 'logical');
Why do we need to set wal_level as logical for subscribers?
--
With Regards,
Amit Kapila.
diff --git a/src/backend/replication/l
On Thu, Oct 12, 2023 at 9:03 PM Tomas Vondra
wrote:
>
> On 7/25/23 12:20, Amit Kapila wrote:
> > ...
> >
> > I have used the debugger to reproduce this as it needs quite some
> > coordination. I just wanted to see if the sequence can go backward and
> > d
ar buffer
pool [1]. You have not provided any explanation as to whether that
approach will have any merits after we do this or whether that
approach is not worth pursuing at all.
[1] - https://commitfest.postgresql.org/43/3514/
--
With Regards,
Amit Kapila.
On Fri, Oct 13, 2023 at 2:03 PM Dilip Kumar wrote:
>
> On Fri, Oct 13, 2023 at 11:07 AM Amit Kapila wrote:
> >
> > > But is this a problem? basically, we will set the
> > > ShmemVariableCache->nextOid counter to the value that we want in the
> > > IsBina
p;& flush)
Two ifs in the above comment sound a bit odd but if we want to keep it
like that then adding 'and' before the second if may slightly improve
it.
--
With Regards,
Amit Kapila.
On Fri, Oct 13, 2023 at 10:04 AM vignesh C wrote:
>
> On Thu, 12 Oct 2023 at 11:10, Amit Kapila wrote:
> >
> > On Sun, Oct 8, 2023 at 8:22 AM vignesh C wrote:
> > >
> >
> > --- a/src/include/catalog/pg_subscription.h
> > +++ b/src/include/c
On Fri, Oct 13, 2023 at 10:37 AM Dilip Kumar wrote:
>
> On Fri, Oct 13, 2023 at 9:29 AM Amit Kapila wrote:
> >
> > On Fri, Oct 13, 2023 at 12:00 AM Robert Haas wrote:
> > >
> > > On Thu, Oct 12, 2023 at 7:17 AM Amit Kapila
> > > wrote:
> &g
On Fri, Oct 13, 2023 at 12:00 AM Robert Haas wrote:
>
> On Thu, Oct 12, 2023 at 7:17 AM Amit Kapila wrote:
> > Now, as mentioned in the first paragraph, it seems we anyway don't
> > need to reset the WAL at the end when setting the next OID for the new
> > cluster w
s?
[1] - https://commitfest.postgresql.org/45/4273/
--
With Regards,
Amit Kapila.
required flag to notify the message's existence
to the caller side. Usually, the flag is set when either the COMMIT or
ABORT records are decoded, but this must be turned on here because the
non-transactional logical message is decoded without waiting for these
records."
--
With Regards,
Amit Kapila.
poses a risk of breaking extensions. In this case, if we want, we
can try to squeeze some padding space or we even can fix it without
introducing a new member. OTOH, it is already debatable whether to fix
it in back branches, so we can even commit this patch just in HEAD.
Thoughts?
--
With Regar
On Mon, Oct 9, 2023 at 12:15 PM Peter Smith wrote:
>
> On Mon, Oct 9, 2023 at 3:32 PM Amit Kapila wrote:
> >
>
> In v1, I used the same pattern as on the CREATE SUBSCRIPTION page,
> which doesn't look like those...
>
Yeah, I think it would have been better if
e can't follow the trick used in exec_bind_message() to
maintain the convention that StringInfos have a trailing null. David,
do you see any better way to fix this case?
--
With Regards,
Amit Kapila.
On Tue, Oct 10, 2023 at 6:17 PM Amit Kapila wrote:
>
> DecodeTXNNeedSkip(LogicalDecodingContext *ctx, XLogRecordBuffer *buf,
> Oid txn_dbid, RepOriginId origin_id)
> {
> - return (SnapBuildXactNeedsSkip(ctx->snapshot_builder, buf->origptr) ||
> - (txn_dbid != Inval
+ if (ctx->decoding_mode == DECODING_MODE_SILENT)
+ ctx->output_skipped = true;
+
+ return need_skip;
I think you need to set the new flag only when we are not skipping the
transaction or in other words when we decide to process the
transaction. Otherwise, how will you distinguish the case where the
xact is already decoded and sent to client?
--
With Regards,
Amit Kapila
nks? I am not against it if we think that is useful
for users but asking as I am not aware of the general practice we
follow in this regard. Does anyone else have any opinion on this
matter?
--
With Regards,
Amit Kapila.
On Sat, Oct 7, 2023 at 3:46 AM Amit Kapila wrote:
>
> On Fri, Oct 6, 2023 at 6:30 PM Hayato Kuroda (Fujitsu)
> >
> > Based on that, I added another binary function
> > binary_upgrade_create_logical_replication_slot().
> > This function is similar to pg_create_logic
a bonus a _missing_ "the"). See the
> > attached patch (which includes your originl one, for completeness).
>
> Thanks, Looks good.
>
Pushed.
--
With Regards,
Amit Kapila.
context)
> - enable_failover at logical slot creation + API to enable/disable it at wish
> - enable_syncslot on the standbys
>
Thanks, these definitions sounds reasonable to me.
--
With Regards,
Amit Kapila.
h. For
example, sql-altersubscription-params-new-name. The other places like
alter_role.sgml and alter_table.sgml uses similar id's. Is there a
reason to be different here?
--
With Regards,
Amit Kapila.
ut
subscription or role changes. Note that role's superuser privilege can
be revoked."
--
With Regards,
Amit Kapila.
made me curious about other duplicate word occurrences, and after a
> few minutes of increasingly-elaborate regexing to exclude false
> postives, I found a few more (plus a bonus a _missing_ "the"). See the
> attached patch (which includes your originl one, for completeness).
>
LGTM.
--
With Regards,
Amit Kapila.
On Thu, Oct 5, 2023 at 6:43 PM Bharath Rupireddy
wrote:
>
> On Thu, Oct 5, 2023 at 1:48 AM Amit Kapila wrote:
> >
>
> > The other potential problem Andres pointed out is that during shutdown
> > if due to some reason, the walreceiver goes down, we won't be able
al_slot_infos() needs to be adjusted as per
recent changes.
2.
@@ -1268,7 +1346,11 @@ stream_start_cb_wrapper(ReorderBuffer *cache,
ReorderBufferTXN *txn,
LogicalErrorCallbackState state;
ErrorContextCallback errcallback;
- Assert(!ctx->fast_forward);
+ /*
+ * In silent mode all the two-phase callbacks are not set so that the
+ * wrapper should not be called.
+ */
+ Assert(ctx->decoding_mode == DECODING_MODE_NORMAL);
This and other similar comments doesn't seems to be consistent as the
function name and comments are not matching.
With Regards,
Amit Kapila.
On Wed, Oct 4, 2023 at 5:34 PM Drouvot, Bertrand
wrote:
>
> On 10/4/23 1:50 PM, shveta malik wrote:
> > On Wed, Oct 4, 2023 at 5:00 PM Amit Kapila wrote:
> >>
> >> On Wed, Oct 4, 2023 at 11:55 AM Drouvot, Bertrand
> >> wrote:
> >>>
> >
On Thu, Oct 5, 2023 at 2:29 PM Dilip Kumar wrote:
>
> On Thu, Oct 5, 2023 at 1:48 AM Amit Kapila
wrote:
> >
> > On Tue, Oct 3, 2023 at 9:58 AM Bharath Rupireddy
> > wrote:
> > >
> > > On Fri, Sep 29, 2023 at 5:27 PM Hayato Kuroda (Fujitsu)
> &
required WALs. As we don't have such functionality available and
it won't be easy to achieve the same, we can leave this for now.
Thoughts?
--
With Regards,
Amit Kapila.
On Wed, Oct 4, 2023 at 11:55 AM Drouvot, Bertrand
wrote:
>
> On 10/4/23 6:26 AM, shveta malik wrote:
> > On Wed, Oct 4, 2023 at 5:36 AM Amit Kapila wrote:
> >>
> >>
> >> How about an alternate scheme where we define sync_slot_names on
> >> stan
On Tue, Oct 3, 2023 at 9:27 PM shveta malik wrote:
>
> On Tue, Oct 3, 2023 at 7:56 PM Drouvot, Bertrand
> wrote:
> >
> > Hi,
> >
> > On 10/3/23 12:54 PM, Amit Kapila wrote:
> > > On Mon, Oct 2, 2023 at 11:39 AM Drouvot, Bertrand
> > > wrote:
On Mon, Oct 2, 2023 at 11:39 AM Drouvot, Bertrand
wrote:
>
> On 9/29/23 1:33 PM, Amit Kapila wrote:
> > On Thu, Sep 28, 2023 at 6:31 PM Drouvot, Bertrand
> > wrote:
> >>
> >
> >> - probably open corner cases like: what if a standby is down? would that
do for it during decoding but the check
in upgrade function expects the reverse value. For example, for WAL
record type XLOG_HEAP_INSERT, the API returns true and that is
indication to the caller that this is an expected record after
confirmed_flush LSN location which doesn't seem correct. Am I missing
something?
--
With Regards,
Amit Kapila.
On Wed, Sep 27, 2023 at 9:14 AM Michael Paquier wrote:
>
> On Tue, Sep 26, 2023 at 09:40:48AM +0530, Amit Kapila wrote:
> > On Mon, Sep 25, 2023 at 11:43 AM Michael Paquier
> > wrote:
> >> Sure, that's assuming that the publisher side is upgraded.
> >
&g
On Thu, Sep 28, 2023 at 6:31 PM Drouvot, Bertrand
wrote:
>
> On 9/25/23 6:10 AM, shveta malik wrote:
> > On Fri, Sep 22, 2023 at 3:48 PM Amit Kapila wrote:
> >>
> >> On Thu, Sep 21, 2023 at 9:16 AM shveta malik
> >> wrote:
> >>>
> >>&
n we don't need to deal with WAL record
types at all.
--
With Regards,
Amit Kapila.
On Thu, Sep 28, 2023 at 2:22 PM Bharath Rupireddy
wrote:
>
> On Fri, Sep 22, 2023 at 9:40 AM Michael Paquier wrote:
> >
> > On Thu, Sep 21, 2023 at 01:50:28PM +0530, Amit Kapila wrote:
> > > We have discussed this point. Normally, we don't have such options in
&g
801 - 900 of 3570 matches
Mail list logo
