public inbox for [email protected]
help / color / mirror / Atom feedFrom: Hubert Zhang <[email protected]>
To: Haozhou Wang <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Tomas Vondra <[email protected]>
Subject: Re: Control your disk usage in PG: Introduction to Disk Quota Extension
Date: Tue, 22 Jan 2019 12:08:46 +0800
Message-ID: <CAB0yrek4pbh-x7vYmKh70YRfbXhd7k1Ceqwhyy35jUYdYRrXAg@mail.gmail.com> (raw)
In-Reply-To: <CAL_NLpKMknkagF0WzgjeJAjAL=ZosoNifbnqo7YgbtcYgSBepA@mail.gmail.com>
References: <CAB0yre=RP_ho6Bq4cV23ELKxRcfhV2Yqrb1zHp0RfUPEWCnBRw@mail.gmail.com>
<[email protected]>
<CAB0yrenHV2iw4hQEnHGkTTpXZN8jMUJNtsCV4=BmWckjnn16Mg@mail.gmail.com>
<CAL_NLp+jLqn9P+hzy1vYSqoUuCsJ4AtTF4QYH5=950rB9S3dSw@mail.gmail.com>
<CA+Tgmoa9m74nhU8LpPfsiW-WS2MvWkipCEn4VW40kFrMMkbhKw@mail.gmail.com>
<[email protected]>
<CAL_NLpKMknkagF0WzgjeJAjAL=ZosoNifbnqo7YgbtcYgSBepA@mail.gmail.com>
>
> > 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 <[email protected]> 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 <[email protected]>
> 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
Attachments:
[application/octet-stream] disk_quota_hooks_v3.patch (4.0K, ../CAB0yrek4pbh-x7vYmKh70YRfbXhd7k1Ceqwhyy35jUYdYRrXAg@mail.gmail.com/3-disk_quota_hooks_v3.patch)
download | inline diff:
From 4aac2c69cbb0001be5e95b6d482fc3b032208f81 Mon Sep 17 00:00:00 2001
From: Hubert Zhang <[email protected]>
Date: Tue, 22 Jan 2019 11:46:29 +0800
Subject: [PATCH] Add smgr*_hook() hook points to extend the logic of storage
management.
Co-authored-by: Haozhou Wang <[email protected]>
Co-authored-by: Hubert Zhang <[email protected]>
Co-authored-by: Hao Wu <[email protected]>
---
src/backend/storage/smgr/smgr.c | 30 ++++++++++++++++++++++++++++++
src/include/storage/smgr.h | 18 ++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index 0c0bba4ab3..b4ba9d9a4d 100644
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -90,6 +90,16 @@ static const f_smgr smgrsw[] = {
static const int NSmgr = lengthof(smgrsw);
+/*
+ * Hook for plugins to extend smgr functions.
+ * for example, collect statistics from smgr functions
+ * via recording the active relfilenode information.
+ */
+smgrcreate_hook_type smgrcreate_hook = NULL;
+smgrextend_hook_type smgrextend_hook = NULL;
+smgrtruncate_hook_type smgrtruncate_hook = NULL;
+smgrdounlinkall_hook_type smgrdounlinkall_hook = NULL;
+
/*
* Each backend has a hashtable that stores all extant SMgrRelation objects.
@@ -397,6 +407,11 @@ smgrcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo)
if (isRedo && reln->md_num_open_segs[forknum] > 0)
return;
+ if (smgrcreate_hook)
+ {
+ (*smgrcreate_hook)(reln, forknum, isRedo);
+ }
+
/*
* We may be using the target table space for the first time in this
* database, so create a per-database subdirectory if needed.
@@ -492,6 +507,11 @@ smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo)
if (nrels == 0)
return;
+ if (smgrdounlinkall_hook)
+ {
+ (*smgrdounlinkall_hook)(rels, nrels, isRedo);
+ }
+
/*
* create an array which contains all relations to be dropped, and close
* each relation's forks at the smgr level while at it
@@ -615,6 +635,11 @@ void
smgrextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
char *buffer, bool skipFsync)
{
+ if (smgrextend_hook)
+ {
+ (*smgrextend_hook)(reln, forknum, blocknum, buffer, skipFsync);
+ }
+
smgrsw[reln->smgr_which].smgr_extend(reln, forknum, blocknum,
buffer, skipFsync);
}
@@ -698,6 +723,11 @@ smgrnblocks(SMgrRelation reln, ForkNumber forknum)
void
smgrtruncate(SMgrRelation reln, ForkNumber forknum, BlockNumber nblocks)
{
+ if (smgrtruncate_hook)
+ {
+ (*smgrtruncate_hook)(reln, forknum, nblocks);
+ }
+
/*
* Get rid of any buffers for the about-to-be-deleted blocks. bufmgr will
* just drop them without bothering to write the contents.
diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h
index 820d08ed4e..5426543b5c 100644
--- a/src/include/storage/smgr.h
+++ b/src/include/storage/smgr.h
@@ -144,5 +144,23 @@ extern void RememberFsyncRequest(RelFileNode rnode, ForkNumber forknum,
extern void ForgetRelationFsyncRequests(RelFileNode rnode, ForkNumber forknum);
extern void ForgetDatabaseFsyncRequests(Oid dbid);
extern void DropRelationFiles(RelFileNode *delrels, int ndelrels, bool isRedo);
+/*
+ * Hook for plugins to extend smgr functions.
+ * for example, collect statistics from smgr functions
+ * via recording the active relfilenode information.
+ */
+typedef void (*smgrcreate_hook_type)(SMgrRelation reln, ForkNumber forknum,
+ bool isRedo);
+extern PGDLLIMPORT smgrcreate_hook_type smgrcreate_hook;
+typedef void (*smgrextend_hook_type)(SMgrRelation reln, ForkNumber forknum,
+ BlockNumber blocknum,
+ char *buffer, bool skipFsync);
+extern PGDLLIMPORT smgrextend_hook_type smgrextend_hook;
+typedef void (*smgrtruncate_hook_type)(SMgrRelation reln, ForkNumber forknum,
+ BlockNumber nblocks);
+extern PGDLLIMPORT smgrtruncate_hook_type smgrtruncate_hook;
+typedef void (*smgrdounlinkall_hook_type)(SMgrRelation *rels, int nrels,
+ bool isRedo);
+extern PGDLLIMPORT smgrdounlinkall_hook_type smgrdounlinkall_hook;
#endif /* SMGR_H */
--
2.11.0 (Apple Git-81)
view thread (24+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Control your disk usage in PG: Introduction to Disk Quota Extension
In-Reply-To: <CAB0yrek4pbh-x7vYmKh70YRfbXhd7k1Ceqwhyy35jUYdYRrXAg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox