Content-Type: multipart/mixed; boundary="------------4YJ5iHbwSbs2N8GgMZOGqyJk"
Message-ID: <f08fee25-1dac-b68b-19af-f4b55ceb3802@gmail.com>
Date: Mon, 6 Mar 2023 08:33:15 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.8.0
Subject: Re: Reconcile stats in find_tabstat_entry() and get rid of
 PgStat_BackendFunctionEntry
To: Andres Freund <andres@anarazel.de>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
 pgsql-hackers@lists.postgresql.org
References: <e4fe4710-09ff-2d74-9680-b63f4b46ebf9@gmail.com>
 <20230214.151102.1070436528757437157.horikyota.ntt@gmail.com>
 <9262b2a9-6785-1f83-bf7f-7fcf90c2c113@gmail.com>
 <20230215.095644.822493184688844040.horikyota.ntt@gmail.com>
 <130095b4-7c6d-b6f1-69d4-94f66231f81d@gmail.com>
 <20230216212121.uvav7xgdh2ysqjy5@awork3.anarazel.de>
Content-Language: en-US
From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
In-Reply-To: <20230216212121.uvav7xgdh2ysqjy5@awork3.anarazel.de>
Archived-At: <https://www.postgresql.org/message-id/f08fee25-1dac-b68b-19af-f4b55ceb3802%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------4YJ5iHbwSbs2N8GgMZOGqyJk
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi,

On 2/16/23 10:21 PM, Andres Freund wrote:
> Hi,
> 
> On 2023-02-15 09:21:48 +0100, Drouvot, Bertrand wrote:
>> diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c
>> index f793ac1516..b26e2a5a7a 100644
>> --- a/src/backend/utils/activity/pgstat_relation.c
>> +++ b/src/backend/utils/activity/pgstat_relation.c
>> @@ -471,20 +471,46 @@ pgstat_fetch_stat_tabentry_ext(bool shared, Oid reloid)
>>    * Find any existing PgStat_TableStatus entry for rel_id in the current
>>    * database. If not found, try finding from shared tables.
>>    *
>> + * If an entry is found, copy it and increment the copy's counters with their
>> + * subtransactions counterparts. Then return the copy. There is no need for the
>> + * caller to pfree the copy as the MemoryContext will be reset soon after.
>> + *
> 
> The "There is no need" bit seems a bit off. Yes, that's true for the current
> callers, but who says that it has to stay that way?
> 

Good point. Wording has been changed in V6 attached.

> Otherwise this looks ready, on a casual scan.
> 

Thanks for having looked at it!

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
--------------4YJ5iHbwSbs2N8GgMZOGqyJk
Content-Type: text/plain; charset=UTF-8;
 name="v6-0001-find_tabstat_entry_recon.patch"
Content-Disposition: attachment;
 filename="v6-0001-find_tabstat_entry_recon.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3V0aWxzL2FjdGl2aXR5L3Bnc3RhdF9yZWxhdGlv
