Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Kuntal Ghosh]

On Fri, Jun 23, 2017 at 3:01 AM, Thomas Munro
 wrote:
> On Thu, Jun 22, 2017 at 4:29 AM, Kuntal Ghosh
>  wrote:
>> On Wed, Jun 21, 2017 at 7:52 PM, Dilip Kumar  wrote:
>>> On Wed, Jun 21, 2017 at 7:47 PM, Kuntal Ghosh
>>>  wrote:
> IMHO, It's not a good idea to use DSM call to verify the DSA handle.
>
 Okay. Is there any particular scenario you've in mind where this may fail?
>>>
>>> It's not about failure, but about the abstraction.  When we are using
>>> the DSA we should not directly access the DSM which is under DSA.
>>>
>> Okay. I thought that I've found at least one usage of
>> dsm_find_mapping() in the code. :-)
>>
>> But, I've some more doubts.
>> 1. When should we use dsm_find_mapping()? (The first few lines of
>> dsm_attach is same as dsm_find_mapping().)
>> 2. As a user of dsa, how should we check whether my dsa handle is
>> already attached? I guess this is required because, if a user tries to
>> re-attach a dsa handle,  it's punishing the user by throwing an error
>> and the user wants to avoid such errors.
>
> I thought about this when designing the DSA API.  I couldn't think of
> any good reason to provide an 'am-I-already-attached?' function
> equivalent to dsm_find_mapping.  It seemed to me that the client code
> shouldn't ever be in any doubt about whether it's attached, and that
> wilfully or absent-mindedly throwing away dsa_area pointers and having
> to ask for them again doesn't seem like a very good design.  I suspect
> the same applies to dsm_find_mapping, and I don't see any callers in
> the source tree or indeed anywhere on the internet (based on a quick
> Google search).  But I could be missing something.
>
Thanks a lot for the clarification.


-- 
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Yugo Nagata]

On Thu, 22 Jun 2017 13:55:26 -0400
Alvaro Herrera  wrote:

> Yugo Nagata wrote:
> > Hi,
> > 
> > As I report in another thread[1], I found the autovacuum launcher occurs
> > the following error in PG 10 when this received SIGINT. I can repuroduce
> > this by pg_cancel_backend or `kill -2 `.
> 
> Thanks for the report, BTW!

Thank you for fixing it!

> 
> -- 
> Álvaro Herrerahttps://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Yugo Nagata 


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Michael Paquier]

On Fri, Jun 23, 2017 at 7:02 AM, Alvaro Herrera
 wrote:
> Thomas Munro wrote:
>> I thought about this when designing the DSA API.  I couldn't think of
>> any good reason to provide an 'am-I-already-attached?' function
>> equivalent to dsm_find_mapping.  It seemed to me that the client code
>> shouldn't ever be in any doubt about whether it's attached, and that
>> wilfully or absent-mindedly throwing away dsa_area pointers and having
>> to ask for them again doesn't seem like a very good design.  I suspect
>> the same applies to dsm_find_mapping, and I don't see any callers in
>> the source tree or indeed anywhere on the internet (based on a quick
>> Google search).  But I could be missing something.

I don't think that's completely exact. dsm_attach() uses at its
duplicates dsm_find_mapping() to see if a segment is already attached.
So dsm_attach could be refactored to directly use dsm_find_mapping().

> I think such an API call is potentially useful for things like assertion
> checks, if nothing else.

Indeed, that's useful here as well.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Alvaro Herrera]

Thomas Munro wrote:

> I thought about this when designing the DSA API.  I couldn't think of
> any good reason to provide an 'am-I-already-attached?' function
> equivalent to dsm_find_mapping.  It seemed to me that the client code
> shouldn't ever be in any doubt about whether it's attached, and that
> wilfully or absent-mindedly throwing away dsa_area pointers and having
> to ask for them again doesn't seem like a very good design.  I suspect
> the same applies to dsm_find_mapping, and I don't see any callers in
> the source tree or indeed anywhere on the internet (based on a quick
> Google search).  But I could be missing something.

