Re: BUG: deadlock between autovacuum worker and client backend during removal of orphan temp tables with sequences

2024-04-04 Thread Akshat Jaimini
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  wrote:

> Akshat Jaimini  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
>


Re: BUG: deadlock between autovacuum worker and client backend during removal of orphan temp tables with sequences

2024-04-02 Thread Michael Zhilin
Thanks to all for review, testing and commit!!! 


On 2 April 2024 22:04:54 GMT+03:00, Tom Lane  wrote:
>Akshat Jaimini  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


Re: BUG: deadlock between autovacuum worker and client backend during removal of orphan temp tables with sequences

2024-04-02 Thread Tom Lane
Akshat Jaimini  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




Re: BUG: deadlock between autovacuum worker and client backend during removal of orphan temp tables with sequences

2024-03-29 Thread Akshat Jaimini
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 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
+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 at this 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


Re: BUG: deadlock between autovacuum worker and client backend during removal of orphan temp tables with sequences

2024-03-27 Thread Andrey M. Borodin


> On 28 Mar 2024, at 10:51, Akshat Jaimini  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.


v2-0001-Avoid-deadlock-and-concurrency-during-orphan-temp.patch
Description: Binary data


Re: BUG: deadlock between autovacuum worker and client backend during removal of orphan temp tables with sequences

2024-03-27 Thread Akshat Jaimini
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