bi5jIGIvc3JjL2JhY2tlbmQvdXRpbHMvYWN0aXZpdHkvcGdzdGF0X3JlbGF0aW9uLmMKaW5k
ZXggZjc5M2FjMTUxNi4uYzAzMGUxNzgyZSAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvdXRp
bHMvYWN0aXZpdHkvcGdzdGF0X3JlbGF0aW9uLmMKKysrIGIvc3JjL2JhY2tlbmQvdXRpbHMv
YWN0aXZpdHkvcGdzdGF0X3JlbGF0aW9uLmMKQEAgLTQ3MSwyMCArNDcxLDQ2IEBAIHBnc3Rh
dF9mZXRjaF9zdGF0X3RhYmVudHJ5X2V4dChib29sIHNoYXJlZCwgT2lkIHJlbG9pZCkKICAq
IEZpbmQgYW55IGV4aXN0aW5nIFBnU3RhdF9UYWJsZVN0YXR1cyBlbnRyeSBmb3IgcmVsX2lk
IGluIHRoZSBjdXJyZW50CiAgKiBkYXRhYmFzZS4gSWYgbm90IGZvdW5kLCB0cnkgZmluZGlu
ZyBmcm9tIHNoYXJlZCB0YWJsZXMuCiAgKgorICogSWYgYW4gZW50cnkgaXMgZm91bmQsIGNv
cHkgaXQgYW5kIGluY3JlbWVudCB0aGUgY29weSdzIGNvdW50ZXJzIHdpdGggdGhlaXIKKyAq
IHN1YnRyYW5zYWN0aW9ucyBjb3VudGVycGFydHMuIFRoZW4gcmV0dXJuIHRoZSBjb3B5LiBU
aGUgY2FsbGVyIG1heSBuZWVkIHRvCisgKiBwZnJlZSB0aGUgY29weSAoaW4gY2FzZSB0aGUg
TWVtb3J5Q29udGV4dCBpcyBub3QgcmVzZXQgc29vbiBhZnRlcikuCisgKgogICogSWYgbm8g
ZW50cnkgZm91bmQsIHJldHVybiBOVUxMLCBkb24ndCBjcmVhdGUgYSBuZXcgb25lCiAgKi8K
IFBnU3RhdF9UYWJsZVN0YXR1cyAqCiBmaW5kX3RhYnN0YXRfZW50cnkoT2lkIHJlbF9pZCkK
IHsKIAlQZ1N0YXRfRW50cnlSZWYgKmVudHJ5X3JlZjsKKwlQZ1N0YXRfVGFibGVYYWN0U3Rh
dHVzICp0cmFuczsKKwlQZ1N0YXRfVGFibGVTdGF0dXMgKnRhYmVudHJ5ID0gTlVMTDsKKwlQ
Z1N0YXRfVGFibGVTdGF0dXMgKnRhYmxlc3RhdHVzID0gTlVMTDsKIAogCWVudHJ5X3JlZiA9
IHBnc3RhdF9mZXRjaF9wZW5kaW5nX2VudHJ5KFBHU1RBVF9LSU5EX1JFTEFUSU9OLCBNeURh
dGFiYXNlSWQsIHJlbF9pZCk7CiAJaWYgKCFlbnRyeV9yZWYpCisJewogCQllbnRyeV9yZWYg
PSBwZ3N0YXRfZmV0Y2hfcGVuZGluZ19lbnRyeShQR1NUQVRfS0lORF9SRUxBVElPTiwgSW52
YWxpZE9pZCwgcmVsX2lkKTsKKwkJaWYoIWVudHJ5X3JlZikKKwkJCXJldHVybiB0YWJsZXN0
YXR1czsKKwl9CisKKwl0YWJlbnRyeSA9IChQZ1N0YXRfVGFibGVTdGF0dXMgKikgZW50cnlf
cmVmLT5wZW5kaW5nOworCXRhYmxlc3RhdHVzID0gcGFsbG9jKHNpemVvZihQZ1N0YXRfVGFi
bGVTdGF0dXMpKTsKKwkqdGFibGVzdGF0dXMgPSAqdGFiZW50cnk7CisKKwkvKgorCSAqIExp
dmUgc3VidHJhbnNhY3Rpb25zJyBjb3VudHMgYXJlbid0IGluIHRfY291bnRzIHlldC4gVGhp
cyBpcyBub3QgYSBob3QKKwkgKiBjb2RlIHBhdGggc28gaXQgc291bmRzIG9rIHRvIHJlY29u
Y2lsZSBmb3IgdHVwbGVzX2luc2VydGVkLAorCSAqIHR1cGxlc191cGRhdGVkIGFuZCB0dXBs
ZXNfZGVsZXRlZCBldmVuIGlmIHRoaXMgaXMgbm90IHdoYXQgdGhlIGNhbGxlcgorCSAqIGlz
IGludGVyZXN0ZWQgaW4uCisJICovCisJZm9yICh0cmFucyA9IHRhYmVudHJ5LT50cmFuczsg
dHJhbnMgIT0gTlVMTDsgdHJhbnMgPSB0cmFucy0+dXBwZXIpCisJeworCQl0YWJsZXN0YXR1
cy0+dF9jb3VudHMudF90dXBsZXNfaW5zZXJ0ZWQgKz0gdHJhbnMtPnR1cGxlc19pbnNlcnRl
ZDsKKwkJdGFibGVzdGF0dXMtPnRfY291bnRzLnRfdHVwbGVzX3VwZGF0ZWQgKz0gdHJhbnMt
PnR1cGxlc191cGRhdGVkOworCQl0YWJsZXN0YXR1cy0+dF9jb3VudHMudF90dXBsZXNfZGVs
ZXRlZCArPSB0cmFucy0+dHVwbGVzX2RlbGV0ZWQ7CisJfQogCi0JaWYgKGVudHJ5X3JlZikK
LQkJcmV0dXJuIGVudHJ5X3JlZi0+cGVuZGluZzsKLQlyZXR1cm4gTlVMTDsKKwlyZXR1cm4g
dGFibGVzdGF0dXM7CiB9CiAKIC8qCmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC91dGlscy9h
ZHQvcGdzdGF0ZnVuY3MuYyBiL3NyYy9iYWNrZW5kL3V0aWxzL2FkdC9wZ3N0YXRmdW5jcy5j
CmluZGV4IGI2MWExMjM4MmIuLmI0ZGU1N2M1MzUgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5k
L3V0aWxzL2FkdC9wZ3N0YXRmdW5jcy5jCisrKyBiL3NyYy9iYWNrZW5kL3V0aWxzL2FkdC9w
Z3N0YXRmdW5jcy5jCkBAIC0xNTQ2LDE3ICsxNTQ2LDExIEBAIHBnX3N0YXRfZ2V0X3hhY3Rf
dHVwbGVzX2luc2VydGVkKFBHX0ZVTkNUSU9OX0FSR1MpCiAJT2lkCQkJcmVsaWQgPSBQR19H
RVRBUkdfT0lEKDApOwogCWludDY0CQlyZXN1bHQ7CiAJUGdTdGF0X1RhYmxlU3RhdHVzICp0
YWJlbnRyeTsKLQlQZ1N0YXRfVGFibGVYYWN0U3RhdHVzICp0cmFuczsKIAogCWlmICgodGFi
ZW50cnkgPSBmaW5kX3RhYnN0YXRfZW50cnkocmVsaWQpKSA9PSBOVUxMKQogCQlyZXN1bHQg
PSAwOwogCWVsc2UKLQl7Ci0JCXJlc3VsdCA9IHRhYmVudHJ5LT50X2NvdW50cy50X3R1cGxl
c19pbnNlcnRlZDsKLQkJLyogbGl2ZSBzdWJ0cmFuc2FjdGlvbnMnIGNvdW50cyBhcmVuJ3Qg
aW4gdF90dXBsZXNfaW5zZXJ0ZWQgeWV0ICovCi0JCWZvciAodHJhbnMgPSB0YWJlbnRyeS0+
dHJhbnM7IHRyYW5zICE9IE5VTEw7IHRyYW5zID0gdHJhbnMtPnVwcGVyKQotCQkJcmVzdWx0
ICs9IHRyYW5zLT50dXBsZXNfaW5zZXJ0ZWQ7Ci0JfQorCQlyZXN1bHQgPSAoaW50NjQpICh0
YWJlbnRyeS0+dF9jb3VudHMudF90dXBsZXNfaW5zZXJ0ZWQpOwogCiAJUEdfUkVUVVJOX0lO
VDY0KHJlc3VsdCk7CiB9CkBAIC0xNTY3LDE3ICsxNTYxLDExIEBAIHBnX3N0YXRfZ2V0X3hh
Y3RfdHVwbGVzX3VwZGF0ZWQoUEdfRlVOQ1RJT05fQVJHUykKIAlPaWQJCQlyZWxpZCA9IFBH
X0dFVEFSR19PSUQoMCk7CiAJaW50NjQJCXJlc3VsdDsKIAlQZ1N0YXRfVGFibGVTdGF0dXMg
KnRhYmVudHJ5OwotCVBnU3RhdF9UYWJsZVhhY3RTdGF0dXMgKnRyYW5zOwogCiAJaWYgKCh0
YWJlbnRyeSA9IGZpbmRfdGFic3RhdF9lbnRyeShyZWxpZCkpID09IE5VTEwpCiAJCXJlc3Vs
dCA9IDA7CiAJZWxzZQotCXsKLQkJcmVzdWx0ID0gdGFiZW50cnktPnRfY291bnRzLnRfdHVw
bGVzX3VwZGF0ZWQ7Ci0JCS8qIGxpdmUgc3VidHJhbnNhY3Rpb25zJyBjb3VudHMgYXJlbid0
IGluIHRfdHVwbGVzX3VwZGF0ZWQgeWV0ICovCi0JCWZvciAodHJhbnMgPSB0YWJlbnRyeS0+
dHJhbnM7IHRyYW5zICE9IE5VTEw7IHRyYW5zID0gdHJhbnMtPnVwcGVyKQotCQkJcmVzdWx0
ICs9IHRyYW5zLT50dXBsZXNfdXBkYXRlZDsKLQl9CisJCXJlc3VsdCA9IChpbnQ2NCkgKHRh
YmVudHJ5LT50X2NvdW50cy50X3R1cGxlc191cGRhdGVkKTsKIAogCVBHX1JFVFVSTl9JTlQ2
NChyZXN1bHQpOwogfQpAQCAtMTU4OCwxNyArMTU3NiwxMSBAQCBwZ19zdGF0X2dldF94YWN0
X3R1cGxlc19kZWxldGVkKFBHX0ZVTkNUSU9OX0FSR1MpCiAJT2lkCQkJcmVsaWQgPSBQR19H
RVRBUkdfT0lEKDApOwogCWludDY0CQlyZXN1bHQ7CiAJUGdTdGF0X1RhYmxlU3RhdHVzICp0
YWJlbnRyeTsKLQlQZ1N0YXRfVGFibGVYYWN0U3RhdHVzICp0cmFuczsKIAogCWlmICgodGFi
ZW50cnkgPSBmaW5kX3RhYnN0YXRfZW50cnkocmVsaWQpKSA9PSBOVUxMKQogCQlyZXN1bHQg
PSAwOwogCWVsc2UKLQl7Ci0JCXJlc3VsdCA9IHRhYmVudHJ5LT50X2NvdW50cy50X3R1cGxl
c19kZWxldGVkOwotCQkvKiBsaXZlIHN1YnRyYW5zYWN0aW9ucycgY291bnRzIGFyZW4ndCBp
biB0X3R1cGxlc19kZWxldGVkIHlldCAqLwotCQlmb3IgKHRyYW5zID0gdGFiZW50cnktPnRy
YW5zOyB0cmFucyAhPSBOVUxMOyB0cmFucyA9IHRyYW5zLT51cHBlcikKLQkJCXJlc3VsdCAr
PSB0cmFucy0+dHVwbGVzX2RlbGV0ZWQ7Ci0JfQorCQlyZXN1bHQgPSAoaW50NjQpICh0YWJl
bnRyeS0+dF9jb3VudHMudF90dXBsZXNfZGVsZXRlZCk7CiAKIAlQR19SRVRVUk5fSU5UNjQo
cmVzdWx0KTsKIH0K

--------------4YJ5iHbwSbs2N8GgMZOGqyJk--