Re: proposal: schema variables

so 2. 11. 2024 v 6:46 odesílatel Laurenz Albe <laurenz.albe@cybertec.at>
napsal:

> On Tue, 2024-10-29 at 08:16 +0100, Pavel Stehule wrote:
> > again, necessary rebase
>
> I have started looking at patch 5, and I have some questions and comments.
>
> - The commit message is headed "memory cleaning after DROP VARIABLE", but
>   the rest of the commit message speaks of sinval messages.  These two
>   things are independent, aren't they?  And both lead to the need to
> validate
>   the variables, right?
>

Maybe it can be formulated differently, but it is true. There are a lot of
sinval messages, but in this case
only sinval messages related to DROP VARIABLE are interesting.


>   Then this code comment would for example be wrong:
>
>      /* true after accepted sinval message */
>      static bool needs_validation = false;
>
>   It also becomes "true" after DROP VARIABLE, right?
>   I am happy to fix the comment, but I want to understand the patch first.
>

sinval message can be raised by any operation over the pg_variable table.

 <-><-->if (hashvalue == 0 || svar->hashvalue == hashvalue)
 <-><-->{
 <-><--><-->svar->is_valid = false;
+<-><--><-->needs_validation = true;
+<-><-->}
+<->}

When I execute DROP VARIABLE, then the hash value is specified, but the
hash can be zero for some massive cleaning, and there are other events that
can send sinval message. I think an ANALYZE does this. So the comment /*
true after accepted sinval message */ is more accurate than /* true after
DROP VARIABLE */.


>
> - I see that the patch adds cleanup of invalid session variable to each
>   COMMIT.  Is that a good idea?  I'd expect that it is good enough to clean
>   up whenever session variables are accessed.
>   Calling remove_invalid_session_variables() during each COMMIT will affect
>   all transactions, and I don't see the benefit.
>

If I remember well, there were two reasons why I did it.

1. Minimize the unwanted surprises for users that will check memory usage -
So if you drop the variables, then the allocated space is released in
possibly near time. The rule - allocated space is released, when in the
next transaction you use any session variable looks a little bit crazy
(although I think so there will not be real significant difference in
functionality). Correct me, if I am wrong, but I don't remember any memory
(or resource) cleaning in Postgres, that is delayed to second transactions.
I agree, there is overhead of cleaning, but this can be very fast when the
user doesn't use session variables, because the hash table with session
variables is not initialized. I can imagine some usage some hooks there as
alternative

2. The main reason why it is implemented is implementation of temporal
variables with RESET or DROP on transaction end. Related code should be
triggered at commit time, it cannot be delayed.


>   Also, do we need to call it during pg_session_variables()?
>

I think it can be removed. Originally pg_session_variables showed only
valid variables, but it is not true now.

Regards

Pavel


>
> Yours,
> Laurenz Albe
>