I think such an API call is potentially useful for things like assertion
checks, if nothing else.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Thomas Munro]

On Thu, Jun 22, 2017 at 4:29 AM, Kuntal Ghosh
 wrote:
> On Wed, Jun 21, 2017 at 7:52 PM, Dilip Kumar  wrote:
>> On Wed, Jun 21, 2017 at 7:47 PM, Kuntal Ghosh
>>  wrote:
 IMHO, It's not a good idea to use DSM call to verify the DSA handle.

>>> Okay. Is there any particular scenario you've in mind where this may fail?
>>
>> It's not about failure, but about the abstraction.  When we are using
>> the DSA we should not directly access the DSM which is under DSA.
>>
> Okay. I thought that I've found at least one usage of
> dsm_find_mapping() in the code. :-)
>
> But, I've some more doubts.
> 1. When should we use dsm_find_mapping()? (The first few lines of
> dsm_attach is same as dsm_find_mapping().)
> 2. As a user of dsa, how should we check whether my dsa handle is
> already attached? I guess this is required because, if a user tries to
> re-attach a dsa handle,  it's punishing the user by throwing an error
> and the user wants to avoid such errors.

I thought about this when designing the DSA API.  I couldn't think of
any good reason to provide an 'am-I-already-attached?' function
equivalent to dsm_find_mapping.  It seemed to me that the client code
shouldn't ever be in any doubt about whether it's attached, and that
wilfully or absent-mindedly throwing away dsa_area pointers and having
to ask for them again doesn't seem like a very good design.  I suspect
the same applies to dsm_find_mapping, and I don't see any callers in
the source tree or indeed anywhere on the internet (based on a quick
Google search).  But I could be missing something.

-- 
Thomas Munro
http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Alvaro Herrera]

Yugo Nagata wrote:
> Hi,
> 
> As I report in another thread[1], I found the autovacuum launcher occurs
> the following error in PG 10 when this received SIGINT. I can repuroduce
> this by pg_cancel_backend or `kill -2 `.

Thanks for the report, BTW!

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Alvaro Herrera]

Thomas Munro wrote:

> Hmm.  So the problem here is that AutoVacLauncherMain assumes that
> there are only two possibilities: (1) there is no handle published in
> shmem yet, so we should create a DSA area and publish the handle, and
> (2) there is a handle published in shmem so we should attach to it.
> But there is a another possiblity: (3) there is a handle published in
> shmem, but we are already attached to it (because we've restarted our
> main loop after SIGINT).
> 
> The suggestion so far was to check if we're already attached to that
> segment (making use of the implementation detail that a DSA handle is
> also a DSM segment handle), but don't we know if we're already
> attached by checking our AutoVacuumDSA variable?

This seems like a good fix to me.  Pushed this way, with an additional
comment.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Thomas Munro]

On Thu, Jun 22, 2017 at 6:10 PM, Michael Paquier
 wrote:
> On Thu, Jun 22, 2017 at 2:44 PM, Kuntal Ghosh
>  wrote:
>> On Thu, Jun 22, 2017 at 9:48 AM, Michael Paquier
>>  wrote:
>>> On Thu, Jun 22, 2017 at 1:29 AM, Kuntal Ghosh
>>>  wrote:
 But, I've some more doubts.
 1. When should we use dsm_find_mapping()? (The first few lines of
 dsm_attach is same as dsm_find_mapping().)
 2. As a user of dsa, how should we check whether my dsa handle is
 already attached? I guess this is required because, if a user tries to
 re-attach a dsa handle,  it's punishing the user by throwing an error
 and the user wants to avoid such errors.
