Content-Type: multipart/mixed; boundary="------------cMsqWnpmPMzPTHxslrPn6GNz"
Message-ID: <71eef655-c192-453f-ac45-2772fec2cb04@gmail.com>
Date: Tue, 25 Feb 2025 19:24:14 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: Alex Friedman <alexf01@gmail.com>
Subject: A small correction to doc and comment of FSM for indexes
Content-Language: en-US
To: pgsql-hackers@lists.postgresql.org
Archived-At: 
 <https://www.postgresql.org/message-id/71eef655-c192-453f-ac45-2772fec2cb04%40gmail.com>
Precedence: bulk

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

Hi,

This patch fixes a couple of small inaccuracies in the doc and the comment for FSM about index handling.

1. In the doc for pg_freespacemap, it currently says:

> For indexes, what is tracked is entirely-unused pages, rather than free space within pages. Therefore, the values are not meaningful, just whether a page is full or empty.

However, as what is tracked is entirely-unused pages, the values mean whether a page is "in-use or empty", rather than "full or empty".


2. In indexfsm.c the header comment says:

>  *     This is similar to the FSM used for heap, in freespace.c, but instead
>  *     of tracking the amount of free space on pages, we only track whether
>  *     pages are completely free or in-use. We use the same FSM implementation
>  *     as for heaps, using BLCKSZ - 1 to denote used pages, and 0 for unused.

However, in the code we see that used pages are marked with 0:

> /*
>  * RecordUsedIndexPage - mark a page as used in the FSM
>  */
> void
> RecordUsedIndexPage(Relation rel, BlockNumber usedBlock)
> {
> 	RecordPageWithFreeSpace(rel, usedBlock, 0);
> }

And free pages are marked with BLCKSZ - 1:

> /*
>  * RecordFreeIndexPage - mark a page as free in the FSM
>  */
> void
> RecordFreeIndexPage(Relation rel, BlockNumber freeBlock)
> {
> 	RecordPageWithFreeSpace(rel, freeBlock, BLCKSZ - 1);
> }

And so, this patch also fixes the comment's "using BLCKSZ - 1 to denote used pages, and 0 for unused" to be "using 0 to denote used pages, and BLCKSZ - 1 for unused".

While these changes are minor, I've seen how this can cause a bit of confusion, and it would be good to clarify it.


Best regards,

Alex Friedman
--------------cMsqWnpmPMzPTHxslrPn6GNz
Content-Type: text/plain; charset=UTF-8;
 name="v1-0001-A-small-correction-to-doc-and-comment-of-FSM-for-.patch"
Content-Disposition: attachment;
 filename*0="v1-0001-A-small-correction-to-doc-and-comment-of-FSM-for-.pa";
 filename*1="tch"
Content-Transfer-Encoding: base64

RnJvbSBhMWI3ODQzODM0M2ZjYTA1M2FhMDAxNDY4N2VhYmEzNGQ1ZTE2MGUwIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBBbGV4IEZyaWVkbWFuIDxhbGV4ZjAxQGdtYWlsLmNv
bT4KRGF0ZTogVHVlLCAyNSBGZWIgMjAyNSAxOToxMjo1MyArMDIwMApTdWJqZWN0OiBbUEFU
Q0ggdjFdIEEgc21hbGwgY29ycmVjdGlvbiB0byBkb2MgYW5kIGNvbW1lbnQgb2YgRlNNIGZv
ciBpbmRleGVzLgoKLS0tCiBkb2Mvc3JjL3NnbWwvcGdmcmVlc3BhY2VtYXAuc2dtbCAgICAg
ICAgIHwgMiArLQogc3JjL2JhY2tlbmQvc3RvcmFnZS9mcmVlc3BhY2UvaW5kZXhmc20uYyB8
IDIgKy0KIDIgZmlsZXMgY2hhbmdlZCwgMiBpbnNlcnRpb25zKCspLCAyIGRlbGV0aW9ucygt
KQoKZGlmZiAtLWdpdCBhL2RvYy9zcmMvc2dtbC9wZ2ZyZWVzcGFjZW1hcC5zZ21sIGIvZG9j
L3NyYy9zZ21sL3BnZnJlZXNwYWNlbWFwLnNnbWwKaW5kZXggODI5YWQ2MGYzMmYuLjM3NzRh
OWY4YzZiIDEwMDY0NAotLS0gYS9kb2Mvc3JjL3NnbWwvcGdmcmVlc3BhY2VtYXAuc2dtbAor
KysgYi9kb2Mvc3JjL3NnbWwvcGdmcmVlc3BhY2VtYXAuc2dtbApAQCAtNjcsNyArNjcsNyBA
QAogICA8cGFyYT4KICAgIEZvciBpbmRleGVzLCB3aGF0IGlzIHRyYWNrZWQgaXMgZW50aXJl
bHktdW51c2VkIHBhZ2VzLCByYXRoZXIgdGhhbiBmcmVlCiAgICBzcGFjZSB3aXRoaW4gcGFn
ZXMuICBUaGVyZWZvcmUsIHRoZSB2YWx1ZXMgYXJlIG5vdCBtZWFuaW5nZnVsLCBqdXN0Ci0g
ICB3aGV0aGVyIGEgcGFnZSBpcyBmdWxsIG9yIGVtcHR5LgorICAgd2hldGhlciBhIHBhZ2Ug
aXMgaW4tdXNlIG9yIGVtcHR5LgogICA8L3BhcmE+CiAgPC9zZWN0Mj4KIApkaWZmIC0tZ2l0
IGEvc3JjL2JhY2tlbmQvc3RvcmFnZS9mcmVlc3BhY2UvaW5kZXhmc20uYyBiL3NyYy9iYWNr
ZW5kL3N0b3JhZ2UvZnJlZXNwYWNlL2luZGV4ZnNtLmMKaW5kZXggMWZjMjYzODkyYTcuLjNj
ZDI0Mzc1OTlkIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9zdG9yYWdlL2ZyZWVzcGFjZS9p
bmRleGZzbS5jCisrKyBiL3NyYy9iYWNrZW5kL3N0b3JhZ2UvZnJlZXNwYWNlL2luZGV4ZnNt
LmMKQEAgLTE2LDcgKzE2LDcgQEAKICAqCVRoaXMgaXMgc2ltaWxhciB0byB0aGUgRlNNIHVz
ZWQgZm9yIGhlYXAsIGluIGZyZWVzcGFjZS5jLCBidXQgaW5zdGVhZAogICoJb2YgdHJhY2tp
bmcgdGhlIGFtb3VudCBvZiBmcmVlIHNwYWNlIG9uIHBhZ2VzLCB3ZSBvbmx5IHRyYWNrIHdo
ZXRoZXIKICAqCXBhZ2VzIGFyZSBjb21wbGV0ZWx5IGZyZWUgb3IgaW4tdXNlLiBXZSB1c2Ug
dGhlIHNhbWUgRlNNIGltcGxlbWVudGF0aW9uCi0gKglhcyBmb3IgaGVhcHMsIHVzaW5nIEJM
Q0tTWiAtIDEgdG8gZGVub3RlIHVzZWQgcGFnZXMsIGFuZCAwIGZvciB1bnVzZWQuCisgKglh
cyBmb3IgaGVhcHMsIHVzaW5nIDAgdG8gZGVub3RlIHVzZWQgcGFnZXMsIGFuZCBCTENLU1og
LSAxIGZvciB1bnVzZWQuCiAgKgogICotLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCiAgKi8KLS0gCjIu
NDEuMAoK

--------------cMsqWnpmPMzPTHxslrPn6GNz--