Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1glnN2-0001YO-Aa for pgsql-hackers@arkaria.postgresql.org; Tue, 22 Jan 2019 04:09:04 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1glnN0-0001mu-ON for pgsql-hackers@arkaria.postgresql.org; Tue, 22 Jan 2019 04:09:02 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1glnN0-0001md-9E for pgsql-hackers@lists.postgresql.org; Tue, 22 Jan 2019 04:09:02 +0000 Received: from mx0b-00296801.pphosted.com ([148.163.153.148]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1glnMu-0001wN-98 for pgsql-hackers@postgresql.org; Tue, 22 Jan 2019 04:08:57 +0000 Received: from pps.filterd (m0114586.ppops.net [127.0.0.1]) by mx0b-00296801.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x0M46Z81013891 for ; Tue, 22 Jan 2019 04:08:55 GMT Received: from mail-lj1-f199.google.com (mail-lj1-f199.google.com [209.85.208.199]) by mx0b-00296801.pphosted.com with ESMTP id 2q3wafsk9b-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Tue, 22 Jan 2019 04:08:54 +0000 Received: by mail-lj1-f199.google.com with SMTP id v24-v6so5638674ljj.10 for ; Mon, 21 Jan 2019 20:08:54 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hlg14F+/M1QumrKpAcvCvmniNng3Y/ui5QkGaLcX6tU=; b=ih4zc8hCUzHtmT3+j3JhosbVCruvNOp2/TlEZACPfyt/nXloVXP+CbtOLpVXYF2VM0 ckal0i0tSaeHtVXzxus7RYH06Jos6z+iTW9TD++QQNlSFaG8VpCT7eOVzWj1Agu1Gggy RttumiKE+sNiK6lBzeiQQVgZbmW65CpvzoRsrGLDWpkGHXEW3Ka5ZAtzf5I2DJvGnUeu 8dhvTvAIb5XRTP15MkuUJEb+jT2h7VkhI9k7kepyEteQpZS5k0HjJyM8Q+diDtaLxsDJ O7+xQ37kw+tXRBZK+w4aWRMJRGMS2RNAn/VbD2UtbinOgQarp6Z/tebIjMJ9FnMkkMuB Qduw== X-Gm-Message-State: AJcUukcA88sYzYsL2W2jv+3OL97tyr8yqXTjxk8uAhTgPoAqyHff2K3w AMSmD2Z4rFFFtjz8fgNT6dzZQVDfkfQMPSnppr9rJaiUxnxwfqMO8DVpT8KrBYq9rwUC77x587g 8vF1ltK8oiKzuDO+ECetee8ATXFeR2r+luxyTLP9ZuUAhlhFPilviMzWLwqf0OzYdDg== X-Received: by 2002:a19:94d5:: with SMTP id o82mr18497942lfk.155.1548130132753; Mon, 21 Jan 2019 20:08:52 -0800 (PST) X-Google-Smtp-Source: ALg8bN5Hkr/qEyiC5NkcDagDgq6DQPZ5WG/rZjRGB456yh3BdmPwPXGDDCi8ztbfi6QgAJL0RYsxNkkmjCt7LMQ95F8= X-Received: by 2002:a19:94d5:: with SMTP id o82mr18497927lfk.155.1548130132395; Mon, 21 Jan 2019 20:08:52 -0800 (PST) MIME-Version: 1.0 References: <20181226055647.GK2234@paquier.xyz> In-Reply-To: From: Hubert Zhang Date: Tue, 22 Jan 2019 12:08:46 +0800 Message-ID: Subject: Re: Control your disk usage in PG: Introduction to Disk Quota Extension To: Haozhou Wang Cc: Michael Paquier , Robert Haas , "pgsql-hackers@postgresql.org" , Tomas Vondra Content-Type: multipart/mixed; boundary="0000000000009bcb2d05800422d9" X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-01-22_02:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=1 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1810050000 definitions=main-1901220033 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000009bcb2d05800422d9 Content-Type: multipart/alternative; boundary="0000000000009bcb2a05800422d7" --0000000000009bcb2a05800422d7 Content-Type: text/plain; charset="UTF-8" > > > 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 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 > 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 --0000000000009bcb2a05800422d7 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
> For this particular purp= ose, I don't immediately see why you need a
> hook in both places.=C2=A0 If Rea= dBuffer is called with P_NEW, aren't we
> guaranteed to end up in smgrextend()?=
Yes, that's a bit awkward.
=C2=A0
=C2=A0Hi Michael, we revisit the ReadBuffer hook and remo= ve 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 diskq= uota worker process.
Let worker process to detect the backends to= be cancelled and send SIGINT to these backends.
So there is no n= eed for ReadBuffer hook anymore.

Our patch current= ly only contains smgr related hooks to catch the file change and get the Ac= tive Table list for diskquota extension.

Thanks Hu= bert.


On Mon, Jan 7, 2019 at 6:56 PM Haozhou Wang <<= a href=3D"mailto:hawang@pivotal.io">hawang@pivotal.io> wrote:
Thanks very much for your comments.

To the best of my knowledge, smgr is a layer that abst= ract the storage operations. Therefore, it is a good place to control or co= llect=C2=A0information the storage operations without touching the physical= storage layer.
Moreover, smgr is coming with actual disk IO oper= ation (not consider the OS cache) for postgres. So we do not need to worry = about the buffer management in postgres.=C2=A0
It will make the p= urpose 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 W= ed, 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.=C2=A0 The hook name sho= uld
> describe the place from which it is called, not the purpose for which<= br> > one imagines that it will be used, because somebody else might imagine=
> another use.=C2=A0 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.=C2=A0 If ReadBuffer is called with P_NEW, aren= 9;t we
> guaranteed to end up in smgrextend()?

Yes, that's a bit awkward.
--
Michael


--
Thanks

Huber= t Zhang
--0000000000009bcb2a05800422d7-- --0000000000009bcb2d05800422d9 Content-Type: application/octet-stream; name="disk_quota_hooks_v3.patch" Content-Disposition: attachment; filename="disk_quota_hooks_v3.patch" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_jr78mfkv0 RnJvbSA0YWFjMmM2OWNiYjAwMDFiZTVlOTViNmQ0ODJmYzNiMDMyMjA4ZjgxIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBIdWJlcnQgWmhhbmcgPGh1YmVydHpoYW5nQGFwYWNoZS5vcmc+ CkRhdGU6IFR1ZSwgMjIgSmFuIDIwMTkgMTE6NDY6MjkgKzA4MDAKU3ViamVjdDogW1BBVENIXSBB ZGQgc21ncipfaG9vaygpIGhvb2sgcG9pbnRzIHRvIGV4dGVuZCB0aGUgbG9naWMgb2Ygc3RvcmFn ZQogbWFuYWdlbWVudC4KCkNvLWF1dGhvcmVkLWJ5OiBIYW96aG91IFdhbmcgPGhhd2FuZ0BwaXZv dGFsLmlvPgpDby1hdXRob3JlZC1ieTogSHViZXJ0IFpoYW5nIDxoemhhbmdAcGl2b3RhbC5pbz4K Q28tYXV0aG9yZWQtYnk6IEhhbyBXdSA8Z2ZwaG9lbml4NzhAZ21haWwuY29tPgotLS0KIHNyYy9i YWNrZW5kL3N0b3JhZ2Uvc21nci9zbWdyLmMgfCAzMCArKysrKysrKysrKysrKysrKysrKysrKysr KysrKysKIHNyYy9pbmNsdWRlL3N0b3JhZ2Uvc21nci5oICAgICAgfCAxOCArKysrKysrKysrKysr KysrKysKIDIgZmlsZXMgY2hhbmdlZCwgNDggaW5zZXJ0aW9ucygrKQoKZGlmZiAtLWdpdCBhL3Ny Yy9iYWNrZW5kL3N0b3JhZ2Uvc21nci9zbWdyLmMgYi9zcmMvYmFja2VuZC9zdG9yYWdlL3NtZ3Iv c21nci5jCmluZGV4IDBjMGJiYTRhYjMuLmI0YmE5ZDlhNGQgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNr ZW5kL3N0b3JhZ2Uvc21nci9zbWdyLmMKKysrIGIvc3JjL2JhY2tlbmQvc3RvcmFnZS9zbWdyL3Nt Z3IuYwpAQCAtOTAsNiArOTAsMTYgQEAgc3RhdGljIGNvbnN0IGZfc21nciBzbWdyc3dbXSA9IHsK IAogc3RhdGljIGNvbnN0IGludCBOU21nciA9IGxlbmd0aG9mKHNtZ3Jzdyk7CiAKKy8qCisgKiBI b29rIGZvciBwbHVnaW5zIHRvIGV4dGVuZCBzbWdyIGZ1bmN0aW9ucy4KKyAqIGZvciBleGFtcGxl LCBjb2xsZWN0IHN0YXRpc3RpY3MgZnJvbSBzbWdyIGZ1bmN0aW9ucworICogdmlhIHJlY29yZGlu ZyB0aGUgYWN0aXZlIHJlbGZpbGVub2RlIGluZm9ybWF0aW9uLgorICovCitzbWdyY3JlYXRlX2hv b2tfdHlwZSBzbWdyY3JlYXRlX2hvb2sgPSBOVUxMOworc21ncmV4dGVuZF9ob29rX3R5cGUgc21n cmV4dGVuZF9ob29rID0gTlVMTDsKK3NtZ3J0cnVuY2F0ZV9ob29rX3R5cGUgc21ncnRydW5jYXRl X2hvb2sgPSBOVUxMOworc21ncmRvdW5saW5rYWxsX2hvb2tfdHlwZSBzbWdyZG91bmxpbmthbGxf aG9vayA9IE5VTEw7CisKIAogLyoKICAqIEVhY2ggYmFja2VuZCBoYXMgYSBoYXNodGFibGUgdGhh dCBzdG9yZXMgYWxsIGV4dGFudCBTTWdyUmVsYXRpb24gb2JqZWN0cy4KQEAgLTM5Nyw2ICs0MDcs MTEgQEAgc21ncmNyZWF0ZShTTWdyUmVsYXRpb24gcmVsbiwgRm9ya051bWJlciBmb3JrbnVtLCBi b29sIGlzUmVkbykKIAlpZiAoaXNSZWRvICYmIHJlbG4tPm1kX251bV9vcGVuX3NlZ3NbZm9ya251 bV0gPiAwKQogCQlyZXR1cm47CiAKKwlpZiAoc21ncmNyZWF0ZV9ob29rKQorCXsKKwkJKCpzbWdy Y3JlYXRlX2hvb2spKHJlbG4sIGZvcmtudW0sIGlzUmVkbyk7CisJfQorCiAJLyoKIAkgKiBXZSBt YXkgYmUgdXNpbmcgdGhlIHRhcmdldCB0YWJsZSBzcGFjZSBmb3IgdGhlIGZpcnN0IHRpbWUgaW4g dGhpcwogCSAqIGRhdGFiYXNlLCBzbyBjcmVhdGUgYSBwZXItZGF0YWJhc2Ugc3ViZGlyZWN0b3J5 IGlmIG5lZWRlZC4KQEAgLTQ5Miw2ICs1MDcsMTEgQEAgc21ncmRvdW5saW5rYWxsKFNNZ3JSZWxh dGlvbiAqcmVscywgaW50IG5yZWxzLCBib29sIGlzUmVkbykKIAlpZiAobnJlbHMgPT0gMCkKIAkJ cmV0dXJuOwogCisJaWYgKHNtZ3Jkb3VubGlua2FsbF9ob29rKQorCXsKKwkJKCpzbWdyZG91bmxp bmthbGxfaG9vaykocmVscywgbnJlbHMsIGlzUmVkbyk7CisJfQorCiAJLyoKIAkgKiBjcmVhdGUg YW4gYXJyYXkgd2hpY2ggY29udGFpbnMgYWxsIHJlbGF0aW9ucyB0byBiZSBkcm9wcGVkLCBhbmQg Y2xvc2UKIAkgKiBlYWNoIHJlbGF0aW9uJ3MgZm9ya3MgYXQgdGhlIHNtZ3IgbGV2ZWwgd2hpbGUg YXQgaXQKQEAgLTYxNSw2ICs2MzUsMTEgQEAgdm9pZAogc21ncmV4dGVuZChTTWdyUmVsYXRpb24g cmVsbiwgRm9ya051bWJlciBmb3JrbnVtLCBCbG9ja051bWJlciBibG9ja251bSwKIAkJICAgY2hh ciAqYnVmZmVyLCBib29sIHNraXBGc3luYykKIHsKKwlpZiAoc21ncmV4dGVuZF9ob29rKQorCXsK KwkJKCpzbWdyZXh0ZW5kX2hvb2spKHJlbG4sIGZvcmtudW0sIGJsb2NrbnVtLCBidWZmZXIsIHNr aXBGc3luYyk7CisJfQorCiAJc21ncnN3W3JlbG4tPnNtZ3Jfd2hpY2hdLnNtZ3JfZXh0ZW5kKHJl bG4sIGZvcmtudW0sIGJsb2NrbnVtLAogCQkJCQkJCQkJCSBidWZmZXIsIHNraXBGc3luYyk7CiB9 CkBAIC02OTgsNiArNzIzLDExIEBAIHNtZ3JuYmxvY2tzKFNNZ3JSZWxhdGlvbiByZWxuLCBGb3Jr TnVtYmVyIGZvcmtudW0pCiB2b2lkCiBzbWdydHJ1bmNhdGUoU01nclJlbGF0aW9uIHJlbG4sIEZv cmtOdW1iZXIgZm9ya251bSwgQmxvY2tOdW1iZXIgbmJsb2NrcykKIHsKKwlpZiAoc21ncnRydW5j YXRlX2hvb2spCisJeworCQkoKnNtZ3J0cnVuY2F0ZV9ob29rKShyZWxuLCBmb3JrbnVtLCBuYmxv Y2tzKTsKKwl9CisKIAkvKgogCSAqIEdldCByaWQgb2YgYW55IGJ1ZmZlcnMgZm9yIHRoZSBhYm91 dC10by1iZS1kZWxldGVkIGJsb2Nrcy4gYnVmbWdyIHdpbGwKIAkgKiBqdXN0IGRyb3AgdGhlbSB3 aXRob3V0IGJvdGhlcmluZyB0byB3cml0ZSB0aGUgY29udGVudHMuCmRpZmYgLS1naXQgYS9zcmMv aW5jbHVkZS9zdG9yYWdlL3NtZ3IuaCBiL3NyYy9pbmNsdWRlL3N0b3JhZ2Uvc21nci5oCmluZGV4 IDgyMGQwOGVkNGUuLjU0MjY1NDNiNWMgMTAwNjQ0Ci0tLSBhL3NyYy9pbmNsdWRlL3N0b3JhZ2Uv c21nci5oCisrKyBiL3NyYy9pbmNsdWRlL3N0b3JhZ2Uvc21nci5oCkBAIC0xNDQsNSArMTQ0LDIz IEBAIGV4dGVybiB2b2lkIFJlbWVtYmVyRnN5bmNSZXF1ZXN0KFJlbEZpbGVOb2RlIHJub2RlLCBG b3JrTnVtYmVyIGZvcmtudW0sCiBleHRlcm4gdm9pZCBGb3JnZXRSZWxhdGlvbkZzeW5jUmVxdWVz dHMoUmVsRmlsZU5vZGUgcm5vZGUsIEZvcmtOdW1iZXIgZm9ya251bSk7CiBleHRlcm4gdm9pZCBG b3JnZXREYXRhYmFzZUZzeW5jUmVxdWVzdHMoT2lkIGRiaWQpOwogZXh0ZXJuIHZvaWQgRHJvcFJl bGF0aW9uRmlsZXMoUmVsRmlsZU5vZGUgKmRlbHJlbHMsIGludCBuZGVscmVscywgYm9vbCBpc1Jl ZG8pOworLyoKKyAqIEhvb2sgZm9yIHBsdWdpbnMgdG8gZXh0ZW5kIHNtZ3IgZnVuY3Rpb25zLgor ICogZm9yIGV4YW1wbGUsIGNvbGxlY3Qgc3RhdGlzdGljcyBmcm9tIHNtZ3IgZnVuY3Rpb25zCisg KiB2aWEgcmVjb3JkaW5nIHRoZSBhY3RpdmUgcmVsZmlsZW5vZGUgaW5mb3JtYXRpb24uCisgKi8K K3R5cGVkZWYgdm9pZCAoKnNtZ3JjcmVhdGVfaG9va190eXBlKShTTWdyUmVsYXRpb24gcmVsbiwg Rm9ya051bWJlciBmb3JrbnVtLAorCQkJCQkJCQkJIGJvb2wgaXNSZWRvKTsKK2V4dGVybiBQR0RM TElNUE9SVCBzbWdyY3JlYXRlX2hvb2tfdHlwZSBzbWdyY3JlYXRlX2hvb2s7Cit0eXBlZGVmIHZv aWQgKCpzbWdyZXh0ZW5kX2hvb2tfdHlwZSkoU01nclJlbGF0aW9uIHJlbG4sIEZvcmtOdW1iZXIg Zm9ya251bSwKKwkJCQkJCQkJCSBCbG9ja051bWJlciBibG9ja251bSwKKwkJCQkJCQkJCSBjaGFy ICpidWZmZXIsIGJvb2wgc2tpcEZzeW5jKTsKK2V4dGVybiBQR0RMTElNUE9SVCBzbWdyZXh0ZW5k X2hvb2tfdHlwZSBzbWdyZXh0ZW5kX2hvb2s7Cit0eXBlZGVmIHZvaWQgKCpzbWdydHJ1bmNhdGVf aG9va190eXBlKShTTWdyUmVsYXRpb24gcmVsbiwgRm9ya051bWJlciBmb3JrbnVtLAorCQkJCQkJ CQkJICAgQmxvY2tOdW1iZXIgbmJsb2Nrcyk7CitleHRlcm4gUEdETExJTVBPUlQgc21ncnRydW5j YXRlX2hvb2tfdHlwZSBzbWdydHJ1bmNhdGVfaG9vazsKK3R5cGVkZWYgdm9pZCAoKnNtZ3Jkb3Vu bGlua2FsbF9ob29rX3R5cGUpKFNNZ3JSZWxhdGlvbiAqcmVscywgaW50IG5yZWxzLAorCQkJCQkJ CQkJCSAgYm9vbCBpc1JlZG8pOworZXh0ZXJuIFBHRExMSU1QT1JUIHNtZ3Jkb3VubGlua2FsbF9o b29rX3R5cGUgc21ncmRvdW5saW5rYWxsX2hvb2s7CiAKICNlbmRpZgkJCQkJCQkvKiBTTUdSX0gg Ki8KLS0gCjIuMTEuMCAoQXBwbGUgR2l0LTgxKQoK --0000000000009bcb2d05800422d9--