>>>
>>> From a logical point of view, there is nothing preventing the use of
>>> dsm_find_mapping() on a DSA handle, still the API layering looks wrong
>>> if you want to check for an existing mapping. So why not defining a
>>> new API, like dsa_find_mapping() that just wraps dsm_find_mapping()
>>> but has its own error handling? This would offer more flexibility for
>>> the future.
>>
>> Yeah. That sounds reasonable. Or, dsa_attach can throw error conditionally.
>
> Maybe, let's see what Robert and Thomas have to tell on the matter as
> they wrote that code. My take on the matter is that the DSA API should
> remain close to its parent. By the way, this is a new issue in
> Postgres 10 as this code has been introduced by 7526e10. So I have
> added an open item with Álvaro as owner.

Hmm.  So the problem here is that AutoVacLauncherMain assumes that
there are only two possibilities: (1) there is no handle published in
shmem yet, so we should create a DSA area and publish the handle, and
(2) there is a handle published in shmem so we should attach to it.
But there is a another possiblity: (3) there is a handle published in
shmem, but we are already attached to it (because we've restarted our
main loop after SIGINT).

The suggestion so far was to check if we're already attached to that
segment (making use of the implementation detail that a DSA handle is
also a DSM segment handle), but don't we know if we're already
attached by checking our AutoVacuumDSA variable?  Like this:

AutoVacuumShmem->av_workitems = InvalidDsaPointer;
LWLockRelease(AutovacuumLock);
}
-   else
+   else if (AutoVacuumDSA == NULL)
{
AutoVacuumDSA = dsa_attach(AutoVacuumShmem->av_dsa_handle);

-- 
Thomas Munro
http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Michael Paquier]

On Thu, Jun 22, 2017 at 2:44 PM, Kuntal Ghosh
 wrote:
> On Thu, Jun 22, 2017 at 9:48 AM, Michael Paquier
>  wrote:
>> On Thu, Jun 22, 2017 at 1:29 AM, Kuntal Ghosh
>>  wrote:
>>> But, I've some more doubts.
>>> 1. When should we use dsm_find_mapping()? (The first few lines of
>>> dsm_attach is same as dsm_find_mapping().)
>>> 2. As a user of dsa, how should we check whether my dsa handle is
>>> already attached? I guess this is required because, if a user tries to
>>> re-attach a dsa handle,  it's punishing the user by throwing an error
>>> and the user wants to avoid such errors.
>>
>> From a logical point of view, there is nothing preventing the use of
>> dsm_find_mapping() on a DSA handle, still the API layering looks wrong
>> if you want to check for an existing mapping. So why not defining a
>> new API, like dsa_find_mapping() that just wraps dsm_find_mapping()
>> but has its own error handling? This would offer more flexibility for
>> the future.
>
> Yeah. That sounds reasonable. Or, dsa_attach can throw error conditionally.

Maybe, let's see what Robert and Thomas have to tell on the matter as
they wrote that code. My take on the matter is that the DSA API should
remain close to its parent. By the way, this is a new issue in
Postgres 10 as this code has been introduced by 7526e10. So I have
added an open item with Álvaro as owner.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Kuntal Ghosh]

On Thu, Jun 22, 2017 at 9:48 AM, Michael Paquier
 wrote:
> On Thu, Jun 22, 2017 at 1:29 AM, Kuntal Ghosh
>  wrote:
>> On Wed, Jun 21, 2017 at 7:52 PM, Dilip Kumar  wrote:
>>> On Wed, Jun 21, 2017 at 7:47 PM, Kuntal Ghosh
>>>  wrote:
> IMHO, It's not a good idea to use DSM call to verify the DSA handle.
>
 Okay. Is there any particular scenario you've in mind where this may fail?
