Thread: pg_shmem_allocations & documentation

pg_shmem_allocations & documentation

From
Benoit Lobréau
Date:
Hi,

While reading the documentation of pg_shmem_allocations, I noticed that the off column is described as such : 

"The offset at which the allocation starts. NULL for anonymous allocations and unused memory."

Whereas, the view returns a value for unused memory: 

[local]:5433 postgres@postgres=# SELECT * FROM pg_shmem_allocations WHERE name IS NULL;
 name |    off    |  size   | allocated_size 
------+-----------+---------+----------------
 ¤    | 178095232 | 1923968 |        1923968
(1 row)

From what I understand, the doc is wrong.
Am I right ?

Benoit

Re: pg_shmem_allocations & documentation

From
Kyotaro Horiguchi
Date:
At Thu, 10 Dec 2020 11:07:47 +0100, Benoit Lobréau <benoit.lobreau@gmail.com> wrote in
> Hi,
>
> While reading the documentation of pg_shmem_allocations, I noticed that the
> off column is described as such :
>
> "The offset at which the allocation starts. NULL for anonymous allocations
> and unused memory."
>
> Whereas, the view returns a value for unused memory:
>
> [local]:5433 postgres@postgres=# SELECT * FROM pg_shmem_allocations WHERE
> name IS NULL;
>  name |    off    |  size   | allocated_size
> ------+-----------+---------+----------------
>  ¤    | 178095232 | 1923968 |        1923968
> (1 row)
>
> From what I understand, the doc is wrong.
> Am I right ?

Good catch! I think you're right.  It seems to me the conclusion in
the discussion is to expose the offset for free memory.

Although we could just rip some words off, I'd like to propose instead
to add an explanation why it is not exposed for anonymous allocations,
like the column allocated_size.

> Benoit
>
> [1] https://www.postgresql.org/docs/13/view-pg-shmem-allocations.html
> [2]
> https://www.postgresql.org/message-id/flat/20140504114417.GM12715%40awork2.anarazel.de
> (original thread)

regards.
¤
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 62711ee83f..8921907f5e 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -12492,8 +12492,9 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
        <structfield>off</structfield> <type>int8</type>
       </para>
       <para>
-       The offset at which the allocation starts. NULL for anonymous
-       allocations and unused memory.
+       The offset at which the allocation starts. For anonymous allocations,
+       no information about individual allocations is available, so the column
+       will be NULL in that case.
       </para></entry>
      </row>


Re: pg_shmem_allocations & documentation

From
Michael Paquier
Date:
On Fri, Dec 11, 2020 at 11:00:58AM +0900, Kyotaro Horiguchi wrote:
> Although we could just rip some words off, I'd like to propose instead
> to add an explanation why it is not exposed for anonymous allocations,
> like the column allocated_size.

Indeed, there is a hiccup between what the code does and what the docs
tell: the offset is not NULL for unused memory.

> -       The offset at which the allocation starts. NULL for anonymous
> -       allocations and unused memory.
> +       The offset at which the allocation starts. For anonymous allocations,
> +       no information about individual allocations is available, so the column
> +       will be NULL in that case.

I'd say: let's be simple and just remove "and unused memory" because
anonymous allocations are...  Anonymous so you cannot know details
related to them.  That's something easy to reason about, and the docs
were written originally to remain simple.
--
Michael

Attachment

Re: pg_shmem_allocations & documentation

From
Kyotaro Horiguchi
Date:
At Fri, 11 Dec 2020 14:42:45 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> On Fri, Dec 11, 2020 at 11:00:58AM +0900, Kyotaro Horiguchi wrote:
> > Although we could just rip some words off, I'd like to propose instead
> > to add an explanation why it is not exposed for anonymous allocations,
> > like the column allocated_size.
> 
> Indeed, there is a hiccup between what the code does and what the docs
> tell: the offset is not NULL for unused memory.
> 
> > -       The offset at which the allocation starts. NULL for anonymous
> > -       allocations and unused memory.
> > +       The offset at which the allocation starts. For anonymous allocations,
> > +       no information about individual allocations is available, so the column
> > +       will be NULL in that case.
> 
> I'd say: let's be simple and just remove "and unused memory" because
> anonymous allocations are...  Anonymous so you cannot know details
> related to them.  That's something easy to reason about, and the docs
> were written originally to remain simple.

