Re: Control your disk usage in PG: Introduction to Disk Quota Extension - Mailing list pgsql-hackers

From Hubert Zhang
Subject Re: Control your disk usage in PG: Introduction to Disk Quota Extension
Date
Msg-id CAB0yrenJg6eGX19DDUV65JTPO+uvD0ojn0YmRc2-rL-ZCjUu1w@mail.gmail.com
Whole thread Raw
In response to Re: Control your disk usage in PG: Introduction to Disk Quota Extension  (Haozhou Wang <hawang@pivotal.io>)
List pgsql-hackers
For this particular purpose, I don't immediately see why you need a
hook in both places.  If ReadBuffer is called with P_NEW, aren't we
guaranteed to end up in smgrextend()?
For the usercase in diskquota.
BufferExtendCheckPerms_hook is used to do dynamic query enforcement, while smgr related hooks are used to detect relfilenodeoid of active tables and write them into shared memory(which is used to accelerate refreshing of diskquota model).
The reason we don't use smgr_extend hook to replace ReadBuffer hook to do enforcement has two folds:
1. As for enforcement, we don't want to affect the performance of insert query. But hooks in smgr_extend need to convert relfilenode to reloid firstly which need an indexscan.
2. Using hooks in ReadBuffer instead of smgr_extend could avoid to enforcement on 'cluster relation' operator. For example, 'vacuum full table' will firstly cluster and create a new table, and then delete the old table. Because the disk usage will first grow and then shrink, if quota limit is reached, then vacuum full will fail.(but in fact we want vacuum full to reduce disk usage) Using hooks in ReadBuffer is one solution to this problem. Of course, there are other solutions. But This is one of the reason we use BufferExtendCheckPerms_hook to do enforcement at current stage.

On Thu, Nov 22, 2018 at 7:26 PM Haozhou Wang <hawang@pivotal.io> wrote:
Thank you very much for your review.
We refactored our patch with new names and comments.

For ReadBufferExtended hook, yes, Readbuffer with P_NEW will then call smgrextend.

But in smgrextend, we cannot get the oid of a relation, and it will take some time to get the oid via smgrrelation.
We would like to add a hook just before the smgrextend to get the oid and avoid use RelidByRelfilenode().

New patch is attached in the attachment.
Thank a lot!

Regards,
Haozhou


On Wed, Nov 21, 2018 at 10:48 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Nov 20, 2018 at 2:20 AM Haozhou Wang <hawang@pivotal.io> wrote:
> We prepared a patch that includes the hook points. And such hook points are needed for disk quota extension.
> There are two hooks.
> One is SmgrStat_hook. It's used to perform ad-hoc logic in storage when doing smgr create/extend/truncate in general. As for disk quota extension, this hook is used to detect active tables(new created tables, tables extending new blocks, or tables being truncated)
> The other is BufferExtendCheckPerms_hook. It's used to perform ad-hoc logic when buffer extend a new block. Since ReadBufferExtended is a hot function, we call this hook only when blockNum == P_NEW. As  for disk quota extension, this hook is used to do query enforcement during the query is loading data.
>
> Any comments are appreciated.

+1 for adding some hooks to support this kind of thing, but I think
the names you've chosen are not very good.  The hook name should
describe the place from which it is called, not the purpose for which
one imagines that it will be used, because somebody else might imagine
another use.  Both BufferExtendCheckPerms_hook_type and
SmgrStat_hook_type are imagining that they know what the hook does -
CheckPerms in the first case and Stat in the second case.

For this particular purpose, I don't immediately see why you need a
hook in both places.  If ReadBuffer is called with P_NEW, aren't we
guaranteed to end up in smgrextend()?

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




--
Thanks

Hubert Zhang

pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: row filtering for logical replication
Next
From: David Rowley
Date:
Subject: Re: Should new partitions inherit their tablespace from their parent?