>>>
>>> It's not about failure, but about the abstraction.  When we are using
>>> the DSA we should not directly access the DSM which is under DSA.
>>>
>> Okay. I thought that I've found at least one usage of
>> dsm_find_mapping() in the code. :-)
>>
>> But, I've some more doubts.
>> 1. When should we use dsm_find_mapping()? (The first few lines of
>> dsm_attach is same as dsm_find_mapping().)
>> 2. As a user of dsa, how should we check whether my dsa handle is
>> already attached? I guess this is required because, if a user tries to
>> re-attach a dsa handle,  it's punishing the user by throwing an error
>> and the user wants to avoid such errors.
>
> From a logical point of view, there is nothing preventing the use of
> dsm_find_mapping() on a DSA handle, still the API layering looks wrong
> if you want to check for an existing mapping. So why not defining a
> new API, like dsa_find_mapping() that just wraps dsm_find_mapping()
> but has its own error handling? This would offer more flexibility for
> the future.
Yeah. That sounds reasonable. Or, dsa_attach can throw error conditionally.


-- 
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Yugo Nagata]

On Thu, 22 Jun 2017 13:12:48 +0900
Michael Paquier  wrote:

> On Wed, Jun 21, 2017 at 9:15 PM, Yugo Nagata  wrote:
> > This errors continue until this process is terminated or the server is 
> > restarted.
> >
> > When SIGINT is issued, the process exits from the main loop and returns
> > to sigsetjmp, and calls dsa_attach() before entering into the loop again,
> > this causes the error.
> >
> > We can fix it by calling dsa_attach() before sigsetjmp. Attached is the 
> > patch.
> 
> Your fix looks like a bad idea to me. If the shared memory area does
> not exist after an exception occurred the process should be able to
> re-attach to the shared memory area if it exists or create a new one
> if that's not the case. That should not be a one-time execution.

Thank you for your comment. I overlooked it and now I understand it.

> -- 
> Michael


-- 
Yugo Nagata 


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Michael Paquier]

On Thu, Jun 22, 2017 at 1:29 AM, Kuntal Ghosh
 wrote:
> On Wed, Jun 21, 2017 at 7:52 PM, Dilip Kumar  wrote:
>> On Wed, Jun 21, 2017 at 7:47 PM, Kuntal Ghosh
>>  wrote:
 IMHO, It's not a good idea to use DSM call to verify the DSA handle.

>>> Okay. Is there any particular scenario you've in mind where this may fail?
>>
>> It's not about failure, but about the abstraction.  When we are using
>> the DSA we should not directly access the DSM which is under DSA.
>>
> Okay. I thought that I've found at least one usage of
> dsm_find_mapping() in the code. :-)
>
> But, I've some more doubts.
> 1. When should we use dsm_find_mapping()? (The first few lines of
> dsm_attach is same as dsm_find_mapping().)
> 2. As a user of dsa, how should we check whether my dsa handle is
> already attached? I guess this is required because, if a user tries to
> re-attach a dsa handle,  it's punishing the user by throwing an error
> and the user wants to avoid such errors.

>From a logical point of view, there is nothing preventing the use of
dsm_find_mapping() on a DSA handle, still the API layering looks wrong
if you want to check for an existing mapping. So why not defining a
new API, like dsa_find_mapping() that just wraps dsm_find_mapping()
but has its own error handling? This would offer more flexibility for
the future.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Michael Paquier]

On Wed, Jun 21, 2017 at 9:15 PM, Yugo Nagata  wrote:
> This errors continue until this process is terminated or the server is 
> restarted.
>
> When SIGINT is issued, the process exits from the main loop and returns
> to sigsetjmp, and calls dsa_attach() before entering into the loop again,
> this causes the error.
>
> We can fix it by calling dsa_attach() before sigsetjmp. Attached is the patch.

Your fix looks like a bad idea to me. If the shared memory area does
not exist after an exception occurred the process should be able to
re-attach to the shared memory area if it exists or create a new one
if that's not the case. That should not be a one-time execution.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Kuntal Ghosh]