Hmm. I don't object to that.  Howerver, isn't the description for
allocated_size too verbose in that sense?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



Re: pg_shmem_allocations & documentation

From
Benoit Lobréau
Date:
Would "NULL for anonymous allocations, since details related to them are not known." be ok ?


Le ven. 11 déc. 2020 à 09:29, Kyotaro Horiguchi <horikyota.ntt@gmail.com> a écrit :
At Fri, 11 Dec 2020 14:42:45 +0900, Michael Paquier <michael@paquier.xyz> wrote in
> On Fri, Dec 11, 2020 at 11:00:58AM +0900, Kyotaro Horiguchi wrote:
> > Although we could just rip some words off, I'd like to propose instead
> > to add an explanation why it is not exposed for anonymous allocations,
> > like the column allocated_size.
>
> Indeed, there is a hiccup between what the code does and what the docs
> tell: the offset is not NULL for unused memory.
>
> > -       The offset at which the allocation starts. NULL for anonymous
> > -       allocations and unused memory.
> > +       The offset at which the allocation starts. For anonymous allocations,
> > +       no information about individual allocations is available, so the column
> > +       will be NULL in that case.
>
> I'd say: let's be simple and just remove "and unused memory" because
> anonymous allocations are...  Anonymous so you cannot know details
> related to them.  That's something easy to reason about, and the docs
> were written originally to remain simple.

Hmm. I don't object to that.  Howerver, isn't the description for
allocated_size too verbose in that sense?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Re: pg_shmem_allocations & documentation

From
Benoit Lobréau
Date:
Here's a proposal patch.

Le ven. 11 déc. 2020 à 09:58, Benoit Lobréau <benoit.lobreau@gmail.com> a écrit :
Would "NULL for anonymous allocations, since details related to them are not known." be ok ?


Le ven. 11 déc. 2020 à 09:29, Kyotaro Horiguchi <horikyota.ntt@gmail.com> a écrit :
At Fri, 11 Dec 2020 14:42:45 +0900, Michael Paquier <michael@paquier.xyz> wrote in
> On Fri, Dec 11, 2020 at 11:00:58AM +0900, Kyotaro Horiguchi wrote:
> > Although we could just rip some words off, I'd like to propose instead
> > to add an explanation why it is not exposed for anonymous allocations,
> > like the column allocated_size.
>
> Indeed, there is a hiccup between what the code does and what the docs
> tell: the offset is not NULL for unused memory.
>
> > -       The offset at which the allocation starts. NULL for anonymous
> > -       allocations and unused memory.
> > +       The offset at which the allocation starts. For anonymous allocations,
> > +       no information about individual allocations is available, so the column
> > +       will be NULL in that case.
>
> I'd say: let's be simple and just remove "and unused memory" because
> anonymous allocations are...  Anonymous so you cannot know details
> related to them.  That's something easy to reason about, and the docs
> were written originally to remain simple.

Hmm. I don't object to that.  Howerver, isn't the description for
allocated_size too verbose in that sense?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachment

Re: pg_shmem_allocations & documentation

From
Michael Paquier
Date:
On Mon, Dec 14, 2020 at 10:33:06AM +0100, Benoit Lobréau wrote:
>        </para>
>        <para>
>         The offset at which the allocation starts. NULL for anonymous
> -       allocations and unused memory.
> +       allocations, since details related to them are not known.
>        </para></entry>

Both of you seem to agree about having more details about that, which
is fine by me at the end.  Horiguchi-san, do you have more thoughts to
offer?  Benoit's version is similar to yours, just simpler.
--
Michael

Attachment

Re: pg_shmem_allocations & documentation

From
Michael Paquier
Date:
On Tue, Dec 15, 2020 at 10:09:35AM +0900, Michael Paquier wrote:
> Both of you seem to agree about having more details about that, which
> is fine by me at the end.  Horiguchi-san, do you have more thoughts to
> offer?  Benoit's version is similar to yours, just simpler.

Okay, applied this one then.  Thanks Benoit and Horigushi-san.
--
Michael

Attachment