MIME-Version: 1.0
From: Ian Lawrence Barwick <barwick@gmail.com>
Date: Thu, 24 Aug 2023 10:22:49 +0900
Message-ID: <CAB8KJ=j-ACb3H4L9a_b3ZG3iCYDW5aEu3WsPAzkm2S7JzS1Few@mail.gmail.com>
Subject: pg_stat_get_backend_subxact() and backend IDs?
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000a1cfc60603a11197"
Archived-At: <https://www.postgresql.org/message-id/CAB8KJ%3Dj-ACb3H4L9a_b3ZG3iCYDW5aEu3WsPAzkm2S7JzS1Few%40mail.gmail.com>
Precedence: bulk

--000000000000a1cfc60603a11197
Content-Type: text/plain; charset="UTF-8"

Hi

I was playing around with "pg_stat_get_backend_subxact()" (commit 10ea0f924)
and see it emits NULL values for some backends, e.g.:

    postgres=# \pset null NULL
    Null display is "NULL".

    postgres=#  SELECT id, pg_stat_get_backend_pid(id), s.*,
                       pg_stat_get_backend_activity (id)
                  FROM pg_stat_get_backend_idset() id
               JOIN LATERAL pg_stat_get_backend_subxact(id) AS s ON TRUE;
     id  | pg_stat_get_backend_pid | subxact_count |
subxact_overflowed |                pg_stat_get_backend_activity
    -----+-------------------------+---------------+--------------------+------------------------------------------------------------
       1 |                 3175972 |             0 | f
 | <command string not enabled>
       2 |                 3175973 |             0 | f
 | <command string not enabled>
       3 |                 3177889 |             0 | f
 | SELECT id, pg_stat_get_backend_pid(id), s.*,              +
         |                         |               |
 |         pg_stat_get_backend_activity (id)                 +
         |                         |               |
 |    FROM pg_stat_get_backend_idset() id                    +
         |                         |               |
 | JOIN LATERAL pg_stat_get_backend_subxact(id) AS s ON TRUE;
       4 |                 3176027 |             5 | f
 | savepoint s4;
     256 |                 3175969 |          NULL | NULL
 | <command string not enabled>
     258 |                 3175968 |          NULL | NULL
 | <command string not enabled>
     259 |                 3175971 |          NULL | NULL
 | <command string not enabled>
    (7 rows)

Reading through the thread [1], it looks like 0/false are intended to be
returned for non-backend processes too [2], so it seems odd that NULL/NULL is
getting returned in some cases, especially as that's what's returned if a
non-existent backend ID is provided.

[1] https://www.postgresql.org/message-id/flat/CAFiTN-uvYAofNRaGF4R%2Bu6_OrABdkqNRoX7V6%2BPP3H_0HuYMwg%40mail.gmail.com
[2] https://www.postgresql.org/message-id/flat/CAFiTN-ut0uwkRJDQJeDPXpVyTWD46m3gt3JDToE02hTfONEN%3DQ%40mail.gmail.com#821f6f40e91314066390efd06d71d5ac

Looking at the code, this is happening because
"pgstat_fetch_stat_local_beentry()"
expects to be passed the backend ID as an integer representing a 1-based index
referring to "localBackendStatusTable", but "pg_stat_get_backend_subxact()"
is presumably intended to take the actual BackendId , as per other
"pg_stat_get_XXX()"
functions.

Also, the comment for "pgstat_fetch_stat_local_beentry()" says:

   Returns NULL if the argument is out of range (no current caller does that).

so the last part is currently incorrect.

Assuming I am not misunderstanding something here (always a
possibility, apologies
in advance if this is merely noise), what is actually needed is a function which
accepts a BackendId (as per "pgstat_fetch_stat_beentry()"), but returns a
LocalPgBackendStatus (as per "pgstat_fetch_stat_local_beentry()") like the
attached, clumsily named "pgstat_fetch_stat_backend_local_beentry()".

Regards

Ian Barwick

--000000000000a1cfc60603a11197
Content-Type: text/x-patch; charset="US-ASCII"; 
	name="pg_stat_get_backend_subxact-fix.v1.patch"
Content-Disposition: attachment; 
	filename="pg_stat_get_backend_subxact-fix.v1.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_llohacsm0>
X-Attachment-Id: f_llohacsm0

Y29tbWl0IGQ0MjkyZDMzNDc2NThmNjE4NTVmZDg0ODI3OTU0ZjU4NzgzOGEzMjQKQXV0aG9yOiBJ
YW4gQmFyd2ljayA8YmFyd2lja0BnbWFpbC5jb20+CkRhdGU6ICAgVGh1IEF1ZyAyNCAxMDoxNjo1
MSAyMDIzICswOTAwCgogICAgcGdfc3RhdF9nZXRfYmFja2VuZF9zdWJ4YWN0KCk6IGhhbmRsZSBi
YWNrZW5kIElEIGNvcnJlY3RseQoKZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3V0aWxzL2FjdGl2
aXR5L2JhY2tlbmRfc3RhdHVzLmMgYi9zcmMvYmFja2VuZC91dGlscy9hY3Rpdml0eS9iYWNrZW5k
X3N0YXR1cy5jCmluZGV4IDM4ZjkxYTQ5NWIuLmJjZTc4MzYyNjQgMTAwNjQ0Ci0tLSBhL3NyYy9i
YWNrZW5kL3V0aWxzL2FjdGl2aXR5L2JhY2tlbmRfc3RhdHVzLmMKKysrIGIvc3JjL2JhY2tlbmQv
dXRpbHMvYWN0aXZpdHkvYmFja2VuZF9zdGF0dXMuYwpAQCAtMTEzNiw2ICsxMTM2LDM5IEBAIHBn
c3RhdF9mZXRjaF9zdGF0X2xvY2FsX2JlZW50cnkoaW50IGJlaWQpCiB9CiAKIAorLyogLS0tLS0t
LS0tLQorICogcGdzdGF0X2ZldGNoX3N0YXRfYmFja2VuZF9sb2NhbF9iZWVudHJ5KCkgLQorICoK
KyAqCUxpa2UgcGdzdGF0X2ZldGNoX3N0YXRfbG9jYWxfYmVlbnRyeSgpIGJ1dCB0YWtlcyB0aGUg
QmFja2VuZElkIG9mIHRoZQorICogIGRlc2lyZWQgc2Vzc2lvbi4KKyAqCisgKglSZXR1cm5zIE5V
TEwgaWYgdGhlIGdpdmVuIGJlaWQgZG9lc24ndCBpZGVudGlmeSBhbnkga25vd24gc2Vzc2lvbi4K
KyAqCisgKglOQjogY2FsbGVyIGlzIHJlc3BvbnNpYmxlIGZvciBhIGNoZWNrIGlmIHRoZSB1c2Vy
IGlzIHBlcm1pdHRlZCB0byBzZWUKKyAqCXRoaXMgaW5mbyAoZXNwZWNpYWxseSB0aGUgcXVlcnlz
dHJpbmcpLgorICogLS0tLS0tLS0tLQorICovCisKK0xvY2FsUGdCYWNrZW5kU3RhdHVzICoKK3Bn
c3RhdF9mZXRjaF9zdGF0X2JhY2tlbmRfbG9jYWxfYmVlbnRyeShCYWNrZW5kSWQgYmVpZCkKK3sK
KwlMb2NhbFBnQmFja2VuZFN0YXR1cyBrZXk7CisKKwlwZ3N0YXRfcmVhZF9jdXJyZW50X3N0YXR1
cygpOworCisJLyoKKwkgKiBTaW5jZSB0aGUgbG9jYWxCYWNrZW5kU3RhdHVzVGFibGUgaXMgaW4g
b3JkZXIgYnkgYmFja2VuZF9pZCwgd2UgY2FuIHVzZQorCSAqIGJzZWFyY2goKSB0byBzZWFyY2gg
aXQgZWZmaWNpZW50bHkuCisJICovCisJa2V5LmJhY2tlbmRfaWQgPSBiZWlkOworCisJcmV0dXJu
IChMb2NhbFBnQmFja2VuZFN0YXR1cyAqKSBic2VhcmNoKCZrZXksIGxvY2FsQmFja2VuZFN0YXR1
c1RhYmxlLAorCQkJCQkJCQkJCQlsb2NhbE51bUJhY2tlbmRzLAorCQkJCQkJCQkJCQlzaXplb2Yo
TG9jYWxQZ0JhY2tlbmRTdGF0dXMpLAorCQkJCQkJCQkJCQljbXBfbGJlc3RhdHVzKTsKK30KKwor
CiAvKiAtLS0tLS0tLS0tCiAgKiBwZ3N0YXRfZmV0Y2hfc3RhdF9udW1iYWNrZW5kcygpIC0KICAq
CmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC91dGlscy9hZHQvcGdzdGF0ZnVuY3MuYyBiL3NyYy9i
YWNrZW5kL3V0aWxzL2FkdC9wZ3N0YXRmdW5jcy5jCmluZGV4IDJiOTc0MmFkMjEuLjg0NzllYTEz
MGIgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL3V0aWxzL2FkdC9wZ3N0YXRmdW5jcy5jCisrKyBi
L3NyYy9iYWNrZW5kL3V0aWxzL2FkdC9wZ3N0YXRmdW5jcy5jCkBAIC03MjcsNyArNzI3LDcgQEAg
cGdfc3RhdF9nZXRfYmFja2VuZF9zdWJ4YWN0KFBHX0ZVTkNUSU9OX0FSR1MpCiAKIAlCbGVzc1R1
cGxlRGVzYyh0dXBkZXNjKTsKIAotCWlmICgobG9jYWxfYmVlbnRyeSA9IHBnc3RhdF9mZXRjaF9z
dGF0X2xvY2FsX2JlZW50cnkoYmVpZCkpICE9IE5VTEwpCisJaWYgKChsb2NhbF9iZWVudHJ5ID0g
cGdzdGF0X2ZldGNoX3N0YXRfYmFja2VuZF9sb2NhbF9iZWVudHJ5KGJlaWQpKSAhPSBOVUxMKQog
CXsKIAkJLyogRmlsbCB2YWx1ZXMgYW5kIE5VTExzICovCiAJCXZhbHVlc1swXSA9IEludDMyR2V0
RGF0dW0obG9jYWxfYmVlbnRyeS0+YmFja2VuZF9zdWJ4YWN0X2NvdW50KTsKZGlmZiAtLWdpdCBh
L3NyYy9pbmNsdWRlL3V0aWxzL2JhY2tlbmRfc3RhdHVzLmggYi9zcmMvaW5jbHVkZS91dGlscy9i
YWNrZW5kX3N0YXR1cy5oCmluZGV4IDc3OTM5YTBhZWQuLjc2MGRkYzQ4YWUgMTAwNjQ0Ci0tLSBh
L3NyYy9pbmNsdWRlL3V0aWxzL2JhY2tlbmRfc3RhdHVzLmgKKysrIGIvc3JjL2luY2x1ZGUvdXRp
bHMvYmFja2VuZF9zdGF0dXMuaApAQCAtMzM0LDYgKzMzNCw3IEBAIGV4dGVybiB1aW50NjQgcGdz
dGF0X2dldF9teV9xdWVyeV9pZCh2b2lkKTsKICAqLwogZXh0ZXJuIGludAlwZ3N0YXRfZmV0Y2hf
c3RhdF9udW1iYWNrZW5kcyh2b2lkKTsKIGV4dGVybiBQZ0JhY2tlbmRTdGF0dXMgKnBnc3RhdF9m
ZXRjaF9zdGF0X2JlZW50cnkoQmFja2VuZElkIGJlaWQpOworZXh0ZXJuIExvY2FsUGdCYWNrZW5k
U3RhdHVzICpwZ3N0YXRfZmV0Y2hfc3RhdF9iYWNrZW5kX2xvY2FsX2JlZW50cnkoQmFja2VuZElk
IGJlaWQpOwogZXh0ZXJuIExvY2FsUGdCYWNrZW5kU3RhdHVzICpwZ3N0YXRfZmV0Y2hfc3RhdF9s
b2NhbF9iZWVudHJ5KGludCBiZWlkKTsKIGV4dGVybiBjaGFyICpwZ3N0YXRfY2xpcF9hY3Rpdml0
eShjb25zdCBjaGFyICpyYXdfYWN0aXZpdHkpOwogCg==
--000000000000a1cfc60603a11197--