Index maintenance function for BRIN doesn't check RecoveryInProgress()
Hi,
Three functions: brin_summarize_new_values, brin_summarize_range and
brin_desummarize_range can be called during recovery as follows.
=# select brin_summarize_new_values('a_idx');
ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database
objects while recovery is in progress
HINT: Only RowExclusiveLock or less can be acquired on database
objects during recovery.
I think we should complaint "recovery is in progress" error in this
case rather than erroring due to lock modes.
Attached patch fixes them.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
brin_maintenance_func.patch
Description: Binary data
Index maintenance function for BRIN doesn't check RecoveryInProgress()
Hi,
Three functions: brin_summarize_new_values, brin_summarize_range and
brin_desummarize_range can be called during recovery as follows.
=# select brin_summarize_new_values('a_idx');
ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database
objects while recovery is in progress
HINT: Only RowExclusiveLock or less can be acquired on database
objects during recovery.
I think we should complaint "recovery is in progress" error in this
case rather than erroring due to lock modes.
Attached patch fixes them.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
brin_maintenance_func.patch
Description: Binary data
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On Wed, Jun 13, 2018 at 2:28 PM, Masahiko Sawada wrote:
> Hi,
>
> Three functions: brin_summarize_new_values, brin_summarize_range and
> brin_desummarize_range can be called during recovery as follows.
>
> =# select brin_summarize_new_values('a_idx');
> ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database
> objects while recovery is in progress
> HINT: Only RowExclusiveLock or less can be acquired on database
> objects during recovery.
>
> I think we should complaint "recovery is in progress" error in this
> case rather than erroring due to lock modes.
+1
--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On Wed, Jun 13, 2018 at 12:48 PM Kuntal Ghosh
wrote:
> On Wed, Jun 13, 2018 at 2:28 PM, Masahiko Sawada
> wrote:
> > Hi,
> >
> > Three functions: brin_summarize_new_values, brin_summarize_range and
> > brin_desummarize_range can be called during recovery as follows.
> >
> > =# select brin_summarize_new_values('a_idx');
> > ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database
> > objects while recovery is in progress
> > HINT: Only RowExclusiveLock or less can be acquired on database
> > objects during recovery.
> >
> > I think we should complaint "recovery is in progress" error in this
> > case rather than erroring due to lock modes.
> +1
+1,
but current behavior doesn't seem to be bug, but rather not precise
enough error reporting. So, I think we shouldn't consider
backpatching this.
--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On 2018-Jun-13, Alexander Korotkov wrote:
> On Wed, Jun 13, 2018 at 12:48 PM Kuntal Ghosh
> wrote:
> > On Wed, Jun 13, 2018 at 2:28 PM, Masahiko Sawada
> > wrote:
> > > Hi,
> > >
> > > Three functions: brin_summarize_new_values, brin_summarize_range and
> > > brin_desummarize_range can be called during recovery as follows.
> > >
> > > =# select brin_summarize_new_values('a_idx');
> > > ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database
> > > objects while recovery is in progress
> > > HINT: Only RowExclusiveLock or less can be acquired on database
> > > objects during recovery.
Good catch!
> > > I think we should complaint "recovery is in progress" error in this
> > > case rather than erroring due to lock modes.
> > +1
>
> +1,
> but current behavior doesn't seem to be bug, but rather not precise
> enough error reporting. So, I think we shouldn't consider
> backpatching this.
I guess you could go either way ... we're just changing one unhelpful
error with a better one: there is no change in behavior. I would
backpatch this, myself, and avoid the code divergence.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On 13 June 2018 at 15:51, Alvaro Herrera wrote:
> On 2018-Jun-13, Alexander Korotkov wrote:
>
>> On Wed, Jun 13, 2018 at 12:48 PM Kuntal Ghosh
>> wrote:
>> > On Wed, Jun 13, 2018 at 2:28 PM, Masahiko Sawada
>> > wrote:
>> > > Hi,
>> > >
>> > > Three functions: brin_summarize_new_values, brin_summarize_range and
>> > > brin_desummarize_range can be called during recovery as follows.
>> > >
>> > > =# select brin_summarize_new_values('a_idx');
>> > > ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database
>> > > objects while recovery is in progress
>> > > HINT: Only RowExclusiveLock or less can be acquired on database
>> > > objects during recovery.
>
> Good catch!
>
>> > > I think we should complaint "recovery is in progress" error in this
>> > > case rather than erroring due to lock modes.
>> > +1
>>
>> +1,
>> but current behavior doesn't seem to be bug, but rather not precise
>> enough error reporting. So, I think we shouldn't consider
>> backpatching this.
>
> I guess you could go either way ... we're just changing one unhelpful
> error with a better one: there is no change in behavior. I would
> backpatch this, myself, and avoid the code divergence.
WAL control functions all say the same thing, so we can do that here also.
I'd prefer it if the message was more generic, so remove the
summarization/desummarization wording from the message. i.e.
"BRIN control functions cannot be executed during recovery"
--
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On Thu, Jun 14, 2018 at 12:04 AM, Simon Riggs wrote:
> On 13 June 2018 at 15:51, Alvaro Herrera wrote:
>> On 2018-Jun-13, Alexander Korotkov wrote:
>>
>>> On Wed, Jun 13, 2018 at 12:48 PM Kuntal Ghosh
>>> wrote:
>>> > On Wed, Jun 13, 2018 at 2:28 PM, Masahiko Sawada
>>> > wrote:
>>> > > Hi,
>>> > >
>>> > > Three functions: brin_summarize_new_values, brin_summarize_range and
>>> > > brin_desummarize_range can be called during recovery as follows.
>>> > >
>>> > > =# select brin_summarize_new_values('a_idx');
>>> > > ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database
>>> > > objects while recovery is in progress
>>> > > HINT: Only RowExclusiveLock or less can be acquired on database
>>> > > objects during recovery.
>>
>> Good catch!
>>
>>> > > I think we should complaint "recovery is in progress" error in this
>>> > > case rather than erroring due to lock modes.
>>> > +1
>>>
>>> +1,
>>> but current behavior doesn't seem to be bug, but rather not precise
>>> enough error reporting. So, I think we shouldn't consider
>>> backpatching this.
>>
>> I guess you could go either way ... we're just changing one unhelpful
>> error with a better one: there is no change in behavior. I would
>> backpatch this, myself, and avoid the code divergence.
>
> WAL control functions all say the same thing, so we can do that here also.
+1
> I'd prefer it if the message was more generic, so remove the
> summarization/desummarization wording from the message. i.e.
> "BRIN control functions cannot be executed during recovery"
>
Agreed. Attached an updated patch.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 60e650d..8453bfe 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -871,6 +871,12 @@ brin_summarize_range(PG_FUNCTION_ARGS)
Relation heapRel;
double numSummarized = 0;
+ if (RecoveryInProgress())
+ ereport(ERROR,
+(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("recovery is in progress"),
+ errhint("BRIN control functions cannot be executed during recovery.")));
+
if (heapBlk64 > BRIN_ALL_BLOCKRANGES || heapBlk64 < 0)
{
char *blk = psprintf(INT64_FORMAT, heapBlk64);
@@ -942,6 +948,12 @@ brin_desummarize_range(PG_FUNCTION_ARGS)
Relation indexRel;
bool done;
+ if (RecoveryInProgress())
+ ereport(ERROR,
+(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("recovery is in progress"),
+ errhint("BRIN control functions cannot be executed during recovery.")));
+
if (heapBlk64 > MaxBlockNumber || heapBlk64 < 0)
{
char *blk = psprintf(INT64_FORMAT, heapBlk64);
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On Thu, Jun 14, 2018 at 02:06:57AM +0900, Masahiko Sawada wrote: > On Thu, Jun 14, 2018 at 12:04 AM, Simon Riggs wrote: >> On 13 June 2018 at 15:51, Alvaro Herrera wrote: >>> I guess you could go either way ... we're just changing one unhelpful >>> error with a better one: there is no change in behavior. I would >>> backpatch this, myself, and avoid the code divergence. >> >> WAL control functions all say the same thing, so we can do that here also. > > +1 +1 for a back-patch to v10. The new error message brings clarity in my opinion. -- Michael signature.asc Description: PGP signature
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On 2018-Jun-14, Michael Paquier wrote: > On Thu, Jun 14, 2018 at 02:06:57AM +0900, Masahiko Sawada wrote: > > On Thu, Jun 14, 2018 at 12:04 AM, Simon Riggs wrote: > >> On 13 June 2018 at 15:51, Alvaro Herrera wrote: > >>> I guess you could go either way ... we're just changing one unhelpful > >>> error with a better one: there is no change in behavior. I would > >>> backpatch this, myself, and avoid the code divergence. > >> > >> WAL control functions all say the same thing, so we can do that here also. > > > > +1 > > +1 for a back-patch to v10. The new error message brings clarity in my > opinion. Pushed, backpatched to 9.5. Thanks! -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On Fri, Jun 15, 2018 at 2:20 AM, Alvaro Herrera wrote: > On 2018-Jun-14, Michael Paquier wrote: > >> On Thu, Jun 14, 2018 at 02:06:57AM +0900, Masahiko Sawada wrote: >> > On Thu, Jun 14, 2018 at 12:04 AM, Simon Riggs >> > wrote: >> >> On 13 June 2018 at 15:51, Alvaro Herrera wrote: >> >>> I guess you could go either way ... we're just changing one unhelpful >> >>> error with a better one: there is no change in behavior. I would >> >>> backpatch this, myself, and avoid the code divergence. >> >> >> >> WAL control functions all say the same thing, so we can do that here also. >> > >> > +1 >> >> +1 for a back-patch to v10. The new error message brings clarity in my >> opinion. > > Pushed, backpatched to 9.5. Thanks! > Thank you! Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On Wed, Jun 13, 2018 at 2:28 PM, Masahiko Sawada wrote:
> Hi,
>
> Three functions: brin_summarize_new_values, brin_summarize_range and
> brin_desummarize_range can be called during recovery as follows.
>
> =# select brin_summarize_new_values('a_idx');
> ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database
> objects while recovery is in progress
> HINT: Only RowExclusiveLock or less can be acquired on database
> objects during recovery.
>
> I think we should complaint "recovery is in progress" error in this
> case rather than erroring due to lock modes.
+1
--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On Wed, Jun 13, 2018 at 12:48 PM Kuntal Ghosh
wrote:
> On Wed, Jun 13, 2018 at 2:28 PM, Masahiko Sawada
> wrote:
> > Hi,
> >
> > Three functions: brin_summarize_new_values, brin_summarize_range and
> > brin_desummarize_range can be called during recovery as follows.
> >
> > =# select brin_summarize_new_values('a_idx');
> > ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database
> > objects while recovery is in progress
> > HINT: Only RowExclusiveLock or less can be acquired on database
> > objects during recovery.
> >
> > I think we should complaint "recovery is in progress" error in this
> > case rather than erroring due to lock modes.
> +1
+1,
but current behavior doesn't seem to be bug, but rather not precise
enough error reporting. So, I think we shouldn't consider
backpatching this.
--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On 2018-Jun-13, Alexander Korotkov wrote:
> On Wed, Jun 13, 2018 at 12:48 PM Kuntal Ghosh
> wrote:
> > On Wed, Jun 13, 2018 at 2:28 PM, Masahiko Sawada
> > wrote:
> > > Hi,
> > >
> > > Three functions: brin_summarize_new_values, brin_summarize_range and
> > > brin_desummarize_range can be called during recovery as follows.
> > >
> > > =# select brin_summarize_new_values('a_idx');
> > > ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database
> > > objects while recovery is in progress
> > > HINT: Only RowExclusiveLock or less can be acquired on database
> > > objects during recovery.
Good catch!
> > > I think we should complaint "recovery is in progress" error in this
> > > case rather than erroring due to lock modes.
> > +1
>
> +1,
> but current behavior doesn't seem to be bug, but rather not precise
> enough error reporting. So, I think we shouldn't consider
> backpatching this.
I guess you could go either way ... we're just changing one unhelpful
error with a better one: there is no change in behavior. I would
backpatch this, myself, and avoid the code divergence.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On 13 June 2018 at 15:51, Alvaro Herrera wrote:
> On 2018-Jun-13, Alexander Korotkov wrote:
>
>> On Wed, Jun 13, 2018 at 12:48 PM Kuntal Ghosh
>> wrote:
>> > On Wed, Jun 13, 2018 at 2:28 PM, Masahiko Sawada
>> > wrote:
>> > > Hi,
>> > >
>> > > Three functions: brin_summarize_new_values, brin_summarize_range and
>> > > brin_desummarize_range can be called during recovery as follows.
>> > >
>> > > =# select brin_summarize_new_values('a_idx');
>> > > ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database
>> > > objects while recovery is in progress
>> > > HINT: Only RowExclusiveLock or less can be acquired on database
>> > > objects during recovery.
>
> Good catch!
>
>> > > I think we should complaint "recovery is in progress" error in this
>> > > case rather than erroring due to lock modes.
>> > +1
>>
>> +1,
>> but current behavior doesn't seem to be bug, but rather not precise
>> enough error reporting. So, I think we shouldn't consider
>> backpatching this.
>
> I guess you could go either way ... we're just changing one unhelpful
> error with a better one: there is no change in behavior. I would
> backpatch this, myself, and avoid the code divergence.
WAL control functions all say the same thing, so we can do that here also.
I'd prefer it if the message was more generic, so remove the
summarization/desummarization wording from the message. i.e.
"BRIN control functions cannot be executed during recovery"
--
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On Thu, Jun 14, 2018 at 12:04 AM, Simon Riggs wrote:
> On 13 June 2018 at 15:51, Alvaro Herrera wrote:
>> On 2018-Jun-13, Alexander Korotkov wrote:
>>
>>> On Wed, Jun 13, 2018 at 12:48 PM Kuntal Ghosh
>>> wrote:
>>> > On Wed, Jun 13, 2018 at 2:28 PM, Masahiko Sawada
>>> > wrote:
>>> > > Hi,
>>> > >
>>> > > Three functions: brin_summarize_new_values, brin_summarize_range and
>>> > > brin_desummarize_range can be called during recovery as follows.
>>> > >
>>> > > =# select brin_summarize_new_values('a_idx');
>>> > > ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database
>>> > > objects while recovery is in progress
>>> > > HINT: Only RowExclusiveLock or less can be acquired on database
>>> > > objects during recovery.
>>
>> Good catch!
>>
>>> > > I think we should complaint "recovery is in progress" error in this
>>> > > case rather than erroring due to lock modes.
>>> > +1
>>>
>>> +1,
>>> but current behavior doesn't seem to be bug, but rather not precise
>>> enough error reporting. So, I think we shouldn't consider
>>> backpatching this.
>>
>> I guess you could go either way ... we're just changing one unhelpful
>> error with a better one: there is no change in behavior. I would
>> backpatch this, myself, and avoid the code divergence.
>
> WAL control functions all say the same thing, so we can do that here also.
+1
> I'd prefer it if the message was more generic, so remove the
> summarization/desummarization wording from the message. i.e.
> "BRIN control functions cannot be executed during recovery"
>
Agreed. Attached an updated patch.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 60e650d..8453bfe 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -871,6 +871,12 @@ brin_summarize_range(PG_FUNCTION_ARGS)
Relation heapRel;
double numSummarized = 0;
+ if (RecoveryInProgress())
+ ereport(ERROR,
+(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("recovery is in progress"),
+ errhint("BRIN control functions cannot be executed during recovery.")));
+
if (heapBlk64 > BRIN_ALL_BLOCKRANGES || heapBlk64 < 0)
{
char *blk = psprintf(INT64_FORMAT, heapBlk64);
@@ -942,6 +948,12 @@ brin_desummarize_range(PG_FUNCTION_ARGS)
Relation indexRel;
bool done;
+ if (RecoveryInProgress())
+ ereport(ERROR,
+(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("recovery is in progress"),
+ errhint("BRIN control functions cannot be executed during recovery.")));
+
if (heapBlk64 > MaxBlockNumber || heapBlk64 < 0)
{
char *blk = psprintf(INT64_FORMAT, heapBlk64);
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On Thu, Jun 14, 2018 at 02:06:57AM +0900, Masahiko Sawada wrote: > On Thu, Jun 14, 2018 at 12:04 AM, Simon Riggs wrote: >> On 13 June 2018 at 15:51, Alvaro Herrera wrote: >>> I guess you could go either way ... we're just changing one unhelpful >>> error with a better one: there is no change in behavior. I would >>> backpatch this, myself, and avoid the code divergence. >> >> WAL control functions all say the same thing, so we can do that here also. > > +1 +1 for a back-patch to v10. The new error message brings clarity in my opinion. -- Michael signature.asc Description: PGP signature
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On 2018-Jun-14, Michael Paquier wrote: > On Thu, Jun 14, 2018 at 02:06:57AM +0900, Masahiko Sawada wrote: > > On Thu, Jun 14, 2018 at 12:04 AM, Simon Riggs wrote: > >> On 13 June 2018 at 15:51, Alvaro Herrera wrote: > >>> I guess you could go either way ... we're just changing one unhelpful > >>> error with a better one: there is no change in behavior. I would > >>> backpatch this, myself, and avoid the code divergence. > >> > >> WAL control functions all say the same thing, so we can do that here also. > > > > +1 > > +1 for a back-patch to v10. The new error message brings clarity in my > opinion. Pushed, backpatched to 9.5. Thanks! -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()
On Fri, Jun 15, 2018 at 2:20 AM, Alvaro Herrera wrote: > On 2018-Jun-14, Michael Paquier wrote: > >> On Thu, Jun 14, 2018 at 02:06:57AM +0900, Masahiko Sawada wrote: >> > On Thu, Jun 14, 2018 at 12:04 AM, Simon Riggs >> > wrote: >> >> On 13 June 2018 at 15:51, Alvaro Herrera wrote: >> >>> I guess you could go either way ... we're just changing one unhelpful >> >>> error with a better one: there is no change in behavior. I would >> >>> backpatch this, myself, and avoid the code divergence. >> >> >> >> WAL control functions all say the same thing, so we can do that here also. >> > >> > +1 >> >> +1 for a back-patch to v10. The new error message brings clarity in my >> opinion. > > Pushed, backpatched to 9.5. Thanks! > Thank you! Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
