Re: proposal: schema variables - Mailing list pgsql-hackers
| From | Pavel Stehule |
|---|---|
| Subject | Re: proposal: schema variables |
| Date | |
| Msg-id | CAFj8pRBuhB_jgsGx4-Nhpow0vi7g98RjKwTRPM3dSbiKjWdqQA@mail.gmail.com Whole thread Raw |
| In response to | Re: proposal: schema variables (Laurenz Albe <laurenz.albe@cybertec.at>) |
| Responses |
Re: proposal: schema variables
|
| List | pgsql-hackers |
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;
+<-><-->}
+<->}
<-><-->{
<-><--><-->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.
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
pgsql-hackers by date: