Thread: GIN vacuum bug

GIN vacuum bug

From
Teodor Sigaev
Date:
Hi!

After reading thread

http://www.postgresql.org/message-id/flat/CAMkU=1xALfLhUUohFP5v33RzedLVb5aknNUjcEuM9KNBKrB6-Q@mail.gmail.com#CAMkU=1xALfLhUUohFP5v33RzedLVb5aknNUjcEuM9KNBKrB6-Q@mail.gmail.com

I agree, that it's a bug, although, seems, with low probability. I'd like to
suggest patch with introduces:
1 cleanup process could be only one at the same time
2 if cleanup called from regular insert sees waiting concurrent cleanup then
   it will stop process in hope that another cleanup is vacuum called.

Insert-called cleanup process locks metapage with
ConditionalLockPage(ExclusiveLock) and if it fails then just goes away, because
other cleanup is already in progress.  Also, insert-called cleanup process will
sometimes do lock/conditional lock metapage. Vacuum-called cleanup process locks
with a help of unconditional LockPage() and will not relock metapage during
process, that gives a warranty to be a single cleanup process. Relock of
insert-called cleanup process allows to leave a rest of work to vacuum if it
runs right now. This reduces latency for insert and allows to vacuum to be sure
that pending list is clear.

Also, patch differentiates which limit of memory to use: autovacuum_work_mem,
maintenance_work_mem or work_mem depending on call path.
--
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/

Attachment

Re: GIN vacuum bug

From
Robert Haas
Date:
On Thu, Sep 24, 2015 at 11:47 AM, Teodor Sigaev <teodor@sigaev.ru> wrote:
> Hi!
>
> After reading thread
>
http://www.postgresql.org/message-id/flat/CAMkU=1xALfLhUUohFP5v33RzedLVb5aknNUjcEuM9KNBKrB6-Q@mail.gmail.com#CAMkU=1xALfLhUUohFP5v33RzedLVb5aknNUjcEuM9KNBKrB6-Q@mail.gmail.com
>
> I agree, that it's a bug, although, seems, with low probability. I'd like to
> suggest patch with introduces:
> 1 cleanup process could be only one at the same time
> 2 if cleanup called from regular insert sees waiting concurrent cleanup then
>   it will stop process in hope that another cleanup is vacuum called.
>
> Insert-called cleanup process locks metapage with
> ConditionalLockPage(ExclusiveLock) and if it fails then just goes away,
> because other cleanup is already in progress.  Also, insert-called cleanup
> process will sometimes do lock/conditional lock metapage. Vacuum-called
> cleanup process locks with a help of unconditional LockPage() and will not
> relock metapage during process, that gives a warranty to be a single cleanup
> process. Relock of insert-called cleanup process allows to leave a rest of
> work to vacuum if it runs right now. This reduces latency for insert and
> allows to vacuum to be sure that pending list is clear.
>
> Also, patch differentiates which limit of memory to use:
> autovacuum_work_mem, maintenance_work_mem or work_mem depending on call
> path.

Using a heavyweight lock on the metapage seems like a better idea to
me than the previous proposal of using the relation lock.  But I think
it would be best to discuss this on the original thread instead of
starting a new one.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company