Re: Index maintenance function for BRIN doesn't check RecoveryInProgress()

On Thu, Jun 14, 2018 at 12:04 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On 13 June 2018 at 15:51, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
>> On 2018-Jun-13, Alexander Korotkov wrote:
>>
>>> On Wed, Jun 13, 2018 at 12:48 PM Kuntal Ghosh
>>> <kuntalghosh(dot)2007(at)gmail(dot)com> wrote:
>>> > On Wed, Jun 13, 2018 at 2:28 PM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> 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