Обсуждение: Re: BUG: deadlock between autovacuum worker and client backend during removal of orphan temp tables with sequences
Hii, I am currently trying to review the submitted patch but I am not able to apply it to the master branch. Regards, Akshat Jaimini
Re: BUG: deadlock between autovacuum worker and client backend during removal of orphan temp tables with sequences
От
"Andrey M. Borodin"
Дата:
> On 28 Mar 2024, at 10:51, Akshat Jaimini <destrex271@gmail.com> wrote: > > I am currently trying to review the submitted patch Great, thank you! > but I am not able to apply it to the master branch. Please find attached rebased version on current HEAD. For some reason CFbot did not notify about that rebases is needed. Best regards, Andrey Borodin.
Вложения
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, passed Spec compliant: tested, passed Documentation: tested, passed Hii, Thanks for the updated patch. I ran make installcheck-world after applying the patch and recompiling it. It did fail fora particular test but from the logs it seems to be unrelated to this particular patch since it fails for the following: ========================== select error_trap_test(); - error_trap_test ---------------------------- - division_by_zero detected -(1 row) - +ERROR: cannot start subtransactions during a parallel operation +CONTEXT: PL/pgSQL function error_trap_test() line 2 during statement block entry +parallel worker reset debug_parallel_query; drop function error_trap_test(); drop function zero_divide(); ========================== The code seems to implement the feature and has good and explanatory comments associated with it. I believe we can go ahead with committing patch although I would request some senior contributors to also take a look atthis patch since I am relatively new to patch reviews. Changing the status to 'Ready for Committer'. Regards, Akshat Jaimini The new status of this patch is: Ready for Committer
Akshat Jaimini <destrex271@gmail.com> writes:
> The code seems to implement the feature and has good and explanatory comments associated with it.
> I believe we can go ahead with committing patch although I would request some senior contributors to also take a look
atthis patch since I am relatively new to patch reviews.
Looks like a good catch and a reasonable fix. Pushed after rewriting
the comments a bit.
As far as this goes:
> I ran make installcheck-world after applying the patch and recompiling it. It did fail for a particular test but from
thelogs it seems to be unrelated to this particular patch since it fails for the following:
> ==========================
> select error_trap_test();
> - error_trap_test
> ----------------------------
> - division_by_zero detected
> -(1 row)
> -
> +ERROR: cannot start subtransactions during a parallel operation
... that's the test case from 0075d7894, and the failure is what
I'd expect from a backend older than that. Maybe you forgot to
recompile/reinstall after updating past that commit?
regards, tom lane
Thanks to all for review, testing and commit!!!
On 2 April 2024 22:04:54 GMT+03:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Akshat Jaimini <destrex271@gmail.com> writes:The code seems to implement the feature and has good and explanatory comments associated with it.
I believe we can go ahead with committing patch although I would request some senior contributors to also take a look at this patch since I am relatively new to patch reviews.
Looks like a good catch and a reasonable fix. Pushed after rewriting
the comments a bit.
As far as this goes:I ran make installcheck-world after applying the patch and recompiling it. It did fail for a particular test but from the logs it seems to be unrelated to this particular patch since it fails for the following:
select error_trap_test();
- error_trap_test
- division_by_zero detected
-(1 row)
-
+ERROR: cannot start subtransactions during a parallel operation
... that's the test case from 0075d7894, and the failure is what
I'd expect from a backend older than that. Maybe you forgot to
recompile/reinstall after updating past that commit?
regards, tom lane
Hi apologies for the late reply.
> Maybe you forgot to recompile/reinstall after updating past that commit?
I did recompile it earlier but just to be sure I followed the steps again and now its working!
Regards,
Akshat Jaimini
On Wed, Apr 3, 2024 at 12:34 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Akshat Jaimini <destrex271@gmail.com> writes:
> The code seems to implement the feature and has good and explanatory comments associated with it.
> I believe we can go ahead with committing patch although I would request some senior contributors to also take a look at this patch since I am relatively new to patch reviews.
Looks like a good catch and a reasonable fix. Pushed after rewriting
the comments a bit.
As far as this goes:
> I ran make installcheck-world after applying the patch and recompiling it. It did fail for a particular test but from the logs it seems to be unrelated to this particular patch since it fails for the following:
> ==========================
> select error_trap_test();
> - error_trap_test
> ----------------------------
> - division_by_zero detected
> -(1 row)
> -
> +ERROR: cannot start subtransactions during a parallel operation
... that's the test case from 0075d7894, and the failure is what
I'd expect from a backend older than that. Maybe you forgot to
recompile/reinstall after updating past that commit?
regards, tom lane