Thread

1. Control your disk usage in PG: Introduction to Disk Quota Extension

   Hubert Zhang <hzhang@pivotal.io> — 2018-11-13T08:47:13Z

   Hi all,
   
   We implement disk quota feature on Postgresql as an extension(link:
   https://github.com/greenplum-db/diskquota),
   If you are interested, try and use it to limit the amount of disk space that
   a schema or a role can use in Postgresql.
   Any feedback or question are appreciated.
   
   Overview
   
   Diskquota is an extension that provides disk usage enforcement for database
   objects in Postgresql. Currently it supports to set quota limit on schema
   and role in a given database and limit the amount of disk space that a
   schema or a role can use.
   
   This project is inspired by Heikki's pg_quota project (link:
   https://github.com/hlinnaka/pg_quota) and enhance it to support different
   kinds of DDL and DML which may change the disk usage of database objects.
   
   Diskquota is a soft limit of disk uages. It has some delay to detect the
   schemas or roles whose quota limit is exceeded. Here 'soft limit' supports
   two kinds of encforcement: Query loading data into out-of-quota schema/role
   will be forbidden before query is running. Query loading data into
   schema/role with rooms will be cancelled when the quota limit is reached
   dynamically during the query is running.
   <https://github.com/greenplum-db/diskquota/blob/master/README.md#design>
   Design
   
   Diskquota extension is based on background worker framework in Postgresql.
   There are two kinds of background workers: diskquota launcher and diskquota
   worker.
   
   There is only one laucher process per database cluster(i.e. one laucher per
   postmaster). Launcher process is reponsible for manage worker processes:
   Calling RegisterDynamicBackgroundWorker() to create new workers and keep
   their handle. Calling TerminateBackgroundWorker() to terminate workers
   which are disabled when DBA modify diskquota.monitor_databases
   
   There are many worker processes, one for each database which is listed in
   diskquota.monitor_databases. Currently, we support to monitor at most 10
   databases at the same time. Worker processes are responsible for monitoring
   the disk usage of schemas and roles for the target database, and do quota
   enfocement. It will periodically (can be set via diskquota.naptime)
   recalcualte the table size of active tables, and update their corresponding
   schema or owner's disk usage. Then compare with quota limit for those
   schemas or roles. If exceeds the limit, put the corresponding schemas or
   roles into the blacklist in shared memory. Schemas or roles in blacklist
   are used to do query enforcement to cancel queries which plan to load data
   into these schemas or roles.
   <https://github.com/greenplum-db/diskquota/blob/master/README.md#active-table>Active
   table
   
   Active tables are the tables whose table size may change in the last quota
   check interval. We use hooks in smgecreate(), smgrextend() and
   smgrtruncate() to detect active tables and store them(currently
   relfilenode) in the shared memory. Diskquota worker process will
   periodically consuming active table in shared memories, convert relfilenode
   to relaton oid, and calcualte table size by calling
   pg_total_relation_size(), which will sum the size of table(including: base,
   vm, fsm, toast and index).
   <https://github.com/greenplum-db/diskquota/blob/master/README.md#enforcement>
   Enforcement
   
   Enforcement is implemented as hooks. There are two kinds of enforcement
   hooks: enforcement before query is running and enforcement during query is
   running. The 'before query' one is implemented at ExecutorCheckPerms_hook
   in function ExecCheckRTPerms() The 'during query' one is implemented at
   BufferExtendCheckPerms_hook in function ReadBufferExtended(). Note that the
   implementation of BufferExtendCheckPerms_hook will firstly check whether
   function request a new block, if not skip directyly.
   <https://github.com/greenplum-db/diskquota/blob/master/README.md#quota-setting-store>Quota
   setting store
   
   Quota limit of a schema or a role is stored in table 'quota_config' in
   'diskquota' schema in monitored database. So each database stores and
   manages its own disk quota configuration. Note that although role is a db
   object in cluster level, we limit the diskquota of a role to be database
   specific. That is to say, a role may has different quota limit on different
   databases and their disk usage is isolated between databases.
   <https://github.com/greenplum-db/diskquota/blob/master/README.md#install>
   Install
   
      1. Add hook functions to Postgres by applying patch. It's required since
      disk quota need to add some new hook functions in postgres core. This step
      would be skipped after patch is merged into postgres in future.
   
   # install patch into postgres_src and rebuild postgres.
   cd postgres_src;
   git apply $diskquota_src/patch/pg_hooks.patch;
   make;
   make install;
   
   
      1. Compile and install disk quota.
   
   cd $diskquota_src;
   make;
   make install;
   
   
      1. Config postgresql.conf
   
   # enable diskquota in preload library.
   shared_preload_libraries = 'diskquota'
   # set monitored databases
   diskquota.monitor_databases = 'postgres'
   # set naptime (second) to refresh the disk quota stats periodically
   diskquota.naptime = 2
   # restart database to load preload library.
   pg_ctl restart
   
   
      1. Create diskquota extension in monitored database.
   
   create extension diskquota;
   
   
      1. Reload database configuraion
   
   # reset monitored database list in postgresql.conf
   diskquota.monitor_databases = 'postgres, postgres2'
   # reload configuration
   pg_ctl reload
   
   <https://github.com/greenplum-db/diskquota/blob/master/README.md#usage>Usage
   
      1. Set/update/delete schema quota limit using diskquota.set_schema_quota
   
   create schema s1;
   select diskquota.set_schema_quota('s1', '1 MB');
   set search_path to s1;
   
   create table a(i int);
   # insert small data succeeded
   insert into a select generate_series(1,100);
   # insert large data failed
   insert into a select generate_series(1,10000000);
   # insert small data failed
   insert into a select generate_series(1,100);
   
   # delete quota configuration
   select diskquota.set_schema_quota('s1', '-1');
   # insert small data succeed
   select pg_sleep(5);
   insert into a select generate_series(1,100);
   reset search_path;
   
   
      1. Set/update/delete role quota limit using diskquota.set_role_quota
   
   create role u1 nologin;
   create table b (i int);
   alter table b owner to u1;
   select diskquota.set_role_quota('u1', '1 MB');
   
   # insert small data succeeded
   insert into b select generate_series(1,100);
   # insert large data failed
   insert into b select generate_series(1,10000000);
   # insert small data failed
   insert into b select generate_series(1,100);
   
   # delete quota configuration
   select diskquota.set_role_quota('u1', '-1');
   # insert small data succeed
   select pg_sleep(5);
   insert into a select generate_series(1,100);
   reset search_path;
   
   
      1. Show schema quota limit and current usage
   
   select * from diskquota.show_schema_quota_view;
   
   <https://github.com/greenplum-db/diskquota/blob/master/README.md#test>Test
   
   Run regression tests.
   
   cd contrib/diskquota;
   make installcheck
   
   <https://github.com/greenplum-db/diskquota/blob/master/README.md#benchmark--performence-test>Benchmark
   & Performence Test
   <https://github.com/greenplum-db/diskquota/blob/master/README.md#cost-of-diskquota-worker>Cost
   of diskquota worker
   
   During each refresh interval, the disk quota worker need to refresh the
   disk quota model.
   
   It take less than 100ms under 100K user tables with no avtive tables.
   
   It take less than 200ms under 100K user tables with 1K active tables.
   <https://github.com/greenplum-db/diskquota/blob/master/README.md#impact-on-oltp-queries>Impact
   on OLTP queries
   
   We test OLTP queries to measure the impact of enabling diskquota feature.
   The range is from 2k tables to 10k tables. Each connection will insert 100
   rows into each table. And the parallel connections range is from 5 to 25.
   Number of active tables will be around 1k.
   
   Without diskquota enabled (seconds)
   2k4k6k8k10k
   5 4.002 11.356 18.460 28.591 41.123
   10 4.832 11.988 21.113 32.829 45.832
   15 6.238 16.896 28.722 45.375 64.642
   20 8.036 21.711 38.499 61.763 87.875
   25 9.909 27.175 47.996 75.688 106.648
   
   With diskquota enabled (seconds)
   2k4k6k8k10k
   5 4.135 10.641 18.776 28.804 41.740
   10 4.773 12.407 22.351 34.243 47.568
   15 6.355 17.305 30.941 46.967 66.216
   20 9.451 22.231 40.645 61.758 88.309
   25 10.096 26.844 48.910 76.537 108.025
   
   The performance difference between with/without diskquota enabled are less
   then 2-3% in most case. Therefore, there is no significant performance
   downgrade when diskquota is enabled.
   <https://github.com/greenplum-db/diskquota/blob/master/README.md#notes>Notes
   
      1. Drop database with diskquota enabled.
   
   If DBA enable monitoring diskquota on a database, there will be a
   connection to this database from diskquota worker process. DBA need to
   first remove this database from diskquota.monitor_databases in
   postgres.conf, and reload configuration by call pg_ctl reload. Then
   database could be dropped successfully.
   
      1. Temp table.
   
   Diskquota supports to limit the disk usage of temp table as well. But
   schema and role are different. For role, i.e. the owner of the temp table,
   diakquota will treat it the same as normal tables and sum its table size to
   its owner's quota. While for schema, temp table is located under namespace
   'pg_temp_backend_id', so temp table size will not sum to the current
   schema's qouta.
   
   
   -- 
   Thanks
   
   Hubert Zhang, Haozhou Wang, Hao Wu, Jack WU
   

2. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

   Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2018-11-13T19:54:34Z

   On Tue, 2018-11-13 at 16:47 +0800, Hubert Zhang wrote:
   > Hi all,
   > 
   > We implement disk quota feature on Postgresql as an extension(link:
   > https://github.com/greenplum-db/diskquota),
   > If you are interested, try and use it to limit the amount of disk
   > space that
   > a schema or a role can use in Postgresql.
   > Any feedback or question are appreciated.
   > 
   
   It's not clear to me what's the goal of this thread? I understand what
   quotas are about, but are you sharing it because (a) it's a useful
   extension, (b) you propose adding a couple of new hooks (and keep the
   extension separate) or (c) you propose adding both the hooks and the
   extension (into contrib)?
   
   I assume it's either (b) and (c), in which case you should add it to
   2019-01 CF: https://commitfest.postgresql.org/21/
   
   In either case, it might be useful to add a LICENSE to the github
   repository, it's not clear what's the situation in this respect. That
   probably matters especially for (b), because for (c) it'd end up with
   PostgreSQL license automatically.
   
   regards
   
   -- 
   Tomas Vondra                  http://www.2ndQuadrant.com
   PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
   
   
   
   

3. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

   Hubert Zhang <hzhang@pivotal.io> — 2018-11-14T10:06:09Z

   Thanks, Tomas,
   
   Yes, we want to add the hooks into postgres repo.
   But before that, we plan to firstly get some feedbacks from community about
   the diskquota extension implementation and usage?
   Later, we'll modify our license and submit the hooks into CF.
   
   Thanks
   Hubert
   
   On Wed, Nov 14, 2018 at 3:54 AM Tomas Vondra <tomas.vondra@2ndquadrant.com>
   wrote:
   
   > On Tue, 2018-11-13 at 16:47 +0800, Hubert Zhang wrote:
   > > Hi all,
   > >
   > > We implement disk quota feature on Postgresql as an extension(link:
   > > https://github.com/greenplum-db/diskquota),
   > > If you are interested, try and use it to limit the amount of disk
   > > space that
   > > a schema or a role can use in Postgresql.
   > > Any feedback or question are appreciated.
   > >
   >
   > It's not clear to me what's the goal of this thread? I understand what
   > quotas are about, but are you sharing it because (a) it's a useful
   > extension, (b) you propose adding a couple of new hooks (and keep the
   > extension separate) or (c) you propose adding both the hooks and the
   > extension (into contrib)?
   >
   > I assume it's either (b) and (c), in which case you should add it to
   > 2019-01 CF: https://commitfest.postgresql.org/21/
   >
   > In either case, it might be useful to add a LICENSE to the github
   > repository, it's not clear what's the situation in this respect. That
   > probably matters especially for (b), because for (c) it'd end up with
   > PostgreSQL license automatically.
   >
   > regards
   >
   > --
   > Tomas Vondra                  http://www.2ndQuadrant.com
   > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
   >
   >
   
   -- 
   Thanks
   
   Hubert Zhang
   

4. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

   Haozhou Wang <hawang@pivotal.io> — 2018-11-20T07:20:27Z

   Hi all,
   
   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.
   
   Regards,
   Haozhou
   
   On Wed, Nov 14, 2018 at 6:07 PM Hubert Zhang <hzhang@pivotal.io> wrote:
   
   > Thanks, Tomas,
   >
   > Yes, we want to add the hooks into postgres repo.
   > But before that, we plan to firstly get some feedbacks from community
   > about the diskquota extension implementation and usage?
   > Later, we'll modify our license and submit the hooks into CF.
   >
   > Thanks
   > Hubert
   >
   > On Wed, Nov 14, 2018 at 3:54 AM Tomas Vondra <tomas.vondra@2ndquadrant.com>
   > wrote:
   >
   >> On Tue, 2018-11-13 at 16:47 +0800, Hubert Zhang wrote:
   >> > Hi all,
   >> >
   >> > We implement disk quota feature on Postgresql as an extension(link:
   >> > https://github.com/greenplum-db/diskquota),
   >> > If you are interested, try and use it to limit the amount of disk
   >> > space that
   >> > a schema or a role can use in Postgresql.
   >> > Any feedback or question are appreciated.
   >> >
   >>
   >> It's not clear to me what's the goal of this thread? I understand what
   >> quotas are about, but are you sharing it because (a) it's a useful
   >> extension, (b) you propose adding a couple of new hooks (and keep the
   >> extension separate) or (c) you propose adding both the hooks and the
   >> extension (into contrib)?
   >>
   >> I assume it's either (b) and (c), in which case you should add it to
   >> 2019-01 CF: https://commitfest.postgresql.org/21/
   >>
   >> In either case, it might be useful to add a LICENSE to the github
   >> repository, it's not clear what's the situation in this respect. That
   >> probably matters especially for (b), because for (c) it'd end up with
   >> PostgreSQL license automatically.
   >>
   >> regards
   >>
   >> --
   >> Tomas Vondra                  http://www.2ndQuadrant.com
   >> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
   >>
   >>
   >
   > --
   > Thanks
   >
   > Hubert Zhang
   >
   

5. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

   Robert Haas <robertmhaas@gmail.com> — 2018-11-21T14:47:44Z

   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
   
   
   

6. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

   Haozhou Wang <hawang@pivotal.io> — 2018-11-22T11:25:37Z

   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
   >
   

7. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

   Hubert Zhang <hzhang@pivotal.io> — 2018-11-23T02:22:27Z

   >
   > 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
   

8. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

   Michael Paquier <michael@paquier.xyz> — 2018-12-26T05:56:47Z

   On Wed, Nov 21, 2018 at 09:47:44AM -0500, Robert Haas wrote:
   > +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.
   
   I personally don't mind making Postgres more pluggable, but I don't
   think that we actually need the extra ones proposed here at the layer
   of smgr, as smgr is already a layer designed to call an underlying set
   of APIs able to extend, unlink, etc. depending on the storage type.
   
   > 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()?
   
   Yes, that's a bit awkward.
   --
   Michael
   

9. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

   Haozhou Wang <hawang@pivotal.io> — 2019-01-07T10:56:09Z

   Thanks very much for your comments.
   
   To the best of my knowledge, smgr is a layer that abstract the storage
   operations. Therefore, it is a good place to control or collect information
   the storage operations without touching the physical storage layer.
   Moreover, smgr is coming with actual disk IO operation (not consider the OS
   cache) for postgres. So we do not need to worry about the buffer management
   in postgres.
   It will make the purpose of hook is pure: a hook for actual disk IO.
   
   Regards,
   Haozhou
   
   On Wed, Dec 26, 2018 at 1:56 PM Michael Paquier <michael@paquier.xyz> wrote:
   
   > On Wed, Nov 21, 2018 at 09:47:44AM -0500, Robert Haas wrote:
   > > +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.
   >
   > I personally don't mind making Postgres more pluggable, but I don't
   > think that we actually need the extra ones proposed here at the layer
   > of smgr, as smgr is already a layer designed to call an underlying set
   > of APIs able to extend, unlink, etc. depending on the storage type.
   >
   > > 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()?
   >
   > Yes, that's a bit awkward.
   > --
   > Michael
   

10. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Hubert Zhang <hzhang@pivotal.io> — 2019-01-22T04:08:46Z

    >
    > > 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()?
    > Yes, that's a bit awkward.
    
    
     Hi Michael, we revisit the ReadBuffer hook and remove it in the latest
    patch.
    ReadBuffer hook is original used to do enforcement(e.g. out of diskquota
    limit) when query is loading data.
    We plan to put the enforcement work of running query to separate diskquota
    worker process.
    Let worker process to detect the backends to be cancelled and send SIGINT
    to these backends.
    So there is no need for ReadBuffer hook anymore.
    
    Our patch currently only contains smgr related hooks to catch the file
    change and get the Active Table list for diskquota extension.
    
    Thanks Hubert.
    
    
    On Mon, Jan 7, 2019 at 6:56 PM Haozhou Wang <hawang@pivotal.io> wrote:
    
    > Thanks very much for your comments.
    >
    > To the best of my knowledge, smgr is a layer that abstract the storage
    > operations. Therefore, it is a good place to control or collect information
    > the storage operations without touching the physical storage layer.
    > Moreover, smgr is coming with actual disk IO operation (not consider the
    > OS cache) for postgres. So we do not need to worry about the buffer
    > management in postgres.
    > It will make the purpose of hook is pure: a hook for actual disk IO.
    >
    > Regards,
    > Haozhou
    >
    > On Wed, Dec 26, 2018 at 1:56 PM Michael Paquier <michael@paquier.xyz>
    > wrote:
    >
    >> On Wed, Nov 21, 2018 at 09:47:44AM -0500, Robert Haas wrote:
    >> > +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.
    >>
    >> I personally don't mind making Postgres more pluggable, but I don't
    >> think that we actually need the extra ones proposed here at the layer
    >> of smgr, as smgr is already a layer designed to call an underlying set
    >> of APIs able to extend, unlink, etc. depending on the storage type.
    >>
    >> > 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()?
    >>
    >> Yes, that's a bit awkward.
    >> --
    >> Michael
    >
    >
    
    -- 
    Thanks
    
    Hubert Zhang
    

11. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Hubert Zhang <hzhang@pivotal.io> — 2019-01-30T02:26:52Z

    Hi Michael, Robert
    For you question about the hook position, I want to explain more about the
    background why we want to introduce these hooks.
    We wrote a diskquota extension <https://github.com/greenplum-db/diskquota>
    for Postgresql(which is inspired by Heikki's pg_quota
    <https://github.com/hlinnaka/pg_quota>). Diskquota extension is used to
    control the disk usage in Postgresql in a fine-grained way, which means:
    1. You could set disk quota limit at schema level or role level.
    2. A background worker will gather the current disk usage for each
    schema/role in realtime.
    3. A background worker will generate the blacklist for schema/role whose
    quota limit is exceeded.
    4. New transaction want to insert data into the schema/role in the
    blacklist will be cancelled.
    
    In step 2, gathering the current disk usage for each schema needs to sum
    disk size of all the tables in this schema. This is a time consuming
    operation. We want to use hooks in SMGR to detect the Active Table, and
    only recalculate the disk size of all the Active Tables.
    For example, the smgrextend hook indicates that you allocate a new block
    and the table need to be treated as Active Table.
    
    Do you have some better hook positions recommend to solve the above user
    case?
    Thanks in advance.
    
    Hubert
    
    
    
    
    
    On Tue, Jan 22, 2019 at 12:08 PM Hubert Zhang <hzhang@pivotal.io> wrote:
    
    > > 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()?
    >> Yes, that's a bit awkward.
    >
    >
    >  Hi Michael, we revisit the ReadBuffer hook and remove it in the latest
    > patch.
    > ReadBuffer hook is original used to do enforcement(e.g. out of diskquota
    > limit) when query is loading data.
    > We plan to put the enforcement work of running query to separate diskquota
    > worker process.
    > Let worker process to detect the backends to be cancelled and send SIGINT
    > to these backends.
    > So there is no need for ReadBuffer hook anymore.
    >
    > Our patch currently only contains smgr related hooks to catch the file
    > change and get the Active Table list for diskquota extension.
    >
    > Thanks Hubert.
    >
    >
    > On Mon, Jan 7, 2019 at 6:56 PM Haozhou Wang <hawang@pivotal.io> wrote:
    >
    >> Thanks very much for your comments.
    >>
    >> To the best of my knowledge, smgr is a layer that abstract the storage
    >> operations. Therefore, it is a good place to control or collect information
    >> the storage operations without touching the physical storage layer.
    >> Moreover, smgr is coming with actual disk IO operation (not consider the
    >> OS cache) for postgres. So we do not need to worry about the buffer
    >> management in postgres.
    >> It will make the purpose of hook is pure: a hook for actual disk IO.
    >>
    >> Regards,
    >> Haozhou
    >>
    >> On Wed, Dec 26, 2018 at 1:56 PM Michael Paquier <michael@paquier.xyz>
    >> wrote:
    >>
    >>> On Wed, Nov 21, 2018 at 09:47:44AM -0500, Robert Haas wrote:
    >>> > +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.
    >>>
    >>> I personally don't mind making Postgres more pluggable, but I don't
    >>> think that we actually need the extra ones proposed here at the layer
    >>> of smgr, as smgr is already a layer designed to call an underlying set
    >>> of APIs able to extend, unlink, etc. depending on the storage type.
    >>>
    >>> > 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()?
    >>>
    >>> Yes, that's a bit awkward.
    >>> --
    >>> Michael
    >>
    >>
    >
    > --
    > Thanks
    >
    > Hubert Zhang
    >
    
    
    -- 
    Thanks
    
    Hubert Zhang
    

12. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Hubert Zhang <hzhang@pivotal.io> — 2019-01-30T12:34:27Z

    Hi all,
    
    Currently we add hooks in SMGR to detect which table is being modified(disk
    size change).
    Inserting rows into existing page with room will not introduce new block,
    and thus should not be treated as active table. smgrextend is a good
    position to meet this behavior.
    We welcome suggestions on other better hook positions!
    
    Besides, suppose we use smgr as hook position, I want to discuss the API
    passed to the hook function.
    Take smgrextend as example. The function interface of smgrextend is like
    that:
    ```
    void smgrextend
    (SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, char *buffer,
    bool skipFsync);
    ```
    So the hook api of smgrextend could have two main options.
    Hook API Option1
    ```
    typedef void (*smgrextend_hook_type)
    (RelFileNode smgr_rnode,ForkNumber forknum);
    ```
    Hook API Option 2
    ```
    typedef void (*smgrextend_hook_type)
    (SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,char *buffer,
    bool skipFsync);
    ```
    
    As for Option1. Since diskquota extension only needs the relfilenode
    information to detect the active tables, Option1 just pass the RelFileNode
    to the hook function. It's more clear and has semantic meaning.
    
    While Option 2 is to pass the original parameters to the hook functions
    without any filter. This is more general and let the different hook
    implementations to decide how to use these parameters.
    
    Option 1 also needs some additional work to handle smgrdounlinkall case,
    whose input parameter is the SMgrRelation list. We may need to palloc
    Relfilenode list and pfree it manually.
    smgrdounlinkall function interface:
    ```
    smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo, char
    *relstorages)
    ```
    
    Based on the assumption we use smgr as hook position, hook API option1 or
    option2 which is better?
    Or we could find some balanced API between option1 and option2?
    
    Again comments on other better hook positions are appreciated!
    
    Thanks
    Hubert
    
    
    On Wed, Jan 30, 2019 at 10:26 AM Hubert Zhang <hzhang@pivotal.io> wrote:
    
    > Hi Michael, Robert
    > For you question about the hook position, I want to explain more about the
    > background why we want to introduce these hooks.
    > We wrote a diskquota extension <https://github.com/greenplum-db/diskquota>
    > for Postgresql(which is inspired by Heikki's pg_quota
    > <https://github.com/hlinnaka/pg_quota>). Diskquota extension is used to
    > control the disk usage in Postgresql in a fine-grained way, which means:
    > 1. You could set disk quota limit at schema level or role level.
    > 2. A background worker will gather the current disk usage for each
    > schema/role in realtime.
    > 3. A background worker will generate the blacklist for schema/role whose
    > quota limit is exceeded.
    > 4. New transaction want to insert data into the schema/role in the
    > blacklist will be cancelled.
    >
    > In step 2, gathering the current disk usage for each schema needs to sum
    > disk size of all the tables in this schema. This is a time consuming
    > operation. We want to use hooks in SMGR to detect the Active Table, and
    > only recalculate the disk size of all the Active Tables.
    > For example, the smgrextend hook indicates that you allocate a new block
    > and the table need to be treated as Active Table.
    >
    > Do you have some better hook positions recommend to solve the above user
    > case?
    > Thanks in advance.
    >
    > Hubert
    >
    >
    >
    >
    >
    > On Tue, Jan 22, 2019 at 12:08 PM Hubert Zhang <hzhang@pivotal.io> wrote:
    >
    >> > 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()?
    >>> Yes, that's a bit awkward.
    >>
    >>
    >>  Hi Michael, we revisit the ReadBuffer hook and remove it in the latest
    >> patch.
    >> ReadBuffer hook is original used to do enforcement(e.g. out of diskquota
    >> limit) when query is loading data.
    >> We plan to put the enforcement work of running query to separate
    >> diskquota worker process.
    >> Let worker process to detect the backends to be cancelled and send SIGINT
    >> to these backends.
    >> So there is no need for ReadBuffer hook anymore.
    >>
    >> Our patch currently only contains smgr related hooks to catch the file
    >> change and get the Active Table list for diskquota extension.
    >>
    >> Thanks Hubert.
    >>
    >>
    >> On Mon, Jan 7, 2019 at 6:56 PM Haozhou Wang <hawang@pivotal.io> wrote:
    >>
    >>> Thanks very much for your comments.
    >>>
    >>> To the best of my knowledge, smgr is a layer that abstract the storage
    >>> operations. Therefore, it is a good place to control or collect information
    >>> the storage operations without touching the physical storage layer.
    >>> Moreover, smgr is coming with actual disk IO operation (not consider the
    >>> OS cache) for postgres. So we do not need to worry about the buffer
    >>> management in postgres.
    >>> It will make the purpose of hook is pure: a hook for actual disk IO.
    >>>
    >>> Regards,
    >>> Haozhou
    >>>
    >>> On Wed, Dec 26, 2018 at 1:56 PM Michael Paquier <michael@paquier.xyz>
    >>> wrote:
    >>>
    >>>> On Wed, Nov 21, 2018 at 09:47:44AM -0500, Robert Haas wrote:
    >>>> > +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.
    >>>>
    >>>> I personally don't mind making Postgres more pluggable, but I don't
    >>>> think that we actually need the extra ones proposed here at the layer
    >>>> of smgr, as smgr is already a layer designed to call an underlying set
    >>>> of APIs able to extend, unlink, etc. depending on the storage type.
    >>>>
    >>>> > 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()?
    >>>>
    >>>> Yes, that's a bit awkward.
    >>>> --
    >>>> Michael
    >>>
    >>>
    >>
    >> --
    >> Thanks
    >>
    >> Hubert Zhang
    >>
    >
    >
    > --
    > Thanks
    >
    > Hubert Zhang
    >
    
    
    -- 
    Thanks
    
    Hubert Zhang
    

13. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Andres Freund <andres@anarazel.de> — 2019-02-16T04:53:51Z

    Hi,
    
    
    On 2019-01-30 10:26:52 +0800, Hubert Zhang wrote:
    > Hi Michael, Robert
    > For you question about the hook position, I want to explain more about the
    > background why we want to introduce these hooks.
    > We wrote a diskquota extension <https://github.com/greenplum-db/diskquota>
    > [ ...]
    > On Tue, Jan 22, 2019 at 12:08 PM Hubert Zhang <hzhang@pivotal.io> wrote:
    > 
    > > > 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()?
    > >> Yes, that's a bit awkward.
    
    Please note that on PG lists the customary and desired style is to quote
    inline in messages rather than top-quote.
    
    Greetings,
    
    Andres Freund
    
    
    

14. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Hubert Zhang <hzhang@pivotal.io> — 2019-02-18T06:39:04Z

    Hi Andres
    
    On Sat, Feb 16, 2019 at 12:53 PM Andres Freund <andres@anarazel.de> wrote:
    
    > Hi,
    > On 2019-01-30 10:26:52 +0800, Hubert Zhang wrote:
    > > Hi Michael, Robert
    > > For you question about the hook position, I want to explain more about
    > the
    > > background why we want to introduce these hooks.
    > > We wrote a diskquota extension <
    > https://github.com/greenplum-db/diskquota>
    > > [ ...]
    > > On Tue, Jan 22, 2019 at 12:08 PM Hubert Zhang <hzhang@pivotal.io> wrote:
    > >
    > > > > 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()?
    > > >> Yes, that's a bit awkward.
    >
    > Please note that on PG lists the customary and desired style is to quote
    > inline in messages rather than top-quote.
    >
    > Greetings,
    >
    > Andres Freund
    >
    
    Thanks for your note. I will reply the above questions again.
    
    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()?
    
    
    We have removed the hook in ReadBufferExtended and only keep the hooks in
    SMGR.
    
    
    On Wed, Dec 26, 2018 at 1:56 PM Michael Paquier <michael@paquier.xyz> wrote:
    
    > On Wed, Nov 21, 2018 at 09:47:44AM -0500, Robert Haas wrote:
    > > +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.
    >
    > I personally don't mind making Postgres more pluggable, but I don't
    > think that we actually need the extra ones proposed here at the layer
    > of smgr, as smgr is already a layer designed to call an underlying set
    > of APIs able to extend, unlink, etc. depending on the storage type.
    >
    >
    The reason to use smgr as the hook position is as follows:
    These hooks will be used by diskquota extension, which needs to gather the
    current disk usage for each schema. We want to use hooks to detect the
    Active Tables, and only recalculate the disk size of all the Active Tables.
    As you mentioned, smgr is the layer to call underlying API to extend/unlink
    files. So it's also the place to detect the table size change, i.e. the
    Active Tables.
    For example, the smgrextend hook indicates that you allocate a new block
    and the corresponding table needs to be treated as Active Table.
    
    
    Besides, suppose we use smgr as hook position, I want to discuss the API
    passed to the hook function.
    Take smgrextend as example. The function interface of smgrextend is like
    that:
    ```
    void smgrextend
    (SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, char *buffer,
    bool skipFsync);
    ```
    So the hook api of smgrextend could have two main options.
    Hook API Option1
    ```
    typedef void (*smgrextend_hook_type)
    (RelFileNode smgr_rnode,ForkNumber forknum);
    ```
    Hook API Option 2
    ```
    typedef void (*smgrextend_hook_type)
    (SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,char *buffer,
    bool skipFsync);
    ```
    
    As for Option1. Since diskquota extension only needs the relfilenode
    information to detect the active tables, Option1 just pass the RelFileNode
    to the hook function. It's more clear and has semantic meaning.
    
    While Option 2 is to pass the original parameters to the hook functions
    without any filter. This is more general and let the different hook
    implementations to decide how to use these parameters.
    
    Option 1 also needs some additional work to handle smgrdounlinkall case,
    whose input parameter is the SMgrRelation list. We may need to palloc
    Relfilenode list and pfree it manually.
    smgrdounlinkall function interface:
    ```
    smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo, char
    *relstorages)
    ```
    
    Based on the assumption we use smgr as hook position, hook API option1 or
    option2 which is better?
    Or we could find some balanced API between option1 and option2?
    
    Again comments on other better hook positions are also appreciated!
    
    
    -- 
    Thanks
    
    Hubert Zhang
    

15. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Thomas Munro <thomas.munro@gmail.com> — 2019-07-07T22:47:06Z

    On Mon, Feb 18, 2019 at 7:39 PM Hubert Zhang <hzhang@pivotal.io> wrote:
    > Based on the assumption we use smgr as hook position, hook API option1 or option2 which is better?
    > Or we could find some balanced API between option1 and option2?
    >
    > Again comments on other better hook positions are also appreciated!
    
    Hi Hubert,
    
    The July Commitfest is now running, and this entry is in "needs
    review" state.  Could you please post a rebased patch?
    
    I have questions about how disk quotas should work and I think we'll
    probably eventually want more hooks than these, but simply adding
    these hooks so extensions can do whatever they want doesn't seem very
    risky for core.  I think it's highly likely that the hook signatures
    will have to change in future releases too, but that seems OK for such
    detailed internal hooks.  As for your question, my first reaction was
    that I preferred your option 1, because SMgrRelation seems quite
    private and there are no existing examples of that object being
    exposed to extensions.  But on reflection, other callbacks don't take
    such a mollycoddling approach.  So my vote is for option 2 "just pass
    all the arguments to the callback", which I understand to be the
    approach of patch you have posted.
    
    +    if (smgrcreate_hook)
    +    {
    +        (*smgrcreate_hook)(reln, forknum, isRedo);
    +    }
    
    Usually we don't use curlies for single line if branches.
    
    -- 
    Thomas Munro
    https://enterprisedb.com
    
    
    
    

16. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Hubert Zhang <hzhang@pivotal.io> — 2019-07-15T04:07:58Z

    Thanks, Thomas.
    
    On Mon, Jul 8, 2019 at 6:47 AM Thomas Munro <thomas.munro@gmail.com> wrote:
    
    > On Mon, Feb 18, 2019 at 7:39 PM Hubert Zhang <hzhang@pivotal.io> wrote:
    > > Based on the assumption we use smgr as hook position, hook API option1
    > or option2 which is better?
    > > Or we could find some balanced API between option1 and option2?
    > >
    > > Again comments on other better hook positions are also appreciated!
    >
    > Hi Hubert,
    >
    > The July Commitfest is now running, and this entry is in "needs
    > review" state.  Could you please post a rebased patch?
    >
    > I have questions about how disk quotas should work and I think we'll
    > probably eventually want more hooks than these, but simply adding
    > these hooks so extensions can do whatever they want doesn't seem very
    > risky for core.  I think it's highly likely that the hook signatures
    > will have to change in future releases too, but that seems OK for such
    > detailed internal hooks.  As for your question, my first reaction was
    > that I preferred your option 1, because SMgrRelation seems quite
    > private and there are no existing examples of that object being
    > exposed to extensions.  But on reflection, other callbacks don't take
    > such a mollycoddling approach.  So my vote is for option 2 "just pass
    > all the arguments to the callback", which I understand to be the
    > approach of patch you have posted.
    >
    > +    if (smgrcreate_hook)
    > +    {
    > +        (*smgrcreate_hook)(reln, forknum, isRedo);
    > +    }
    >
    > Usually we don't use curlies for single line if branches.
    >
    >
    I have rebased the patch to v4 and removed the unnecessary braces.
    As your comments, Options 2 is still used in patch v4.
    
    Agree that diskquota extension may use more hooks in future.
    Currently the behavior of diskquota extension is that we use smgr hooks to
    detect active tables and record them in the shared memory. Bgworkers of
    diskquota extension will read these active tables from shared memory and
    calculate the latest table size and sum them into the size of schema or
    role. If size of schema of role exceeds their quota limit, they will be put
    into a black list in shared memory. When a new query comes,
    ExecutorCheckPerms_hook will be used to check the black list the cancel the
    query if needed.
    
    -- 
    Thanks
    
    Hubert Zhang
    

17. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-09-25T19:54:35Z

    This patch no longer applies. Can you please rebase?
    
    -- 
    Álvaro Herrera                https://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
    
    
    
    

18. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Haozhou Wang <hawang@pivotal.io> — 2019-09-27T03:30:08Z

    Thanks Alvaro!
    I rebased this patch with the newest master branch. Attached the new
    patch disk_quota_hooks_v5.patch in the attachment.
    
    Regards,
    Haozhou
    
    On Thu, Sep 26, 2019 at 3:54 AM Alvaro Herrera <alvherre@2ndquadrant.com>
    wrote:
    
    > This patch no longer applies. Can you please rebase?
    >
    > --
    > Álvaro Herrera
    > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.2ndQuadrant.com_&d=DwIDAw&c=lnl9vOaLMzsy2niBC8-h_K-7QJuNJEsFrzdndhuJ3Sw&r=nGUCIcT5CVp6-pQcplYyagWnpAqoYm8YxWruds9UGI0&m=aNzGoEI15bAE-vAivY34BtG2WdgrVojH-B-kjvuXsYA&s=sT6zyiq4s8meelNuFw-lGD_mdvmUzv9zpVYWbFWusI0&e=
    > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
    >
    >
    >
    
    -- 
    Regards,
    Haozhou
    

19. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Michael Paquier <michael@paquier.xyz> — 2019-12-01T03:20:11Z

    On Fri, Sep 27, 2019 at 11:30:08AM +0800, Haozhou Wang wrote:
    > I rebased this patch with the newest master branch. Attached the new
    > patch disk_quota_hooks_v5.patch in the attachment.
    
    This again needs a rebase, so I have switched it as waiting on
    author.
    --
    Michael
    

20. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Haozhou Wang <hawang@pivotal.io> — 2019-12-02T06:39:43Z

    Hi Michael,
    
    Thank you very much for your email. I rebased the code with the newest
    master and attached it in the attachment.
    
    Thank you very much!
    
    Regards,
    Haozhou
    
    On Sun, Dec 1, 2019 at 11:20 AM Michael Paquier <michael@paquier.xyz> wrote:
    
    > On Fri, Sep 27, 2019 at 11:30:08AM +0800, Haozhou Wang wrote:
    > > I rebased this patch with the newest master branch. Attached the new
    > > patch disk_quota_hooks_v5.patch in the attachment.
    >
    > This again needs a rebase, so I have switched it as waiting on
    > author.
    > --
    > Michael
    >
    
    
    -- 
    Regards,
    Haozhou
    

21. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    David Steele <david@pgmasters.net> — 2020-03-24T16:00:36Z

    On 12/2/19 1:39 AM, Haozhou Wang wrote:
    > 
    > Thank you very much for your email. I rebased the code with the newest 
    > master and attached it in the attachment.
    
    This patch applies but no longer builds on Linux or Windows:
    https://travis-ci.org/github/postgresql-cfbot/postgresql/builds/666113036
    https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.85284
    
    The CF entry has been updated to Waiting on Author.
    
    Regards,
    -- 
    -David
    david@pgmasters.net
    
    
    
    

22. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Haozhou Wang <hawang@pivotal.io> — 2020-03-27T10:22:09Z

    Thanks David!
    
    We rebased this patch with the newest master.
    
    Thank you very much!
    
    Regards,
    Haozhou
    
    
    On Wed, Mar 25, 2020 at 12:00 AM David Steele <david@pgmasters.net> wrote:
    
    > On 12/2/19 1:39 AM, Haozhou Wang wrote:
    > >
    > > Thank you very much for your email. I rebased the code with the newest
    > > master and attached it in the attachment.
    >
    > This patch applies but no longer builds on Linux or Windows:
    >
    > https://urldefense.proofpoint.com/v2/url?u=https-3A__travis-2Dci.org_github_postgresql-2Dcfbot_postgresql_builds_666113036&d=DwICaQ&c=lnl9vOaLMzsy2niBC8-h_K-7QJuNJEsFrzdndhuJ3Sw&r=nGUCIcT5CVp6-pQcplYyagWnpAqoYm8YxWruds9UGI0&m=UfgeYnaIkXdE7XmY-eCIHV_ZCTEZqCAjXkPausd5qZI&s=zHJ5pgH_P4mMhZAuXtfTCGUB7lyo7dJ4VeoapRhIN4g&e=
    >
    > https://urldefense.proofpoint.com/v2/url?u=https-3A__ci.appveyor.com_project_postgresql-2Dcfbot_postgresql_build_1.0.85284&d=DwICaQ&c=lnl9vOaLMzsy2niBC8-h_K-7QJuNJEsFrzdndhuJ3Sw&r=nGUCIcT5CVp6-pQcplYyagWnpAqoYm8YxWruds9UGI0&m=UfgeYnaIkXdE7XmY-eCIHV_ZCTEZqCAjXkPausd5qZI&s=WmyVNF-rItz10LoeLtY-Xu6l-iV3uwI_U-R9lR7cKjc&e=
    >
    > The CF entry has been updated to Waiting on Author.
    >
    > Regards,
    > --
    > -David
    > david@pgmasters.net
    >
    
    
    -- 
    Regards,
    Haozhou
    

23. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Daniel Gustafsson <daniel@yesql.se> — 2020-07-01T08:36:43Z

    > On 27 Mar 2020, at 11:22, Haozhou Wang <hawang@pivotal.io> wrote:
    
    > We rebased this patch with the newest master.
    
    This patch now fails to build according to Travis:
    
    smgr.c: In function ‘smgrtruncate’:
    smgr.c:578:47: error: passing argument 4 of ‘smgrtruncate_hook’ makes integer from pointer without a cast [-Werror=int-conversion]
       (*smgrtruncate_hook)(reln, forknum, nforks, nblocks);
                                                   ^
    smgr.c:578:47: note: expected ‘BlockNumber {aka unsigned int}’ but argument is of type ‘BlockNumber * {aka unsigned int *}’
    
    
    The warning is also present in the Windows build:
    
    src/backend/storage/smgr/smgr.c(578): warning C4047: 'function' : 'BlockNumber' differs in levels of indirection from 'BlockNumber *' [C:\projects\postgresql\postgres.vcxproj]
    src/backend/storage/smgr/smgr.c(578): warning C4024: 'smgrtruncate_hook' : different types for formal and actual parameter 4 [C:\projects\postgresql\postgres.vcxproj]
    2 Warning(s)
    
    Marking the patch as Waiting for Author. 
    
    cheers ./daniel
    
    
    

24. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Daniel Gustafsson <daniel@yesql.se> — 2020-07-31T19:35:18Z

    > On 1 Jul 2020, at 10:36, Daniel Gustafsson <daniel@yesql.se> wrote:
    > 
    >> On 27 Mar 2020, at 11:22, Haozhou Wang <hawang@pivotal.io> wrote:
    > 
    >> We rebased this patch with the newest master.
    > 
    > This patch now fails to build according to Travis:
    > 
    > smgr.c: In function ‘smgrtruncate’:
    > smgr.c:578:47: error: passing argument 4 of ‘smgrtruncate_hook’ makes integer from pointer without a cast [-Werror=int-conversion]
    >   (*smgrtruncate_hook)(reln, forknum, nforks, nblocks);
    >                                               ^
    > smgr.c:578:47: note: expected ‘BlockNumber {aka unsigned int}’ but argument is of type ‘BlockNumber * {aka unsigned int *}’
    > 
    > 
    > The warning is also present in the Windows build:
    > 
    > src/backend/storage/smgr/smgr.c(578): warning C4047: 'function' : 'BlockNumber' differs in levels of indirection from 'BlockNumber *' [C:\projects\postgresql\postgres.vcxproj]
    > src/backend/storage/smgr/smgr.c(578): warning C4024: 'smgrtruncate_hook' : different types for formal and actual parameter 4 [C:\projects\postgresql\postgres.vcxproj]
    > 2 Warning(s)
    > 
    > Marking the patch as Waiting for Author. 
    
    As the thread has stalled and the above compilation issue hasn't been
    addressed, I'm marking this entry Returned with Feedback.  Feel free to open a
    new entry when there is a fixed patch.
    
    cheers ./daniel
    
    
    

25. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Xing Guo <higuoxing@gmail.com> — 2024-02-26T11:42:18Z

    On Mon, Feb 26, 2024 at 7:27 PM Daniel Gustafsson <daniel@yesql.se> wrote:
    >
    > > On 1 Jul 2020, at 10:36, Daniel Gustafsson <daniel@yesql.se> wrote:
    > >
    > >> On 27 Mar 2020, at 11:22, Haozhou Wang <hawang@pivotal.io> wrote:
    > >
    > >> We rebased this patch with the newest master.
    > >
    > > This patch now fails to build according to Travis:
    > >
    > > smgr.c: In function ‘smgrtruncate’:
    > > smgr.c:578:47: error: passing argument 4 of ‘smgrtruncate_hook’ makes integer from pointer without a cast [-Werror=int-conversion]
    > >   (*smgrtruncate_hook)(reln, forknum, nforks, nblocks);
    > >                                               ^
    > > smgr.c:578:47: note: expected ‘BlockNumber {aka unsigned int}’ but argument is of type ‘BlockNumber * {aka unsigned int *}’
    > >
    > >
    > > The warning is also present in the Windows build:
    > >
    > > src/backend/storage/smgr/smgr.c(578): warning C4047: 'function' : 'BlockNumber' differs in levels of indirection from 'BlockNumber *' [C:\projects\postgresql\postgres.vcxproj]
    > > src/backend/storage/smgr/smgr.c(578): warning C4024: 'smgrtruncate_hook' : different types for formal and actual parameter 4 [C:\projects\postgresql\postgres.vcxproj]
    > > 2 Warning(s)
    > >
    > > Marking the patch as Waiting for Author.
    >
    > As the thread has stalled and the above compilation issue hasn't been
    > addressed, I'm marking this entry Returned with Feedback.  Feel free to open a
    > new entry when there is a fixed patch.
    
    Hi,
    
    Looks that many hackers are happy with the original patch except that
    the original patch needs a simple rebase, though it has been 3 years.
    Shall we push forward this patch so that it can benefit extensions not
    only diskquota?
    
    The attachment is a rebased version of the original patch.
    
    >
    > cheers ./daniel
    >
    >
    >
    

26. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Stephen Frost <sfrost@snowman.net> — 2024-02-26T22:38:50Z

    Greetings,
    
    * Xing Guo (higuoxing@gmail.com) wrote:
    > Looks that many hackers are happy with the original patch except that
    > the original patch needs a simple rebase, though it has been 3 years.
    
    I'm not completely against the idea of adding these hooks, but I have to
    say that it's unfortunate to imagine having to use an extension for
    something like quotas as it's really something that would ideally be in
    core.
    
    > Shall we push forward this patch so that it can benefit extensions not
    > only diskquota?
    
    Would be great to hear about other use-cases for these hooks, which
    would also help us be comfortable that these are the right hooks to
    introduce with the correct arguments.  What are the other extensions
    that you're referring to here..?
    
    Thanks,
    
    Stephen
    

27. Re: Control your disk usage in PG: Introduction to Disk Quota Extension

    Xing Guo <higuoxing@gmail.com> — 2024-02-27T01:41:45Z

    On Tue, Feb 27, 2024 at 6:38 AM Stephen Frost <sfrost@snowman.net> wrote:
    >
    > Greetings,
    >
    > * Xing Guo (higuoxing@gmail.com) wrote:
    > > Looks that many hackers are happy with the original patch except that
    > > the original patch needs a simple rebase, though it has been 3 years.
    >
    > I'm not completely against the idea of adding these hooks, but I have to
    > say that it's unfortunate to imagine having to use an extension for
    > something like quotas as it's really something that would ideally be in
    > core.
    >
    > > Shall we push forward this patch so that it can benefit extensions not
    > > only diskquota?
    >
    > Would be great to hear about other use-cases for these hooks, which
    > would also help us be comfortable that these are the right hooks to
    > introduce with the correct arguments.  What are the other extensions
    > that you're referring to here..?
    
    Sorry, we don't have another concrete extension that utilizes the
    smgrcreate / smgrtruncate / smgrdounlinkall hooks right now. But we
    have a transparent data encryption extension that utilizes the
    smgrread and smgrwrite hooks to mutate the read buffer and write
    buffer in place to perform file level encryption and decryption. I
    think smgrwrite / smgrread / smgrcreate / smgrtruncate and
    smgrdounlinkall hooks fall into the same catagory and these hooks are
    different from the recent proposed storage manager API hooks[1].
    Probably it is worth starting a new thread and discussing them.
    
    [1]: https://www.postgresql.org/message-id/flat/CAEze2WgMySu2suO_TLvFyGY3URa4mAx22WeoEicnK%3DPCNWEMrA%40mail.gmail.com
    
    > Thanks,
    >
    > Stephen
    
    Best Regards,
    Xing