RE: Non-superuser subscription owners

On Tuesday, April 4, 2023 1:57 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Sat, Apr 1, 2023 at 12:00 PM Alexander Lakhin <exclusion(at)gmail(dot)com>
> wrote:
> > I've managed to reproduce it using the following script:
> > for ((i=1;i<=10;i++)); do
> > echo "iteration $i"
> > echo "
> > CREATE ROLE sub_user;
> > CREATE SUBSCRIPTION testsub CONNECTION 'dbname=db'
> > PUBLICATION testpub WITH (connect = false); ALTER SUBSCRIPTION
> > testsub ENABLE; DROP SUBSCRIPTION testsub; SELECT pg_sleep(0.001);
> > DROP ROLE sub_user; " | psql psql -c "ALTER SUBSCRIPTION testsub
> > DISABLE;"
> > psql -c "ALTER SUBSCRIPTION testsub SET (slot_name = NONE);"
> > psql -c "DROP SUBSCRIPTION testsub;"
> > grep 'TRAP' server.log && break
> > done
>
> After a bit of experimentation this repro worked for me -- I needed
> -DRELCACHE_FORCE_RELEASE as well, and a bigger iteration count. I verified
> that the patch fixed it, and committed the patch with the addition of a
> comment.

Thanks for pushing!

While testing this, I found a similar problem in table sync worker,
as we also invoke superuser_arg() in table sync worker which is not in a
transaction.

LogicalRepSyncTableStart
...
/* Is the use of a password mandatory? */
must_use_password = MySubscription->passwordrequired &&
!superuser_arg(MySubscription->owner);

#0  0x00007f18bb55aaff in raise () from /lib64/libc.so.6
#1  0x00007f18bb52dea5 in abort () from /lib64/libc.so.6
#2  0x0000000000b69a22 in ExceptionalCondition (conditionName=0xda4338 "IsTransactionState()", fileName=0xda403e "catcache.c", lineNumber=1208) at assert.c:66
#3  0x0000000000b4842a in SearchCatCacheInternal (cache=0x27cab80, nkeys=1, v1=10, v2=0, v3=0, v4=0) at catcache.c:1208
#4  0x0000000000b48329 in SearchCatCache1 (cache=0x27cab80, v1=10) at catcache.c:1162
#5  0x0000000000b630c7 in SearchSysCache1 (cacheId=11, key1=10) at syscache.c:825
#6  0x0000000000b982e3 in superuser_arg (roleid=10) at superuser.c:70

I can reproduce this via gdb following similar steps in [1].

I think we need to move this call into a transaction as well and here is an attempt
to do that.

[1] https://www.postgresql.org/message-id/OS0PR01MB5716E596E4FB83DE46F592FE948C9%40OS0PR01MB5716.jpnprd01.prod.outlook.com

Best Regards,
Hou zj