pgsql: Use locale-aware value for \watch in 005_timeouts.pl
Use locale-aware value for \watch in 005_timeouts.pl Reported-by: Alexander Lakhin Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/605062227fdc8331f2ffa0f60e7a8724b6e56480 Modified Files -- src/test/modules/test_misc/t/005_timeouts.pl | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-)
pgsql: Fix handling of expecteddir in pg_regress
Fix handling of expecteddir in pg_regress Commit c855872074b introduced a new parameter to pg_regress to set the directory where to look for expected files, but accidentally only implemented it for when compiling pg_regress for ECPG tests. Fix by adding support for the parameter to the main regression test compilation of pg_regress as well. Backpatch to v16 where --expecteddir was introduced. Author: Anthonin Bonnefoy Discussion: https://postgr.es/m/CAO6_Xqq5yKJHcJsq__LPcKwSY0XHRqVERNWGxx5ttNXXj7+W=a...@mail.gmail.com Backpatch-through: 16 Branch -- REL_16_STABLE Details --- https://git.postgresql.org/pg/commitdiff/5643262736b5cdd6fa8e56545d8ac5da281f Modified Files -- src/test/regress/pg_regress_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Fix handling of expecteddir in pg_regress
Fix handling of expecteddir in pg_regress Commit c855872074b introduced a new parameter to pg_regress to set the directory where to look for expected files, but accidentally only implemented it for when compiling pg_regress for ECPG tests. Fix by adding support for the parameter to the main regression test compilation of pg_regress as well. Backpatch to v16 where --expecteddir was introduced. Author: Anthonin Bonnefoy Discussion: https://postgr.es/m/CAO6_Xqq5yKJHcJsq__LPcKwSY0XHRqVERNWGxx5ttNXXj7+W=a...@mail.gmail.com Backpatch-through: 16 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/196eeb6b2f7cf5f95293259efe0e5d73e8d0025d Modified Files -- src/test/regress/pg_regress_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Fix backstop in gin test if injection point is not reached
Fix backstop in gin test if injection point is not reached Per Tom Lane's observation that the test got stuck in infinite loop if the injection_points module was not loaded. It was supposed to give up after 1 iterations, but the backstop was broken. Discussion: https://www.postgresql.org/message-id/2498595.1710511222%40sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d802ff06d0216d59aab3ddba6cb158d1f4b014db Modified Files -- src/test/modules/gin/expected/gin_incomplete_splits.out | 5 +++-- src/test/modules/gin/sql/gin_incomplete_splits.sql | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-)
pgsql: Try to unbreak injection-fault tests in the buildfarm
Try to unbreak injection-fault tests in the buildfarm The buildfarm script attempts to run all tests marked as NO_INSTALLCHECK under src/test/modules without paying attention to whether they are enabled or disabled in the parent Makefile. That hasn't been a problem so far, because all the tests marked with NO_INSTALLCHECK ran unconditionally in "make check". But commit e2e3b8ae9e changed that: the injection fault tests are marked as NO_INSTALLCHECK, and also depend on --enable-injection-points. Try to work around that by ensuring that "make check" does nothing in the those subdirectories. We can hopefully get rid of this hack soon, after fixing the buildfarm client, or by switching to meson. Discussion: https://www.postgresql.org/message-id/8e4cf596-dd70-432e-9068-16466ed596ed%40iki.fi Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/85f65d7a26fc24ad9711aa5f1ca7e20ea3ea77e6 Modified Files -- src/test/modules/gin/Makefile | 12 src/test/modules/injection_points/Makefile | 12 2 files changed, 24 insertions(+)
Re: pgsql: Add TAP tests for timeouts
On Fri, Mar 15, 2024 at 2:25 PM Andrey M. Borodin wrote: > > On 15 Mar 2024, at 16:30, Alexander Korotkov wrote: > > > > Maybe, but do you see any negative side effects of the unconditionally > > unset of flags? > > Nope, just expressed possible option. > > > If not, I would prefer to keep the code simple. > > IMO that's fine. Let's incorporate wording improvement from Kyotaro to the > same patch? PFA draft for this. Thank you, I've pushed this, but split into two commits for bugfix and for improved wordings. > Maybe it worth to wait a little to see if some other failures will pop up? I prefer to push now because we're quite confident this fix will improve the situation. It should increase the chances of catching other bugs if any, because buildfarm which was failing on this bug will go ahead and have a chance to spot more. -- Regards, Alexander Korotkov
pgsql: Fix wordings in timeouts TAP test
Fix wordings in timeouts TAP test Reported-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/20240315.104235.1835366724413653745.horikyota.ntt%40gmail.com Author: Andrey Borodin Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/7a65cc079e717c5d87f1920929affe7138c31eee Modified Files -- src/test/modules/test_misc/t/005_timeouts.pl | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-)
pgsql: Fix race condition in transaction timeout TAP tests
Fix race condition in transaction timeout TAP tests The interruption handler within the injection point can get stuck in an infinite loop while handling transaction timeout. To avoid this situation we reset the timeout flag before invoking the injection point. Author: Alexander Korotkov Reviewed-by: Andrey Borodin Discussion: https://postgr.es/m/ZfPchPC6oNN71X2J%40paquier.xyz Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/4c2eda67f55a8a263820d12aaeaa7e1dfe7406ee Modified Files -- src/backend/tcop/postgres.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-)
Re: pgsql: Add TAP tests for timeouts
> On 15 Mar 2024, at 16:30, Alexander Korotkov wrote: > > Maybe, but do you see any negative side effects of the unconditionally > unset of flags? Nope, just expressed possible option. > If not, I would prefer to keep the code simple. IMO that's fine. Let's incorporate wording improvement from Kyotaro to the same patch? PFA draft for this. Maybe it worth to wait a little to see if some other failures will pop up? Best regards, Andrey Borodin. v2-0001-Fix-race-condition-in-transaction-timeout-TAP-tes.patch Description: Binary data
Re: pgsql: Add TAP tests for timeouts
On Fri, Mar 15, 2024 at 1:27 PM Andrey M. Borodin wrote: > > > On 15 Mar 2024, at 15:44, Alexander Korotkov wrote: > > > > We loop in the interrupt checking, given that the injection point handler > > checks for interrupts internally. I propose to unset the timeout flag > > before the injection point (see the attached patch). > > Oh, cool. > As far as I understand, this is only necessary for the test with injection > point. > So, maybe unset it only when injection points are enabled? Something like > this is already used in GIN. > > #ifdef USE_INJECTION_POINTS > > #endif Maybe, but do you see any negative side effects of the unconditionally unset of flags? If not, I would prefer to keep the code simple. -- Regards, Alexander Korotkov
Re: pgsql: Add TAP tests for timeouts
> On 15 Mar 2024, at 15:44, Alexander Korotkov wrote: > > We loop in the interrupt checking, given that the injection point handler > checks for interrupts internally. I propose to unset the timeout flag before > the injection point (see the attached patch). Oh, cool. As far as I understand, this is only necessary for the test with injection point. So, maybe unset it only when injection points are enabled? Something like this is already used in GIN. #ifdef USE_INJECTION_POINTS #endif Best regards, Andrey Borodin.
pgsql: Improve log messages referring to background worker processes
Improve log messages referring to background worker processes "Worker" could also mean autovacuum worker or slot sync worker, so let's be more explicit. Per Tristan Partin's suggestion. Discussion: https://www.postgresql.org/message-id/czm6wdx5h4qi.nzg1yuck...@neon.tech Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/a3f349c612923b53bd168b1dfb541a4ec712b421 Modified Files -- src/backend/postmaster/postmaster.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Disable tests using injection points in installcheck
Disable tests using injection points in installcheck The 'gin' test injections faults to GIN index build. If another test running concurrently in the same cluster also tries to create a GIN index, it will hit the fault, too. To fix, disable tests using injection points when running against an existing cluster. A better long-term solution would be to make the injection points scoped to the database or process, but this will do for now. Discussion: https://www.postgresql.org/message-id/ca%2bhukgjyhcg_o2nwsk6r01eozjwnwujubx%3d%3davnw84f-%2b8y...@mail.gmail.com Discussion: https://www.postgresql.org/message-id/10fd6cdd-c5d9-46fe-9fa1-7e6611913...@iki.fi Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/e2e3b8ae9ed73fcd3096c5ca93971891a7767388 Modified Files -- src/test/modules/gin/Makefile | 3 +++ src/test/modules/gin/meson.build | 2 ++ src/test/modules/injection_points/Makefile| 3 +++ src/test/modules/injection_points/meson.build | 2 ++ 4 files changed, 10 insertions(+)
Re: pgsql: Add TAP tests for timeouts
On Fri, Mar 15, 2024 at 9:44 AM Michael Paquier wrote: > On Fri, Mar 15, 2024 at 11:20:49AM +0500, Andrey M. Borodin wrote: > > And hachi sometimes pass this test too [0]. > > > > I’ll look more on this. Do I understand right that we have only 2 buildfarm members with injection points? > > I have added --enable-injection-points to three of them: hachi, > gokiburi and batta. The switch is also enabled in the CI by defaultm > where you may be able to capture a bit more information than the > buildfarm. I managed to reproduce the issue locally by setting the following extra config via TEMP_CONFIG: log_line_prefix = '%m [%p:%l] %q%a ' log_connections = 'true' log_disconnections = 'true' log_checkpoints = 'true' log_statement = 'all' wal_compression = 'on' debug_parallel_query = regress wal_compression = lz4 default_toast_compression = lz4 The backtrace is as follows. We loop in the interrupt checking, given that the injection point handler checks for interrupts internally. I propose to unset the timeout flag before the injection point (see the attached patch). * thread #1, name = 'postgres', stop reason = signal SIGSTOP * frame #0: 0x7f3756f289e7 libc.so.6`epoll_wait(epfd=9, events=0x556a302cabc8, maxevents=1, timeout=-1) at epoll_wait.c:30:10 frame #1: 0x556a2fc41aab postgres`WaitEventSetWait at latch.c:1570:7 frame #2: 0x556a2fc41e95 postgres`WaitLatch(latch=, wakeEvents=, timeout=, wait_event_info=117440513) at latch.c:538:6 frame #3: 0x556a2fc4f611 postgres`ConditionVariableTimedSleep(cv=0x7f3757b0e224, timeout=-1, wait_event_info=117440513) at condition_variable.c:163:10 frame #4: 0x7f3757b10628 injection_points.so`injection_wait at injection_points.c:153:3 frame #5: 0x556a2fdd4f54 postgres`InjectionPointRun at injection_point.c:313:2 frame #6: 0x556a2fc6a8b3 postgres`ProcessInterrupts.part.0 at postgres.c:3430:4 frame #7: 0x556a2fc4f70b postgres`ConditionVariableTimedSleep(cv=0x7f3757b0e224, timeout=-1, wait_event_info=117440513) at condition_variable.c:196:3 frame #8: 0x7f3757b10628 injection_points.so`injection_wait at injection_points.c:153:3 frame #9: 0x556a2fdd4f54 postgres`InjectionPointRun at injection_point.c:313:2 frame #10: 0x556a2fc6a8b3 postgres`ProcessInterrupts.part.0 at postgres.c:3430:4 frame #11: 0x556a2fc4f70b postgres`ConditionVariableTimedSleep(cv=0x7f3757b0e224, timeout=-1, wait_event_info=117440513) at condition_variable.c:196:3 frame #12: 0x7f3757b10628 injection_points.so`injection_wait at injection_points.c:153:3 frame #13: 0x556a2fdd4f54 postgres`InjectionPointRun at injection_point.c:313:2 frame #14: 0x556a2fc6a8b3 postgres`ProcessInterrupts.part.0 at postgres.c:3430:4 frame #15: 0x556a2fabe63d postgres`ExecGather at nodeGather.c:315:3 frame #16: 0x556a2fabe574 postgres`ExecGather at nodeGather.c:269:10 frame #17: 0x556a2fabe549 postgres`ExecGather(pstate=0x556a303d8138) at nodeGather.c:222:9 frame #18: 0x556a2faa21da postgres`standard_ExecutorRun at executor.h:274:9 frame #19: 0x556a2fc6f3cf postgres`PortalRunSelect at pquery.c:924:4 frame #20: 0x556a2fc70b7e postgres`PortalRun at pquery.c:768:18 frame #21: 0x556a2fc6c996 postgres`exec_simple_query at postgres.c:1274:10 frame #22: 0x556a2fc6e378 postgres`PostgresMain at postgres.c:4682:7 frame #23: 0x556a2fbcefa0 postgres`ServerLoop.isra.0 at postmaster.c:4452:2 frame #24: 0x556a2fbcff8b postgres`PostmasterMain at postmaster.c:1478:11 frame #25: 0x556a2f8cb5a7 postgres`main(argc=4, argv=0x556a302ca340) at main.c:197:3 frame #26: 0x7f3756e28150 libc.so.6`__libc_start_call_main(main=(postgres`main at main.c:59:1), argc=4, argv=0x7c17e678) at libc_start_call_main.h:58:16 frame #27: 0x7f3756e28209 libc.so.6`__libc_start_main_impl(main=(postgres`main at main.c:59:1), argc=4, argv=0x7c17e678, init=, fini=, rtld_fini=, stack_end=0x7c17e668) at libc-start.c:360:3 frame #28: 0x556a2f8cbb75 postgres`_start + 37 -- Regards, Alexander Korotkov transaction_timeout_patch_fix.patch Description: Binary data
Re: pgsql: Add TAP tests for timeouts
On Fri, Mar 15, 2024 at 11:20:49AM +0500, Andrey M. Borodin wrote: > And hachi sometimes pass this test too [0]. > > I’ll look more on this. Do I understand right that we have only 2 buildfarm > members with injection points? I have added --enable-injection-points to three of them: hachi, gokiburi and batta. The switch is also enabled in the CI by defaultm where you may be able to capture a bit more information than the buildfarm. -- Michael signature.asc Description: PGP signature
