MIME-Version: 1.0
References: 
 <CAB0yre=RP_ho6Bq4cV23ELKxRcfhV2Yqrb1zHp0RfUPEWCnBRw@mail.gmail.com>
 <eface0f387a6f7530862ef40e3e3e55ac915ea83.camel@2ndquadrant.com>
 <CAB0yrenHV2iw4hQEnHGkTTpXZN8jMUJNtsCV4=BmWckjnn16Mg@mail.gmail.com>
 <CAL_NLp+jLqn9P+hzy1vYSqoUuCsJ4AtTF4QYH5=950rB9S3dSw@mail.gmail.com>
In-Reply-To: 
 <CAL_NLp+jLqn9P+hzy1vYSqoUuCsJ4AtTF4QYH5=950rB9S3dSw@mail.gmail.com>
From: Robert Haas <robertmhaas@gmail.com>
Date: Wed, 21 Nov 2018 09:47:44 -0500
Message-ID: 
 <CA+Tgmoa9m74nhU8LpPfsiW-WS2MvWkipCEn4VW40kFrMMkbhKw@mail.gmail.com>
Subject: Re: Control your disk usage in PG: Introduction to Disk Quota
 Extension
To: hawang@pivotal.io
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
	Tomas Vondra <tomas.vondra@2ndquadrant.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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 a=
re needed for disk quota extension.
> There are two hooks.
> One is SmgrStat_hook. It's used to perform ad-hoc logic in storage when d=
oing smgr create/extend/truncate in general. As for disk quota extension, t=
his hook is used to detect active tables(new created tables, tables extendi=
ng new blocks, or tables being truncated)
> The other is BufferExtendCheckPerms_hook. It's used to perform ad-hoc log=
ic when buffer extend a new block. Since ReadBufferExtended is a hot functi=
on, we call this hook only when blockNum =3D=3D P_NEW. As  for disk quota e=
xtension, this hook is used to do query enforcement during the query is loa=
ding 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()?

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