On Fri, Jan 10, 2020 at 12:59:09PM -0500, Tom Lane wrote:
! [ redirecting to -hackers ]
! I modified the kerberos test so that it tries a query with a less
! negligible amount of data, and what I find is:
!
! * passes on Fedora 30, with either default or 1500 mtu
! * passes on FreeBSD 12.0 with d
On Fri, Jan 10, 2020 at 10:51:50PM -0500, Tom Lane wrote:
! Here's a draft patch that cleans up all the logic errors I could find.
Okiee, thank You!
Let's see (was a bit busy yesterday trying to upgrade pgadmin3 -
difficult matter), now lets sort this out:
With the first patch applied (as from Fr
/postgr.es/m/CAH2-Wz=ilnf+0csab37efxcgmrjo1dyjw5hmzm7tp1axg1n...@mail.gmail.com
-- scroll down to "TPC-C", which has the relevant autovacuum log
output for the orders table, covering a 24 hour period
--
Peter Geoghegan
sed on averting MultiXact
wraparound? I'm hoping that the patch that adds smarter tracking of
final relfrozenxid/relminmxid values during VACUUM makes this less of
a problem automatically.
--
Peter Geoghegan
Thanks for addressing my previous comments. Now I have looked at v19.
On Mon, Feb 21, 2022 at 11:25 AM osumi.takami...@fujitsu.com
wrote:
>
> On Friday, February 18, 2022 3:27 PM Peter Smith
> wrote:
> > Hi. Below are my code review comments for v18.
> Thank you for your
eaning of the previous behaviors? Does bad
padding even give correct answers on decryption? What does encryption
without padding even do on incorrect input sizes?From 848289f7458d71742a327c1625c24a30981f9229 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Mon, 21 Feb 2022 11:24:27 +0100
S
On 20.02.22 01:39, Tom Lane wrote:
Hm, wouldn't it be less code to just use printf?
Meh --- it'd be different from the way we do it in the rest
of initdb, and it would not be "less code". Maybe it'd run
a shade faster, but I refuse to believe that that'd be
enough to matter.
There is a PG_CMD
On Mon, Feb 21, 2022 at 11:44 PM osumi.takami...@fujitsu.com
wrote:
>
> On Monday, February 21, 2022 2:56 PM Peter Smith
> wrote:
> > Thanks for addressing my previous comments. Now I have looked at v19.
> >
> > On Mon, Feb 21, 2022 at 11:25 AM osumi.takam
FYI - the latest v18 patch no longer applies due to a recent push [1].
--
[1]
https://github.com/postgres/postgres/commit/52e4f0cd472d39d07732b99559989ea3b615be78
Kind Regards,
Peter Smith.
Fujitsu Australia
On Tue, Feb 22, 2022 at 3:11 PM osumi.takami...@fujitsu.com
wrote:
>
> On Tuesday, February 22, 2022 7:53 AM Peter Smith
> wrote:
> > On Mon, Feb 21, 2022 at 11:44 PM osumi.takami...@fujitsu.com
> > wrote:
> > >
> > > On Monday, February 21, 2022 2:56 PM
were some discussions earlier in the thread that some version of
some patch had broken some use of pagers. Does anyone remember details?
Anything worth testing specifically?From 6e75c1bec73f2128b94131305e6a37b97257f7c3 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Tue, 22 Feb 2022 13:4
slow the last few times I used
it, which wasn't for long enough for it to really matter. This must
have been why. I might have to rescind my recommendation of lld.
--
Peter Geoghegan
- * subworkers is the hash table of PgStat_StatSubWorkerEntry which stores
- * statistics of logical replication workers: apply worker and table sync
- * worker.
+ * tables, functions, and subscription must be last in the struct, because
+ * we don't write the pointers out to the stats file.
*/
Should that say "tables, functions, and subscriptions" (plural)
--
Kind Regards,
Peter Smith.
Fujitsu Australia
ot the tablesync worker (which already completed)
"Truncate test_tab1 so that table sync can continue." --> "Truncate
test_tab1 so that the apply worker can continue."
--
[Osumi]
https://www.postgresql.org/message-id/CAD21AoBRt%3DcyKsZP83rcMkHnT498gHH0TEP34fZBrGCxT-Ahwg%40mail.gmail.com
[Tang]
https://www.postgresql.org/message-id/TYCPR01MB612840D018FEBD38268CC83BFB3C9%40TYCPR01MB6128.jpnprd01.prod.outlook.com
Kind Regards,
Peter Smith.
Fujitsu Australia
ort for a table where the failsafe kicked in).
--
Peter Geoghegan
On 23.02.22 21:30, Andres Freund wrote:
hen verifying that the meson port actually runs all perl based tests I came
across src/interfaces/libpq/test/regress.pl. Instead of running tests yet
another way, it seems better to convert it to a tap test.
I hope others agree?
Where would we want that
On 23.02.22 23:58, Tom Lane wrote:
Peter Eisentraut writes:
On 23.02.22 21:30, Andres Freund wrote:
Where would we want that test to live? Right now we have the slightly odd
convention that some tap tests live in src/test/{misc,modules,...}. But
e.g. frontend binary ones are below src/bin
cgi-bin/show_log.pl?nm=komodoensis&dt=2022-02-23%2016%3A12%3A03
Kind Regards,
Peter Smith.
Fujitsu Australia.
On Thu, Feb 24, 2022 at 1:33 PM Amit Kapila wrote:
>
> On Thu, Feb 24, 2022 at 7:57 AM Peter Smith wrote:
> >
> > I noticed that there was a build-farm failure on the machine 'komodoensis'
> > [1]
> >
> > # Failed test 'check r
not */
(void) hash_search(subscriptionStatHash, (void *) &(msg->m_subid),
HASH_REMOVE, NULL);
}
SUGGESTION
Wouldn't the above code be simpler written like:
if (subscriptionStatHash)
{
/* Remove from hashtable if present; we don't care if it's not */
(void) hash_search(
On 21.02.22 17:17, Andres Freund wrote:
Hi,
On 2022-02-21 14:49:01 +0530, Amit Kapila wrote:
On Mon, Feb 21, 2022 at 1:18 PM Andres Freund wrote:
* stats_reset (Time at which these statistics were last reset)
The view name could be pg_stat_subscription_lrep,
pg_stat_logical_replication, or s
On 23.02.22 03:14, Andres Freund wrote:
Why are the stats stored in the per-database stats file / as a second level
below the database? While they're also associated with a database, it's a
global catalog, so it seems to make more sense to have them "live" globally as
well?
pg_subscription bein
On 24.02.22 02:32, Masahiko Sawada wrote:
On Wed, Feb 23, 2022 at 12:08 PM Peter Smith wrote:
Hi. Below are my review comments for the v1 patch.
Thank you for the comments! I've attached the latest version patch
that incorporated all comments I got so far. The primary change fro
test source
code is laid out, so it makes following the tests and locating failing
test code easier.From 3f50bc5236d7793939904222a38f7e13a2cda47c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Thu, 24 Feb 2022 11:01:47 +0100
Subject: [PATCH v2] Readd use of TAP subtests
On 23.02.22 12:10, Amit Kapila wrote:
Isn't it better to support this with a syntax as indicated by Tom in
one of his earlier emails on this topic [1]? IIUC, it would be as
follows:
CREATE PUBLICATION p FOR ALL TABLES, ALL SEQUENCES;
I don't think there is any point in supporting this. What F
On 18.12.21 00:53, Brar Piening wrote:
The purpose is that you can directly link to the id in the public html
docs which still gets generated (e. g.
https://www.postgresql.org/docs/14/protocol-replication.html#PROTOCOL-REPLICATION-BASE-BACKUP).
Essentially it gives people discussing the prot
On 24.02.22 12:46, Masahiko Sawada wrote:
We have a view called pg_stat_activity, which is very well known. From
that perspective, "activity" means what is happening right now or what
has happened most recently. The reworked view in this patch does not
contain that (we already have pg_stat_su
On 24.02.22 02:52, Tom Lane wrote:
Peter Eisentraut writes:
On 23.02.22 23:58, Tom Lane wrote:
Peter Eisentraut writes:
libpq TAP tests should be in src/interfaces/libpq/t/.
That's failing to account for the fact that a libpq test can't
really be a pure-perl TAP test; you n
On 23.02.22 00:23, Tom Lane wrote:
This conversation seems to have tailed off without full resolution,
but I observe that pretty much everyone except Peter is on board
with defining pg_log_fatal as pg_log_error + exit(1). So I think
we should just do that, unless Peter wants to produce a
On 18.02.22 11:17, Peter Eisentraut wrote:
I noticed that the JSON path lexer does not support the decimal literal
syntax forms
.1
1.
(that is, there are no digits before or after the decimal point). This
is allowed by the relevant ECMAScript standard
(https://262.ecma-international.org
rep` as well as more reliable given specific variations
> in output style
> depending on how the blocks are specified.
Sounds useful to me.
--
Peter Geoghegan
Below are my review comments for the v3 patch.
==
1. Commit message
(An earlier review comment [Peter-v2] #2 was only partly fixed)
"there are no longer any relation information" --> "there is no longer
any relation information"
~~~
2. doc/s
On Sun, Feb 20, 2022 at 12:27 PM Peter Geoghegan wrote:
> You've given me a lot of high quality feedback on all of this, which
> I'll work through soon. It's hard to get the balance right here, but
> it's made much easier by this kind of feedback.
Attached is v9
SyncRepWaitForLSN' before it was turned
into a function, and there is a long comment in SyncRepWaitForLSN
describing the risks of this logic. e.g.
... If it's true, we need to check it again
* later while holding the lock, to check the flag and operate the sync
* rep queue atomically. This is necessary to avoid the race condition
* described in SyncRepUpdateSyncStandbysDefined().
This same function is now called from walsender.c. I think maybe it is
OK but please confirm it.
Anyway, the point is maybe this SyncRepEnabled function should be
better commented to make some reference about the race concerns of the
original comment. Otherwise some future caller of this function may be
unaware of it and come to grief.
---
Kind Regards,
Peter Smith.
Fujitsu Australia
On 25.02.22 06:36, Brar Piening wrote:
Yes, that would be possible. In fact appending a link and optionally
adding a tiny bit of CSS like I show below does the trick.
The major problem in that regard would probably be my lack of
XSLT/docbook skills but if no one can jump in here, I can see if I
On 24.02.22 16:17, Tom Lane wrote:
I think that having t/ directories contain only Perl test scripts
is a good convention that we should stick to. Peter's proposal
of a separate test/ subdirectory for C test scaffolding is
probably fine.
I wonder if there are any conventions in the Perl commun
On 24.02.22 16:00, Andres Freund wrote:
I've incidentally played with subtests yesterdays, when porting
src/interfaces/libpq/test/regress.pl to a tap test. Unfortunately it seems
that subtests aren't actually specified in the tap format, and that different
libraries generate different output form
s themselves
are similar. We want options and maximum flexibility, everywhere.
> but if you are going to rescan the heap
> again next time before doing any index vacuuming then why we want to
> store them anyway.
It all depends, of course. The decision needs to be made using a cost
model. I suspect it will be necessary to try it out, and see.
--
Peter Geoghegan
_FROZEN(vacrel->rel, blkno, &vmbuffer))
vacrel->frozenskipped_pages++;
continue;
}
The fact that this is conditioned in part on "vacrel->aggressive"
concerns me here. Why should we have a special case for this, where we
condition something on aggressive-ness that isn't actually strictly
related to that? Why not just remember that the range that we're
skipping was all-frozen up-front?
That way non-aggressive VACUUMs are not unnecessarily at a
disadvantage, when it comes to being able to advance relfrozenxid.
What if we end up not incrementing vacrel->frozenskipped_pages when we
easily could have, just because this is a non-aggressive VACUUM? I
think that it's worth avoiding stuff like that whenever possible.
Maybe this particular example isn't the most important one. For
example it probably isn't as bad as the one was fixed by the
lazy_scan_noprune work. But why even take a chance? Seems easier to
remove the special case -- which is what this really is.
> FWIW, I'd really like to get rid of SKIP_PAGES_THRESHOLD. It often ends up
> causing a lot of time doing IO that we never need, completely trashing all CPU
> caches, while not actually causing decent readaead IO from what I've seen.
I am also suspicious of SKIP_PAGES_THRESHOLD. But if we want to get
rid of it, we'll need to be sensitive to how that affects relfrozenxid
advancement in non-aggressive VACUUMs IMV.
Thanks again for the review!
--
Peter Geoghegan
On Fri, Feb 25, 2022 at 2:00 PM Peter Geoghegan wrote:
> > Hm. I guess I'll have to look at the code for it. It doesn't immediately
> > "feel" quite right.
>
> I kinda think it might be. Please let me know if you see a problem
> with what I've sai
is this:
in general, there are probably quite a few opportunities for
FreezeMultiXactId() to avoid allocating new XMIDs (just to freeze
XIDs) by having the full context. And maybe by making the dialog
between lazy_scan_prune and heap_prepare_freeze_tuple a bit more
nuanced.
--
Peter Geoghegan
> It might make sense to separate the purposes of SKIP_PAGES_THRESHOLD. The
> relfrozenxid advancement doesn't benefit from visiting all-frozen pages, just
> because there are only 30 of them in a row.
Right. I imagine that SKIP_PAGES_THRESHOLD actually does help with
this, but if we actually tried we'd find a much better way.
> I wish somebody would tackle merging heap_page_prune() with
> vacuuming. Primarily so we only do a single WAL record. But also because the
> separation has caused a *lot* of complexity. I've already more projects than
> I should, otherwise I'd start on it...
That has value, but it doesn't feel as urgent.
--
Peter Geoghegan
ats */
+} PgStat_MsgResetsubcounter;
BEFORE
InvalidOid for clearing all subscription stats
SUGGESTED
InvalidOid means reset all subscription stats
--
Kind Regards,
Peter Smith.
Fujitsu Australia
You set this commit fest entry to Waiting on Author, but there were no
reviews posted and the patch still applies and builds AFAICT, so I don't
know what you meant by that.
On 13.01.22 00:49, Bossart, Nathan wrote:
On 12/28/21, 8:25 AM, "Peter Eisentraut"
wrote:
(The "
y a bit more compact like this:
psql (15devel)
SSL connection (protocol: TLSv1.3, cipher: TLS_AES_256_GCM_SHA384)
Type "help" for help.
See attached patch.From 65042b40c874ff9f3877e5bbb1915321f5759b68 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Mon, 28 Feb 2022 09:54:16 +0100
S
On 28.02.22 09:41, Brar Piening wrote:
On Feb 25, 2022 at 14:31, Peter Eisentraut wrote:
I think that kind of stuff would be added in via the web site
stylesheets, so you wouldn't have to deal with XSLT at all.
True for the CSS but adding the HTML (#)
will need either XSLT or Javas
rebased patch, no functional changes
On 11.02.22 10:12, Peter Eisentraut wrote:
On 25.06.19 20:37, Andres Freund wrote:
I.e. I think it'd be better if we just added a fork argument to
fill_seq_with_data(), and then do something like
smgrcreate(srel, INIT_FORKNUM, false);
log_smgrcreate
On 27.02.22 10:42, Jille Timmermans wrote:
I wanted to be able to allocate a bunch of numbers from a sequence at
once. Multiple people seem to be struggling with this
(https://stackoverflow.com/questions/896274/select-multiple-ids-from-a-postgresql-sequence,
https://www.depesz.com/2008/03/20/ge
On 28.02.22 00:17, Jeff Davis wrote:
I also have in mind here that there has been discussion of giving
libpq a feature to refuse, on the client side, to send cleartext
passwords.
I am generally in favor of that idea, but I'm not sure that will
completely resolve the issue. For instance, should i
On 17.02.22 20:25, samay sharma wrote:
A use case where this is useful are environments where you want
authentication to be centrally managed across different services. This
is a common deployment model for cloud providers where customers like to
use single sign on and authenticate across diffe
On 28.02.22 11:50, Dagfinn Ilmari Mannsåker wrote:
Daniel Gustafsson writes:
On 28 Feb 2022, at 10:02, Peter Eisentraut
wrote:
Since support for SSL compression has been removed from PostgreSQL, it
doesn't seem sensible to display it anymore.
This was originally done, but all c
On 28.02.22 12:21, Michael Paquier wrote:
What about that making the information
shown version-aware? I would choose to show the compression part only
for server versions where it is settable.
That might lead to confusing results if you are not connecting to
something that is a stock PostgreS
On 25.02.22 14:06, Magnus Hagander wrote:
+OUT jit_generation_time float8,
+OUT jit_inlining_time float8,
+OUT jit_optimization_time float8,
+OUT jit_emission_time float8
Perhaps those should be of type interval?
On 25.02.22 17:26, Andres Freund wrote:
Ok that's good to know. What exactly happens when it tries to parse them?
Does it not count them or does it fail somehow? The way the output is
structured
Says that it can't pase a line of the tap output:
Ok, then I suppose I'm withdrawing this.
Perha
028_disable_on_error.pl - filename
The 028 number needs to be bumped because there is already a TAP test
called 028 now
~~~
9. src/test/subscription/t/028_disable_on_error.pl - missing test
There was no test case for the last combination where the user correct
the apply worker problem: E.g. After a prev
extra blank line can be removed.
~~~
20. Test for the column names.
The patch added a couple of new columns to statistics so I was
surprised there were no regression test updates needed for these? How
can that be? Shouldn’t there be just one regression test that
validates the view column names are what they are expected to be?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On 28.02.22 16:12, Tom Lane wrote:
Daniel Gustafsson writes:
On 28 Feb 2022, at 12:56, Peter Eisentraut
wrote:
On 28.02.22 11:50, Dagfinn Ilmari Mannsåker wrote:
How about making it show "compression: on" if compression is on, but
nothing in the common "off" case?
Th
On 25.02.22 21:19, Jacob Champion wrote:
On Fri, 2022-02-25 at 16:28 +, Jacob Champion wrote:
Ha, opr_sanity caught my use of cstring. I'll work on a fix later
today.
Fixed in v3.
This patch contains no documentation. I'm having a hard time
understanding what the name "session_authn_id
ssions are quite possible, and
a real concern -- but regressions *like that* are unlikely. Avoiding
doing what is clearly the wrong thing just seems to work out that way,
in general.
--
Peter Geoghegan
at the previously failing insert was applied OK.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
elation "public.test" in transaction 726 committed at LSN
0/14BFA88 and timestamp 2022-02-28 20:58:27.964238+09 from
replication origin "pg_16395"
After:
CONTEXT: processing remote data during "INSERT" for replication target
relation "public.test" (txid 726, LSN 0/14BFA88, ts 2022-02-28
20:58:27.964238+09, origin "pg_16395")
--
Kind Regards,
Peter Smith.
Fujitsu Australia.
7732b99559989ea3b615be78
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-PG-docs-Logical-Replication-Filtering.patch
Description: Binary data
On 01.03.22 23:05, Jacob Champion wrote:
On Tue, 2022-03-01 at 19:56 +0100, Peter Eisentraut wrote:
This patch contains no documentation. I'm having a hard time
understanding what the name "session_authn_id" is supposed to convey.
The comment for the Port.authn_id field s
On 01.03.22 22:17, Jonathan S. Katz wrote:
If you're moving to a newer version of PostgreSQL, you likely have to
update your connection drivers anyway (rebuilt against new libpq,
supporting any changes in the protocol, etc). I would prefer more data
to support that argument, but this is general
On 01.03.22 22:34, Andres Freund wrote:
The cases I've heard about are about centralizing auth across multiple cloud
services. Including secret management in some form. E.g. allowing an
application to auth to postgres, redis and having the secret provided by
infrastructure, rather than having to
On 01.03.22 20:50, Brar Piening wrote:
Patch is attached. I don't think it should get applied this way, though.
The fact that you only get links for section headers that have manually
assigned ids would be pretty surprising for users of the docs and in
some files (e.g. protocol-flow.html) only ev
On 01.03.22 18:27, Brar Piening wrote:
Also I'm not sure how well the autogenerated ids are reproducible over
time/versions/builds, and if it is wise to use them as targets to link
to from somewhere else.
Autogenerated ids are stable across builds of the same source. They
would change if the
On 02.03.22 05:47, Peter Smith wrote:
This patch introduces a new "Filtering" page to give a common place
where all kinds of logical replication filtering can be described.
(e.g. It is envisaged that a "Column Filters" section can be added
sometime in the future).
The pendi
On 02.03.22 15:16, Jonathan S. Katz wrote:
I find that a lot of people are still purposely using md5. Removing
it now or in a year would be quite a disruption.
What are the reasons they are still purposely using it? The ones I have
seen/heard are:
- Using an older driver
- On a pre-v10 PG
ersion number for future patch attachments?
--
KInd Regards,
Peter Smith.
Fujitsu Australia
On Wed, Mar 2, 2022 at 8:43 PM Aleksander Alekseev
wrote:
...
> Here is an updated version of the patch.
Thanks for your review comments and fixes. The updated v2 patch looks
good to me.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
, we can always extend the
> existing docs.
>
+1
--
Kind Regards,
Peter Smith.
Fujitsu Australia.
On Thu, Mar 3, 2022 at 2:15 PM Amit Kapila wrote:
>
> On Wed, Mar 2, 2022 at 8:00 PM Peter Eisentraut
> wrote:
> >
> > On 02.03.22 05:47, Peter Smith wrote:
> > > This patch introduces a new "Filtering" page to give a common place
> > > where
On 02.03.22 20:12, Jille Timmermans wrote:
On 2022-02-28 11:13, Peter Eisentraut wrote:
On 27.02.22 10:42, Jille Timmermans wrote:
I wanted to be able to allocate a bunch of numbers from a sequence at
once. Multiple people seem to be struggling with this
(https://stackoverflow.com/questions
On 02.03.22 21:26, Stephen Frost wrote:
Part of the point, for my part anyway, of dropping support for plaintext
transmission would be to remove support for that from libpq, otherwise a
compromised server could still potentially convince a client to provide
a plaintext password be sent to it.
I
On 02.03.22 16:45, Jonathan S. Katz wrote:
By that argument, we should have kept "password" (plain) as an
authentication method.
For comparison, the time between adding md5 and removing password was 16
years. It has been 5 years since scram was added.
On 02.03.22 15:16, Jonathan S. Katz wrote:
What are the reasons they are still purposely using it? The ones I have
seen/heard are:
- Using an older driver
- On a pre-v10 PG
- Unaware of SCRAM
Another reason is that SCRAM presents subtle operational issues in
distributed systems. As someone
On 02.03.22 21:49, samay sharma wrote:
I think we are discussing two topics in this thread which in my opinion
are orthogonal.
(a) Should we make authentication methods pluggable by exposing these
hooks? - These will allow users to add plugins of their own to support
whatever auth method they
PSA patch v3 to address all comments received so far.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v3-0001-Update-the-documentation-for-logical-replication.patch
Description: Binary data
On Thu, Mar 3, 2022 at 2:15 PM Amit Kapila wrote:
>
> On Wed, Mar 2, 2022 at 8:00 PM Peter Eisentraut
> wrote:
> >
> > On 02.03.22 05:47, Peter Smith wrote:
> > > This patch introduces a new "Filtering" page to give a common place
> > > where
ck if synchronous replication is enabled
+ */
+bool
+SyncRepEnabled(void)
Missing period for that function comment.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On 15.01.22 10:00, Fabien COELHO wrote:
The reason this test constantly fails on cfbot windows is a
use-after-free
bug.
Indeed! Thanks a lot for the catch and the debug!
The ClearOrSaveResult function is quite annoying because it may or may
not clear the result as a side effect.
Attached v
PSA patch to fix a comment typo.
(The 'OR' should not be uppercase - that keyword is irrelevant here).
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-Fix-comment-typo-CheckCmdReplicaIdentity.patch
Description: Binary data
e table synchronization failure in an idle state.
> + */
> +HOLD_INTERRUPTS();
> +EmitErrorReport();
> + FlushErrorState();
> +AbortOutOfAnyTransaction();
> +RESUME_INTERRUPTS();
> +pgstat_report_subscription_error(MySubscription->oid, false);
> +
> +if (MySubscription->disableonerr)
> +{
> +DisableSubscriptionOnError();
> +proc_exit(0);
> +}
> +
> +PG_RE_THROW();
>
> If the disableonerr is false, the same error is reported twice. Also,
> the code in PG_CATCH() in both start_apply() and start_table_sync()
> are almost the same. Can we create a common function to do post-error
> processing?
>
> The worker should exit with return code 1.
>
> I've attached a fixup patch for changes to worker.c for your
> reference. Feel free to adopt the changes.
The way that common function is implemented required removal of the
existing PG_RE_THROW logic, which in turn was only possible using
special knowledge that this just happens to be the last try/catch
block for the apply worker. Yes, I believe everything will work ok,
but it just seemed like a step too far for me to change the throw
logic. I feel that once you get to the point of having to write
special comments in the code to explain "why we can get away with
doing this..." then that is an indication that perhaps it's not really
the best way...
Is there some alternative way to share common code, but without having
to change the existing throw error logic to do so?
OTOH, maybe others think it is ok?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
current xid. The similar functions
> logicalmsg_decode() or heap_decode() do call ReorderBufferProcessXid
> even if they decide not to queue or send the change. Is there a reason
> for not doing the same here? However, I am not able to deduce any
> scenario where lack of this will lead to such an Assertion failure.
> Any thoughts?
>
--
[1]
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2022-03-03%2023%3A14%3A26
Kind Regards,
Peter Smith.
Fujitsu Australia
--
Kind Regards,
Peter Smith.
Fujitsu Australia.
why does the patch use syntax option 1?
--
Kind Regards,
Peter Smith
Fujitsu Australia
On Mon, Mar 7, 2022 at 3:56 PM Peter Smith wrote:
>
> Hi Vignesh, I also have not looked at the patch yet, but I have what
> seems like a very fundamental (and possibly dumb) question...
>
> Basically, I do not understand the choice of syntax for setting things up.
>
>
On Mon, Mar 7, 2022 at 4:20 PM vignesh C wrote:
>
> On Mon, Mar 7, 2022 at 10:26 AM Peter Smith wrote:
> >
> > Hi Vignesh, I also have not looked at the patch yet, but I have what
> > seems like a very fundamental (and possibly dumb) question...
> >
> > Basic
On Mon, Mar 7, 2022 at 5:12 PM kuroda.hay...@fujitsu.com
wrote:
>
> Dear Peter,
>
> > > So, why does the patch use syntax option 1?
>
> IMU it might be useful for the following case.
>
> Assuming that multi-master configuration with node1, node2.
> Node1 has a pu
On Mon, Mar 7, 2022 at 6:17 PM vignesh C wrote:
>
> On Mon, Mar 7, 2022 at 11:45 AM Peter Smith wrote:
> >
> > On Mon, Mar 7, 2022 at 4:20 PM vignesh C wrote:
> > >
> > > On Mon, Mar 7, 2022 at 10:26 AM Peter Smith wrote:
> > > >
> > > &
comment.
~~~
15. src/test/regress/sql/subscription.sql
ALTER SUBSCRIPTION test missing?
--
[1]
https://www.postgresql.org/message-id/CAHut%2BPsAWaETh9VMymbBfMrqiE1KuqMq%2BwpBg0s7eMzwLATr%2Bw%40mail.gmail.com
[2]
https://www.postgresql.org/message-id/CAHut%2BPvQonJd5epJBM0Yfh1499mL9kTL9a%3DGrMhvnL6Ok05zqw%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
Is there a current patch set to review in this thread at the moment?
d/CAHut%2BPucrizJpqhSyD7dKj1yRkNMskqmiekD_RRqYpdDdusMRQ%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
nd up with an option that
turned out to be inconsistent looking on the subscriber-side /
publisher-side.
- we should try to avoid accidentally painting ourselves into a corner
(e.g. stuck with a boolean option that cannot be enhanced later on)
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Tue, Mar 8, 2022 at 4:31 PM Michael Paquier wrote:
>
> On Mon, Mar 07, 2022 at 10:28:08AM +0800, Julien Rouhaud wrote:
> > +1
>
> And done.
> --
> Michael
Thanks!
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Tue, Mar 8, 2022 at 4:21 PM Amit Kapila wrote:
>
> On Tue, Mar 8, 2022 at 10:31 AM Peter Smith wrote:
> >
> > IIUC the new option may be implemented subscriber-side and/or
> > publisher-side and/or both, and the subscriber-side option may be
> > "enhance
latest understanding was that the DDL-time checking would be removed. I
think it's basically useless now, since as the test cases show, you can
subvert those checks by altering the replica identity later.From d0e9df4674389cda9f891f5678f476d35095c618 Mon Sep 17 00:00:00 2001
From: Peter
introduced in
commit 2bd78eb8d51cc9ee03ba0287b23ff4c266dcd9b9
Author: Peter Eisentraut
Date: 2011-04-06 22:36:06 +0300
Add traceback information to PL/Python errors
We would probably try to write this test differently today, but at this
point I wouldn't bother and just wait for Pytho
On 28.02.22 19:51, Nathan Bossart wrote:
On Mon, Feb 28, 2022 at 07:46:40AM +0100, Peter Eisentraut wrote:
You set this commit fest entry to Waiting on Author, but there were no
reviews posted and the patch still applies and builds AFAICT, so I don't
know what you meant by that.
Apologie
1 - 100 of 10718 matches
Mail list logo
