RE: Non-superuser subscription owners - Mailing list pgsql-hackers

On Friday, March 31, 2023 12:05 AM Robert Haas <robertmhaas@gmail.com> wrote:

Hi,

> 
> On Tue, Mar 28, 2023 at 1:52 PM Jeff Davis <pgsql@j-davis.com> wrote:
> > On Fri, 2023-03-24 at 00:17 -0700, Jeff Davis wrote:
> > > The other patch you posted seems like it makes a lot of progress in
> > > that direction, and I think that should go in first. That was one of
> > > the items I suggested previously[2], so thank you for working on
> > > that.
> >
> > The above is not a hard objection.
> 
> The other patch is starting to go in a direction that is going to have some
> conflicts with this one, so I went ahead and committed this one to avoid
> rebasing pain.

I noticed the BF[1] report a core dump after this commit.

#0  0xfd581864 in _lwp_kill () from /usr/lib/libc.so.12
#0  0xfd581864 in _lwp_kill () from /usr/lib/libc.so.12
#1  0xfd5817dc in raise () from /usr/lib/libc.so.12
#2  0xfd581c88 in abort () from /usr/lib/libc.so.12
#3  0x01e6c8d4 in ExceptionalCondition (conditionName=conditionName@entry=0x2007758 "IsTransactionState()",
fileName=fileName@entry=0x20565c4"catcache.c", lineNumber=lineNumber@entry=1208) at assert.c:66
 
#4  0x01e4e404 in SearchCatCacheInternal (cache=0xfd21e500, nkeys=nkeys@entry=1, v1=v1@entry=28985, v2=v2@entry=0,
v3=v3@entry=0,v4=v4@entry=0) at catcache.c:1208
 
#5  0x01e4eea0 in SearchCatCache1 (cache=<optimized out>, v1=v1@entry=28985) at catcache.c:1162
#6  0x01e66e34 in SearchSysCache1 (cacheId=cacheId@entry=11, key1=key1@entry=28985) at syscache.c:825
#7  0x01e98c40 in superuser_arg (roleid=28985) at superuser.c:70
#8  0x01c657bc in ApplyWorkerMain (main_arg=<optimized out>) at worker.c:4552
#9  0x01c1ceac in StartBackgroundWorker () at bgworker.c:861
#10 0x01c23be0 in do_start_bgworker (rw=<optimized out>) at postmaster.c:5762
#11 maybe_start_bgworkers () at postmaster.c:5986
#12 0x01c2459c in process_pm_pmsignal () at postmaster.c:5149
#13 ServerLoop () at postmaster.c:1770
#14 0x01c26cdc in PostmasterMain (argc=argc@entry=4, argv=argv@entry=0xffffe0e4) at postmaster.c:1463
#15 0x01ee2c8c in main (argc=4, argv=0xffffe0e4) at main.c:200

It looks like the super user check is out of a transaction, I haven't checked why
it only failed on one BF animal, but it seems we can put the check into the
transaction like the following:

diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 6fd674b5d6..08f10fc331 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4545,12 +4545,13 @@ ApplyWorkerMain(Datum main_arg)
         replorigin_session_setup(originid, 0);
         replorigin_session_origin = originid;
         origin_startpos = replorigin_session_get_progress(false);
-        CommitTransactionCommand();
 
         /* Is the use of a password mandatory? */
         must_use_password = MySubscription->passwordrequired &&
             !superuser_arg(MySubscription->owner);
 
+        CommitTransactionCommand();
+

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mamba&dt=2023-03-30%2019%3A41%3A08

Best Regards,
Hou Zhijie