On Wed, Jun 21, 2017 at 7:52 PM, Dilip Kumar  wrote:
> On Wed, Jun 21, 2017 at 7:47 PM, Kuntal Ghosh
>  wrote:
>>> IMHO, It's not a good idea to use DSM call to verify the DSA handle.
>>>
>> Okay. Is there any particular scenario you've in mind where this may fail?
>
> It's not about failure, but about the abstraction.  When we are using
> the DSA we should not directly access the DSM which is under DSA.
>
Okay. I thought that I've found at least one usage of
dsm_find_mapping() in the code. :-)

But, I've some more doubts.
1. When should we use dsm_find_mapping()? (The first few lines of
dsm_attach is same as dsm_find_mapping().)
2. As a user of dsa, how should we check whether my dsa handle is
already attached? I guess this is required because, if a user tries to
re-attach a dsa handle,  it's punishing the user by throwing an error
and the user wants to avoid such errors.



-- 
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Dilip Kumar]

On Wed, Jun 21, 2017 at 7:47 PM, Kuntal Ghosh
 wrote:
>> IMHO, It's not a good idea to use DSM call to verify the DSA handle.
>>
> Okay. Is there any particular scenario you've in mind where this may fail?

It's not about failure, but about the abstraction.  When we are using
the DSA we should not directly access the DSM which is under DSA.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Kuntal Ghosh]

On Wed, Jun 21, 2017 at 7:07 PM, Dilip Kumar  wrote:
> On Wed, Jun 21, 2017 at 6:50 PM, Kuntal Ghosh
>  wrote:
>> I think we can just check dsm_find_mapping() to check whether the dsm
>> handle is already attached. Something like,
>>
>> }
>> -   else
>> +   else if(!dsm_find_mapping(AutoVacuumShmem->av_dsa_handle))
>> {
>> AutoVacuumDSA = dsa_attach(AutoVacuumShmem->av_dsa_handle);
>> dsa_pin_mapping(AutoVacuumDSA);
>>
>> Thoughts?
>
> IMHO, It's not a good idea to use DSM call to verify the DSA handle.
>
Okay. Is there any particular scenario you've in mind where this may fail?



-- 
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Dilip Kumar]

On Wed, Jun 21, 2017 at 6:50 PM, Kuntal Ghosh
 wrote:
