On Wed, Dec 22, 2021 at 5:39 PM houzj.f...@fujitsu.com
wrote:
>
> On Wed, Dec 22, 2021 11:36 AM Masahiko Sawada wrote:
> > On Tue, Dec 21, 2021 at 10:24 PM Amit Kapila
> > wrote:
>
> The patch looks mostly good to me.
> I only have few comments.
>
> 1)
&
On Wed, Dec 22, 2021 at 6:22 PM Amit Kapila wrote:
>
> On Wed, Dec 22, 2021 at 5:39 PM houzj.f...@fujitsu.com
> wrote:
> >
> >
> > 2)
> > +#include "utils/rel.h"
> > +#include "utils/lsyscache.h"
> > +#include "utils/memutils
ll recompile postgres with your patch to debug.
>
Okay, that might help.
--
With Regards,
Amit Kapila.
On Thu, Dec 23, 2021 at 10:56 AM Masahiko Sawada wrote:
>
> On Wed, Dec 22, 2021 at 10:55 PM Amit Kapila wrote:
> >
> > On Wed, Dec 22, 2021 at 6:22 PM Amit Kapila wrote:
> > >
> > > On Wed, Dec 22, 2021 at 5:39 PM houzj.f...@fujitsu.com
>
>
> e.g.
> BEFORE:
> + A nullable column in the WHERE clause could cause the
> + expression to evaluate to false; avoid using columns without not-null
> + constraints in the WHERE clause.
> AFTER:
> + A nullable column in the WHERE clause could cause the
> + expression to evaluate to false. To avoid unexpected results, any possible
> + null values should be accounted for.
>
Your suggested wording sounds reasonable to me. Euler, others, any thoughts?
--
With Regards,
Amit Kapila.
have replication slots always have two_phase_at value and remove the
> two_phase field from the view.
>
I am not sure how that will work because we can allow streaming of
prepared transactions when the same is enabled at the CREATE
SUBSCRIPTION time, the default for which is false.
--
With Regards,
Amit Kapila.
On Mon, Dec 27, 2021 at 9:54 AM Masahiko Sawada wrote:
>
> On Thu, Dec 16, 2021 at 2:42 PM Masahiko Sawada wrote:
> >
> > On Thu, Dec 16, 2021 at 2:21 PM Amit Kapila wrote:
> > >
> > > On Thu, Dec 16, 2021 at 10:37 AM Masahiko Sawada
> > > wrote:
y possible value is
> REORDER_BUFFER_CHANGE_UPDATE. I will make these fixes in a future
> version.
>
That sounds fine to me too. One more thing is that you don't need to
modify the action in case it remains update as the caller has already
set that value. Currently, we are modifying
On Wed, Dec 22, 2021 at 5:26 AM Peter Smith wrote:
>
> On Mon, Dec 20, 2021 at 9:30 PM Amit Kapila wrote:
> >
> > On Mon, Dec 20, 2021 at 8:41 AM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > Thanks for the comments, I agree with all the comments.
>
do not include UPDATE and DELETE.
> + */
>
> Some minor rewording of the comment:
>
...
> "UPDATE and DELETE" --> "UPDATE or DELETE"
>
The existing comment seems correct to me. Hou-San can confirm it once
as I think this is written by him.
--
With Regards,
Amit Kapila.
create publication pub for table t1 where (10);
^
Also, transformPubWhereClauses() seems to be returning the same list
as it was passed to it. Do we really need to return anything from
transformPubWhereClauses()?
--
With Regards,
Amit Kapila.
On Wed, Jan 5, 2022 at 2:45 PM Amit Kapila wrote:
>
> On Tue, Dec 28, 2021 at 6:33 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Mon, Dec 27, 2021 9:19 PM Hou Zhijie wrote:
> > > On Mon, Dec 27, 2021 9:16 PM houzj.f...@fujitsu.com
> > >
> > > wr
.c
accepts invalidation messages only when lock mode is not 'NoLock'.
[1] -
https://www.postgresql.org/message-id/CAA4eK1Ks%2Bp8wDbzhDr7yMYEWDbWFRJAd_uOY-moikc%2Bzr9ER%2Bg%40mail.gmail.com
--
With Regards,
Amit Kapila.
On Thu, Jan 6, 2022 at 8:43 AM Peter Smith wrote:
>
> On Wed, Jan 5, 2022 at 9:52 PM Amit Kapila wrote:
> >
> ...
>
> > Another minor comment:
> > +static bool pgoutput_row_filter(enum ReorderBufferChangeType changetype,
> >
> > Do we need to specify t
On Wed, Jan 5, 2022 at 9:48 AM Dilip Kumar wrote:
>
> On Wed, Jan 5, 2022 at 9:01 AM Amit Kapila wrote:
> >
> > On Mon, Dec 27, 2021 at 9:54 AM Masahiko Sawada
> > wrote:
> >
> > Do you mean to say that you want to omit it even when we are
> > committ
ee(pg_catalog.pg_partition_root(%u)))",
+ lrel->remoteid);
IIUC, this doesn't deal with cases when some publication has not
specified table attrs. In those cases, I think it should return all
attrs? Also, it is not very clear to me what exactly we want to do
with partitions?
--
With Regards,
Amit Kapila.
eter
publish_via_partition_root determines if it uses the partition row
filter (if the parameter is false, the default) or the root
partitioned table row filter. During initial tablesync, it doesn't do
any special handling for partitions.
--
With Regards,
Amit Kapila.
On Fri, Jan 7, 2022 at 1:28 AM Tom Lane wrote:
>
> Amit Kapila writes:
>
> > + * *during* a callback if we do any syscache or table access in the
> > + * callback.
>
> > As we don't take locks on tables, can invalidation events be accepted
> > during t
On Thu, Jan 6, 2022 at 6:42 PM Euler Taveira wrote:
>
> On Thu, Jan 6, 2022, at 1:18 AM, Amit Kapila wrote:
>
> On Thu, Jan 6, 2022 at 8:43 AM Peter Smith wrote:
> >
> > On Wed, Jan 5, 2022 at 9:52 PM Amit Kapila wrote:
> > >
> > ...
> >
&g
On Fri, Jan 7, 2022 at 6:35 AM Masahiko Sawada wrote:
>
> On Wed, Jan 5, 2022 at 12:31 PM Amit Kapila wrote:
> >
> > On Mon, Dec 27, 2021 at 9:54 AM Masahiko Sawada
> > wrote:
> > >
> > > On Thu, Dec 16, 2021 at 2:42 PM Masahiko Sawada
> > &g
On Fri, Jan 7, 2022 at 9:44 AM Amit Kapila wrote:
>
> On Thu, Jan 6, 2022 at 6:42 PM Euler Taveira wrote:
> >
> > IMO we shouldn't reuse ReorderBufferChangeType. For a long-term solution,
> > it is
> > fragile. ReorderBufferChangeType has values that do not ma
On Fri, Jan 7, 2022 at 12:05 PM Amit Kapila wrote:
>
> On Fri, Jan 7, 2022 at 9:44 AM Amit Kapila wrote:
> >
> > On Thu, Jan 6, 2022 at 6:42 PM Euler Taveira wrote:
> > >
> > > IMO we shouldn't reuse ReorderBufferChangeType. For a long-
UC, after restoring
4096 changes from snap files, we send them to the subscriber, and then
apply worker should apply those one by one. Now, is it taking one minute to
restore 4096 changes due to which apply worker is timed out?
Could this function in* Apply main loop* in worker.c help to find a
> solution?
>
> rc = WaitLatchOrSocket(MyLatch,
> WL_SOCKET_READABLE | WL_LATCH_SET |
> WL_TIMEOUT | WL_POSTMASTER_DEATH,
> fd, wait_time,
> WAIT_EVENT_LOGICAL_APPLY_MAIN);
>
Can you explain why you think this will help in solving your current
problem?
--
With Regards,
Amit Kapila.
I also raised the same point [2] related to INSERTs.
[1] -
https://www.postgresql.org/message-id/OS0PR01MB5716330FFE3803DF887D073C94789%40OS0PR01MB5716.jpnprd01.prod.outlook.com
[2] -
https://www.postgresql.org/message-id/CAA4eK1%2BFoJ-J7wUG5s8zCtY0iBuN9LcjQcYhV4BD17xhuHfoug%40mail.gmail.com
--
With Regards,
Amit Kapila.
On Fri, Jan 7, 2022 at 11:20 PM Euler Taveira wrote:
>
> On Fri, Jan 7, 2022, at 6:05 AM, Amit Kapila wrote:
>
> Euler, I have one more question about this patch for you. I see that
> in the patch we are calling coerce_to_target_type() in
> pgoutput_row_filter_init_expr() but
icy forbids.
>
> Version 4 of the patch, attached.
>
For Update/Delete, we do read the table first via
FindReplTupleInLocalRel(), so is there a need to check ACL_SELECT
before that?
--
With Regards,
Amit Kapila.
On Sat, Jan 8, 2022 at 1:01 PM Jeff Davis wrote:
>
> On Sat, 2022-01-08 at 12:27 +0530, Amit Kapila wrote:
> > For Update/Delete, we do read the table first via
> > FindReplTupleInLocalRel(), so is there a need to check ACL_SELECT
> > before that?
>
> If it's
ould like to know the time window where s1 is
> not actively acknowledging the commits and the writes are dependent on s2.
> Also if the service layer decides to failover to s2 instead of s1 because s1
> is lagging I need evidence in the log to explain the behavior.
>
Isn't it bet
p rewrites have to log enough
> - * information to allow the decoding backend to updates its internal mapping
> + * information to allow the decoding backend to update its internal mapping
>
LGTM.
--
With Regards,
Amit Kapila.
n. If there are, then prepare the necessary ExprState and cache
it in entry->exprstate. To build an expression state, we need to
ensure the following:"
--
With Regards,
Amit Kapila.
On Thu, Dec 16, 2021 at 11:12 AM Masahiko Sawada wrote:
>
> On Thu, Dec 16, 2021 at 2:21 PM Amit Kapila wrote:
> >
> > >
> > > So if skip_xid is already changed, the apply worker would do
> > > replorigin_advance() with WAL logging, instead
ome automatic functionality like you are proposing or
something else, it could be done as a separate patch but let's wait
and see what Sawada-San or others think about this?
--
With Regards,
Amit Kapila.
we need to be careful to avoid any duplicate updates in
docs, other than that I think this will be helpful.
--
With Regards,
Amit Kapila.
On Mon, Jan 10, 2022 at 10:22 AM Amit Kapila wrote:
>
> On Mon, Jan 10, 2022 at 9:42 AM Bharath Rupireddy
> wrote:
> >
> > Hi,
> >
> > It looks like there's a typo, attaching a tiny patch to fix it.
> >
> > *
> > * When doing logic
On Tue, Jan 11, 2022 at 8:52 AM Masahiko Sawada wrote:
>
> On Mon, Jan 10, 2022 at 8:50 PM Amit Kapila wrote:
> >
> > I was thinking what if we don't advance origin explicitly in this
> > case? Actually, that will be no different than the transactions where
> >
pr(pr.prqual, pr.prrelid)
> FROM pg_publication p
> LEFT OUTER JOIN pg_publication_rel pr
> ON (p.oid = pr.prpubid AND pr.prrelid = 16387),
> LATERAL pg_get_publication_tables(p.pubname) GPT
> WHERE GPT.relid = 16387 AND p.pubname IN ( 'puba', 'pubb' );
> pg_get_expr
> -
> (a > 10)
>
> (2 rows)
>
One advantage of this query is that it seems to have simplified the
original query by removing NOT conditions. I haven't tested this yet
but logically it appears correct to me.
--
With Regards,
Amit Kapila.
On Tue, Jan 11, 2022 at 1:51 PM Masahiko Sawada wrote:
>
> On Tue, Jan 11, 2022 at 3:12 PM Amit Kapila wrote:
> >
> > On Tue, Jan 11, 2022 at 8:52 AM Masahiko Sawada
> > wrote:
> > >
> > > On Mon, Jan 10, 2022 at 8:50 PM Amit Kapila
> > &g
On Tue, Jan 11, 2022 at 8:52 AM Masahiko Sawada wrote:
>
> On Mon, Jan 10, 2022 at 8:50 PM Amit Kapila wrote:
> >
> >
> > Few other comments on the latest patch:
> > =
> > 1.
> > A conflict will produce an error
| 642064 | ExclusiveLock
> | t | t
>
> virtualxid | | | | | 1/1|
> | | | | 1/0| 17333 | ExclusiveLock
> | t | t
>
> (3 rows)
>
It seems both master and standby have an exclusive lock on db:16384
and relation:12141. Which is this database/relation and why is the
app/database holding a lock on it?
--
With Regards,
Amit Kapila.
On Wed, Jan 12, 2022 at 5:49 AM Masahiko Sawada wrote:
>
> On Tue, Jan 11, 2022 at 7:08 PM Amit Kapila wrote:
> >
> > On Tue, Jan 11, 2022 at 1:51 PM Masahiko Sawada
> > wrote:
> > >
> > > On second thought, the same is true for other cases, for ex
hen
> it enters pgoutput, and then used as a slot throughout.
>
One another thing that we can improve about 0002 is to unify the APIs
for row filtering for update and insert/delete. I find having separate
APIs a bit awkward.
--
With Regards,
Amit Kapila.
#x27;m therfore volunteering to manage this commitfest,
>
Thanks!
--
With Regards,
Amit Kapila.
phase, the snap files must be decoded. However after one minute
> (wal_receiver_timeout parameter set to 1 minute) the worker process stop
> with a timeout.
>
>
What exactly do you mean by the first and second phase in the above
description?
--
With Regards,
Amit Kapila.
only have old tuple.
> + */
>
> There are several things not quite right with that comment:
> a. I thought now it should refer to "slots" instead of "tuples"
>
I feel tuple still makes sense as it makes the comments/code easy to understand.
--
With Regards,
Amit Kapila.
On Wed, Jan 12, 2022 at 7:19 PM houzj.f...@fujitsu.com
wrote:
>
> On Wed, Jan 12, 2022 5:38 PM Amit Kapila wrote:
>
> Attach the v63 patch set which include the following changes.
>
Few comments:
=
1.
+
+
+
+ prqual pg_node_tree
+
+ Expr
skip sending the change.
--
With Regards,
Amit Kapila.
> false."). Then, that 2nd text can be removed (from where it is later
> > in this same comment).
>
> Hi Hou-san, thanks for all the v64 updates!
>
> I think the above comment was only partly fixed.
>
> The v64-0001 comment still says:
> + * If it returns true, the change is replicated, otherwise, it is not.
>
...
...
>
> But maybe it is best to rearrange the whole thing like:
> "Returns true if the change is to be replicated, else false."
>
+1 to change as per this suggestion.
--
With Regards,
Amit Kapila.
n the publications, whether
publish_via_partition_root is true or false.
We have done some testing w.r.t above cases with both patches and my
colleague will share the results.
--
With Regards,
Amit Kapila.
e problem. Can you try by calling
WalSndKeepaliveIfNecessary() instead of WalSndKeepalive()?
--
With Regards,
Amit Kapila.
s code comments.
>
LGTM. I'll take care of this unless someone thinks otherwise.
--
With Regards,
Amit Kapila.
On Fri, Jan 14, 2022 at 7:08 PM Alvaro Herrera wrote:
>
> On 2022-Jan-14, Amit Kapila wrote:
>
> > 1. Replica Identity handling: Currently the column filter patch gives
> > an error during create/alter subscription if the specified column list
> > is invalid (Replica I
origin_lsn,
+ TimestampTz origin_timestamp)
{
..
..
+ if (!IsTransactionState())
+ StartTransactionCommand();
..
..
+ CommitTransactionCommand();
..
}
The transaction should be committed in this function if it is started
here otherwise it should be the responsibility of the caller to commit
it.
--
With Regards,
Amit Kapila.
+ else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, "SKIP"))
+ COMPLETE_WITH("(");
I might be missing something but why do you think the handling of SKIP
be any different than what we are doing for SET?
--
With Regards,
Amit Kapila.
On Fri, Jan 14, 2022 at 8:05 PM Amit Kapila wrote:
>
> On Fri, Jan 14, 2022 at 8:55 AM Bharath Rupireddy
> wrote:
> >
> > The function CreateCheckPoint is specified as CreateCheckpoint in some
> > of the code comments whereas in other places it is correctly
> >
On Mon, Jan 17, 2022 at 9:49 AM Masahiko Sawada wrote:
>
> On Sat, Jan 15, 2022 at 7:24 PM Amit Kapila wrote:
> >
>
> > 6.
> > +static void
> > +maybe_start_skipping_changes(TransactionId xid)
> > +{
> > + Assert(!is_skipping_changes());
>
On Wed, Mar 17, 2021 at 4:56 AM Peter Smith wrote:
>
> On Wed, Mar 17, 2021 at 12:45 AM Amit Kapila wrote:
> >
> >
> > Attached, please find the patch to update the description of substream
> > in pg_subscription.
> >
>
> I applied your patch and r
e the similar test. Pushed!
--
With Regards,
Amit Kapila.
On Wed, Mar 17, 2021 at 8:07 AM vignesh C wrote:
>
> On Tue, Mar 16, 2021 at 7:22 PM Amit Kapila wrote:
> >
> > On Tue, Mar 16, 2021 at 6:22 PM vignesh C wrote:
> > >
> > > On Mon, Mar 15, 2021 at 6:14 PM Ajin Cherian wrote:
> > > >
>
On Tue, Mar 16, 2021 at 5:03 PM Amit Kapila wrote:
>
> On Mon, Mar 15, 2021 at 6:14 PM Ajin Cherian wrote:
> >
> > Here's a new patch-set that implements this new solution proposed by Amit.
> > Patchset-v60 implements:
> >
>
> I have reviewed the latest
napshot().
>
> Done:
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=5f79580ad69f6e696365bdc63bc265f45bd77211
>
Thank you!
--
With Regards,
Amit Kapila.
t;
>
Your patch looks good to me. While checking this, I notice a typo in
the previous patch:
- planner parameter parallel_workers.
+ planner parameter parallel_workers and
+ parallel_insert_enabled.
Here, it should be /planner parameter/planner parameters.
--
With Regards,
Amit Kapila.
On Wed, Mar 17, 2021 at 11:27 AM Amit Kapila wrote:
>
> 5. I have modified the comments atop worker.c to explain the design
> and some of the problems clearly. See attached. If you are fine with
> this, please include it in the next version of the patch.
>
I have further expanded
D,
leading to an error on the subscriber and making the publisher wait
forever.
Thoughts?
[1] -
https://www.postgresql.org/message-id/CAHut%2BPv3X7YH_nDEjH1ZJf5U6M6DHHtEjevu7PY5Dv5071jQ4A%40mail.gmail.com
--
With Regards,
Amit Kapila.
> > + planner parameter parallel_workers and
> > + parallel_insert_enabled.
> >
> > Here, it should be /planner parameter/planner parameters.
>
> Thanks amit and justin for pointing this out !
> The changes looks good to me.
>
Pushed!
--
With Regards,
Amit Kapila.
On Thu, Mar 18, 2021 at 5:31 PM vignesh C wrote:
>
> On Thu, Mar 18, 2021 at 3:16 PM Amit Kapila wrote:
> >
> >
> > In short, on the subscriber, both the apply workers (corresponding to
> > two subscriptions) are getting the same prepare transaction GID,
> > l
On Fri, Mar 19, 2021 at 9:22 PM Markus Wanner
wrote:
>
> On 18.03.21 10:45, Amit Kapila wrote:
> > While reviewing/testing subscriber-side work for $SUBJECT [1], I
> > noticed a problem that seems to need a broader discussion, so started
> > this thread. We can get prepa
On Sat, Mar 20, 2021 at 7:07 AM Ajin Cherian wrote:
>
> On Sat, Mar 20, 2021 at 1:35 AM Amit Kapila wrote:
>>
>> On Fri, Mar 19, 2021 at 5:03 AM Ajin Cherian wrote:
>> >
>> > Missed the patch - 0001, resending.
>> >
>>
>>
>> @
nquire about slots by index as well and get the list of slots to report
> stats for from slot.c infrastructure.
>
But how will you detect in your idea that some of the stats from the
already dropped slot?
I'll create an entry for this in PG14 Open items wiki.
--
With Regards,
Amit Kapila.
On Sat, Mar 20, 2021 at 9:25 AM Amit Kapila wrote:
>
> On Sat, Mar 20, 2021 at 12:22 AM Andres Freund wrote:
> >
> > And then more generally about the feature:
> > - If a slot was used to stream out a large amount of changes (say an
> > initial data load), but t
On Sat, Mar 20, 2021 at 2:57 PM Markus Wanner
wrote:
>
> On 20.03.21 03:17, Amit Kapila wrote:
> > Are you saying that users might use the same GID which we have
> > constructed internally (say by combining origin and xid: originid_xid)
> > and then there will be confl
On Sat, Mar 20, 2021 at 4:02 PM Dilip Kumar wrote:
>
> On Sat, Mar 20, 2021 at 7:50 AM Amit Kapila wrote:
> >
> > On Fri, Mar 19, 2021 at 9:22 PM Markus Wanner
> > wrote:
>
> > So, I think you are using xid of publisher and origin_id of
> > subscription
On Sat, Mar 20, 2021 at 8:53 PM Amit Kapila wrote:
>
> On Sat, Mar 20, 2021 at 4:02 PM Dilip Kumar wrote:
> >
> > On Sat, Mar 20, 2021 at 7:50 AM Amit Kapila wrote:
> > >
> > > On Fri, Mar 19, 2021 at 9:22 PM Markus Wanner
> > > wrote:
> >
&
On Sun, Mar 21, 2021 at 2:47 PM Markus Wanner
wrote:
>
> On 20.03.21 16:14, Amit Kapila wrote:
> > Right, but I guess in our case using user-provided GID will conflict
> > if we use multiple subscriptions on the same node. So, it is better to
> > generate a uniqu
the last parameter of hash_search API and then
perform Assert based on that? See similar usage in reorderbuffer.c and
rewriteheap.c.
--
With Regards,
Amit Kapila.
On Sun, Mar 21, 2021 at 2:57 AM Andres Freund wrote:
>
> Hi,
>
> On 2021-03-20 10:28:06 +0530, Amit Kapila wrote:
> > On Sat, Mar 20, 2021 at 9:25 AM Amit Kapila wrote:
> > > This idea is worth exploring to address the complaints but what do we
> > > do when w
On Sun, Mar 21, 2021 at 2:56 AM Andres Freund wrote:
>
> On 2021-03-20 09:25:40 +0530, Amit Kapila wrote:
> > On Sat, Mar 20, 2021 at 12:22 AM Andres Freund wrote:
> > >
> > > And then more generally about the feature:
> > > - If a slot was used to strea
On Sat, Mar 13, 2021 at 3:43 PM Amit Kapila wrote:
>
> On Thu, Mar 11, 2021 at 2:44 PM Markus Wanner
> wrote:
> >
> > On 11.03.21 04:58, Amit Kapila wrote:
> > > But this happens when we are decoding prepare, so it is clear that the
> > > transactio
On Mon, Mar 22, 2021 at 3:20 AM Peter Smith wrote:
>
> On Sun, Mar 21, 2021 at 8:54 PM Amit Kapila wrote:
> >
> > On Sat, Mar 20, 2021 at 12:54 PM Peter Smith wrote:
> > >
> > > PSA my patch to correct this by firstly doing a HASH_FIND, then only
> > &g
On Mon, Mar 22, 2021 at 3:10 AM Andres Freund wrote:
>
> On 2021-03-21 16:08:00 +0530, Amit Kapila wrote:
> > On Sun, Mar 21, 2021 at 2:57 AM Andres Freund wrote:
> > > On 2021-03-20 10:28:06 +0530, Amit Kapila wrote:
> > > > On Sat, Mar 20, 2021 at 9:
On Mon, Mar 22, 2021 at 7:57 AM Amit Kapila wrote:
>
> On Mon, Mar 22, 2021 at 3:20 AM Peter Smith wrote:
> >
> > On Sun, Mar 21, 2021 at 8:54 PM Amit Kapila wrote:
> > >
> > > On Sat, Mar 20, 2021 at 12:54 PM Peter Smith
> > > wrote:
> >
new
> stats for the slot. Is that what you mean, Andres?
>
I wonder how in this scheme, we will remove the risk of running out of
'replSlotStats' and still restore correct stats assuming the drop
message is lost? Do we want to check after restoring each slot info
whether the slot with that name exists?
--
With Regards,
Amit Kapila.
we would have some OID type of
identifier for each slot. But, without that may be index location of
ReplicationSlotCtl->replication_slots and slotname combination can
reduce the chances of slot stats go wrong quite less even if not zero.
If not name, do we have anything else in a slot that can be used for
some sort of sanity checking?
--
With Regards,
Amit Kapila.
the state as PENDING, which
+ * allows ALTER SUBSCRIPTION ... REFRESH PUBLICATION to work.
+ */
+ become_two_phase_enabled = list_length(subrels) > 0;
This code is similar at both the places it is used. Isn't it better to
move this inside AllTablesyncsReady and if required then we can change
the name of the function.
--
With Regards,
Amit Kapila.
rm users
> to not have locks
> * on catalog tables in such transactions.
> */
>
> Since we will have in-core implementation of prepares, should we update the
> comments here ?
>
Fixed this in the latest patch posted by me. I have additionally
updated the docs to reflect the same.
--
With Regards,
Amit Kapila.
On Thu, Mar 25, 2021 at 11:36 AM Masahiko Sawada wrote:
>
> On Wed, Mar 24, 2021 at 7:06 PM Amit Kapila wrote:
> >
> >
> > Leaving aside restart case, without some sort of such sanity checking,
> > if both drop (of old slot) and create (of new slot) messages a
On Fri, Mar 26, 2021 at 1:17 AM Andres Freund wrote:
>
> Hi,
>
> On 2021-03-25 17:12:31 +0530, Amit Kapila wrote:
> > On Thu, Mar 25, 2021 at 11:36 AM Masahiko Sawada
> > wrote:
> > >
> > > On Wed, Mar 24, 2021 at 7:06 PM Amit Kapila
> > > w
saction status).
>
I think as you have noted that stream_abort or rollback_prepared will
be sent (the remaining changes in-between will be skipped) as we
decode them from WAL so it is not clear to me how it causes any delays
as opposed to where we don't detect concurrent abort say because after
that we didn't access catalog table.
--
With Regards,
Amit Kapila.
On Fri, Mar 26, 2021 at 2:42 PM Markus Wanner
wrote:
>
> On 26.03.21 04:28, Amit Kapila wrote:
> > I think as you have noted that stream_abort or rollback_prepared will
> > be sent (the remaining changes in-between will be skipped) as we
> > decode them from WAL
>
>
On Fri, Mar 26, 2021 at 5:50 PM Markus Wanner
wrote:
>
> On 26.03.21 11:19, Amit Kapila wrote:
> > No, I am not assuming that. I am just trying to describe you that it
> > is not necessary that we will be able to detect concurrent abort in
> > each and every case.
>
a given pair of
Why do you think that this callback can be invoked several times per
transaction? I think it could be called at most two times, once at
prepare time, then at commit or rollback time. So, I think using
'multiple' instead of 'several' times is better.
--
With Regards,
Amit Kapila.
On Mon, Mar 29, 2021 at 11:42 AM Amit Kapila wrote:
>
> On Thu, Mar 25, 2021 at 2:07 PM Markus Wanner
> wrote:
> >
> > On 22.03.21 09:50, Markus Wanner wrote:
> > > thank you for reconsidering this patch. I updated it to include the
> > > required ad
On Mon, Mar 29, 2021 at 12:57 PM Markus Wanner
wrote:
>
> On 29.03.21 08:23, Amit Kapila wrote:
> > On Mon, Mar 29, 2021 at 11:42 AM Amit Kapila
> > wrote:
> >
> > What exactly is the node identifier here? Is it a publisher or
> > subscriber node id? We might
On Mon, Mar 29, 2021 at 12:36 PM Markus Wanner
wrote:
>
> On 27.03.21 07:37, Amit Kapila wrote:
> > Isn't it better to send prepare from the publisher in such a case so
> > that subscribers can know about it when rollback prepared arrives?
>
> That's exactly what
On Mon, Mar 29, 2021 at 3:11 PM Markus Wanner
wrote:
>
> On 29.03.21 11:13, Amit Kapila wrote:
> > This might or might not be valid for all logical replication solutions
> > but in the publisher-subscriber model, it would easily lead to
> > duplicate identifiers and bl
On Mon, Mar 29, 2021 at 4:46 PM Markus Wanner
wrote:
>
> On 29.03.21 13:04, vignesh C wrote:
> > The above content looks sufficient to me.
>
> Good, thanks. Based on that, I'm adding v7 of the patch.
>
Pushed. In the last version, you have named the patch incorrectl
the
following paragraph: "The users that want to decode prepared
transactions need to be careful ."
[1] -
https://www.postgresql.org/docs/devel/logicaldecoding-two-phase-commits.html
--
With Regards,
Amit Kapila.
tion slots".
>
> PSA a trivial patch which (for consistency) now calls them all the
> same - "tablesync slots"
>
+1 for the consistency. But I think it better to use "table
synchronization slots" in the user-facing docs as that makes it easier
for users to understand.
--
With Regards,
Amit Kapila.
ublication \"%s\"",
+ NameStr(pubform->pubname;
+ }
+ }
Is there a reason to deal with this here separately rather than in the
ALTER PUBLICATION grammar?
--
With Regards,
Amit Kapila.
tch. It can be backported till
> > v11 where we added those test cases.
>
> +1 for the change. It looks like a typo and can be backported.
>
Looks good to me as well but I think one can choose not to backpatch
as there is no functional impact but OTOH, there is some value in
keeping tests/code consistent.
--
With Regards,
Amit Kapila.
On Wed, Mar 31, 2021 at 9:35 AM Michael Paquier wrote:
>
> On Tue, Mar 30, 2021 at 05:00:53PM +0530, Amit Kapila wrote:
> > Looks good to me as well but I think one can choose not to backpatch
> > as there is no functional impact but OTOH, there is some value in
> > keepi
701 - 800 of 6720 matches
Mail list logo