> I think we can just check dsm_find_mapping() to check whether the dsm
> handle is already attached. Something like,
>
> }
> -   else
> +   else if(!dsm_find_mapping(AutoVacuumShmem->av_dsa_handle))
> {
> AutoVacuumDSA = dsa_attach(AutoVacuumShmem->av_dsa_handle);
> dsa_pin_mapping(AutoVacuumDSA);
>
> Thoughts?

IMHO, It's not a good idea to use DSM call to verify the DSA handle.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Kuntal Ghosh]

On Wed, Jun 21, 2017 at 5:45 PM, Yugo Nagata  wrote:
> Hi,
>
> As I report in another thread[1], I found the autovacuum launcher occurs
> the following error in PG 10 when this received SIGINT. I can repuroduce
> this by pg_cancel_backend or `kill -2 `.
>
> 2017-06-21 13:56:07.010 JST [32483] ERROR:  canceling statement due to user 
> request
> 2017-06-21 13:56:08.022 JST [32483] ERROR:  can't attach the same segment 
> more than once
> 2017-06-21 13:56:09.034 JST [32483] ERROR:  can't attach the same segment 
> more than once
> 2017-06-21 13:56:10.045 JST [32483] ERROR:  can't attach the same segment 
> more than once
> ...
>
> This errors continue until this process is terminated or the server is 
> restarted.
>
> When SIGINT is issued, the process exits from the main loop and returns
> to sigsetjmp, and calls dsa_attach() before entering into the loop again,
> this causes the error.
>
> We can fix it by calling dsa_attach() before sigsetjmp. Attached is the patch.
>
I think we can just check dsm_find_mapping() to check whether the dsm
handle is already attached. Something like,

}
-   else
+   else if(!dsm_find_mapping(AutoVacuumShmem->av_dsa_handle))
{
AutoVacuumDSA = dsa_attach(AutoVacuumShmem->av_dsa_handle);
dsa_pin_mapping(AutoVacuumDSA);

Thoughts?

-- 
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Autovacuum launcher occurs error when cancelled by SIGINT

[Yugo Nagata]

Hi,

As I report in another thread[1], I found the autovacuum launcher occurs
the following error in PG 10 when this received SIGINT. I can repuroduce
this by pg_cancel_backend or `kill -2 `.

2017-06-21 13:56:07.010 JST [32483] ERROR:  canceling statement due to user 
request
2017-06-21 13:56:08.022 JST [32483] ERROR:  can't attach the same segment more 
than once
2017-06-21 13:56:09.034 JST [32483] ERROR:  can't attach the same segment more 
than once
2017-06-21 13:56:10.045 JST [32483] ERROR:  can't attach the same segment more 
than once
...

This errors continue until this process is terminated or the server is 
restarted.

When SIGINT is issued, the process exits from the main loop and returns
to sigsetjmp, and calls dsa_attach() before entering into the loop again,
this causes the error. 

We can fix it by calling dsa_attach() before sigsetjmp. Attached is the patch. 

Regards,

[1] 
https://www.postgresql.org/message-id/20170621205657.61d90605.nagata%40sraoss.co.jp

-- 
Yugo Nagata 
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 89dd3b3..8a41a98 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -502,6 +502,28 @@ AutoVacLauncherMain(int argc, char *argv[])
 	MemoryContextSwitchTo(AutovacMemCxt);
 
 	/*
+	 * Set up our DSA so that backends can install work-item requests.  It may
+	 * already exist as created by a previous launcher.
+	 */
+	if (!AutoVacuumShmem->av_dsa_handle)
+	{
+		LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
+		AutoVacuumDSA = dsa_create(AutovacuumLock->tranche);
+		/* make sure it doesn't go away even if we do */
+		dsa_pin(AutoVacuumDSA);
+		dsa_pin_mapping(AutoVacuumDSA);
+		AutoVacuumShmem->av_dsa_handle = dsa_get_handle(AutoVacuumDSA);
+		/* delay array allocation until first request */
+		AutoVacuumShmem->av_workitems = InvalidDsaPointer;
+		LWLockRelease(AutovacuumLock);
+	}
+	else
+	{
+		AutoVacuumDSA = dsa_attach(AutoVacuumShmem->av_dsa_handle);
+		dsa_pin_mapping(AutoVacuumDSA);
+	}
+
+	/*
 	 * If an exception is encountered, processing resumes here.
 	 *
 	 * This code is a stripped down version of PostgresMain error recovery.
@@ -610,28 +632,6 @@ AutoVacLauncherMain(int argc, char *argv[])
 	 */
 	rebuild_database_list(InvalidOid);
 
-	/*
-	 * Set up our DSA so that backends can install work-item requests.  It may
-	 * already exist as created by a previous launcher.
-	 */
-	if (!AutoVacuumShmem->av_dsa_handle)
-	{
-		LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
-		AutoVacuumDSA = dsa_create(AutovacuumLock->tranche);
-		/* make sure it doesn't go away even if we do */
-		dsa_pin(AutoVacuumDSA);
-		dsa_pin_mapping(AutoVacuumDSA);
-		AutoVacuumShmem->av_dsa_handle = dsa_get_handle(AutoVacuumDSA);
-		/* delay array allocation until first request */
-		AutoVacuumShmem->av_workitems = InvalidDsaPointer;
-		LWLockRelease(AutovacuumLock);
-	}
-	else
-	{
-		AutoVacuumDSA = dsa_attach(AutoVacuumShmem->av_dsa_handle);
-		dsa_pin_mapping(AutoVacuumDSA);
-	}
-
 	/* loop until shutdown request */
 	while (!got_SIGTERM)
 	{

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers