Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1t78iB-000Dpm-Sw for pgsql-hackers@arkaria.postgresql.org; Sat, 02 Nov 2024 07:38:20 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1t78hB-004E9H-TM for pgsql-hackers@arkaria.postgresql.org; Sat, 02 Nov 2024 07:37:18 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1t78hB-004E99-GS for pgsql-hackers@lists.postgresql.org; Sat, 02 Nov 2024 07:37:17 +0000 Received: from mail-yb1-xb30.google.com ([2607:f8b0:4864:20::b30]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1t78h5-004JgL-ES for pgsql-hackers@lists.postgresql.org; Sat, 02 Nov 2024 07:37:17 +0000 Received: by mail-yb1-xb30.google.com with SMTP id 3f1490d57ef6-e2918664a3fso2261729276.0 for ; Sat, 02 Nov 2024 00:37:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730533029; x=1731137829; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=vQn2oYPNQAUNmzyJuFmlxqCP13ArjMEjCzSz+JaJIWg=; b=Px3hcuEYuFfROTQEtcJEJq2yK3UEP+fTyU7rxqSAUtx+y7EF1Q3tCcdWMZ43eQVCFK vJalp+IhG1LJZ5hSiC3BeR17nZla4RNny8JWKbvhTfGsWkwBpDHqXMdSpZw/vw3ns8PY 9VUtzdysz/lgWlO+3o2m5eQ2lf0r75ikJnVAlL/gYpvcEXVwt2ohVNuHK6agBipzLJJN bKHmp/YTlRC5/tp3N+qM/5yP7Fvn2iyu68Hq5h6iDf08wB5MJRuMS/PH8YP6tm43O1sL cK6IlmqFe/Sj9hLVMOFIY8RiJvjhBooGTGQZf1IJVIdaLppBUpTLiUdQ4VT4y6KqhVjN lCLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730533029; x=1731137829; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=vQn2oYPNQAUNmzyJuFmlxqCP13ArjMEjCzSz+JaJIWg=; b=RKm/ljc5wHPQPNSWCLSeSM3y1AOIbqBSJHn4qW77SuSLFb3H6AHjwfrAwe6o02DrH/ z2sAvCd2/WuR1ow5GBfyNayQw89mchsRDJOaCcv+NvRyfzIkFGPVBmGfsJGbOKGDihgU iZ9k0IL8cjsu+1JUgGDHB2OacMBRFR/MhA2iNOP3YiEuQ7olSYqAy0gzWBV2QK7GTPGu UMzb/1aJcbZv1odddoxmSaO3GriOeFaPdX1/rwVE1Zly1sD7WZT74zgAUrbESJ3AKTyY 9lx2EvrVMI/XAwZ+dFU+AfObJeJiSQlumdpLIoZ4pInDX/6nwBI46IYTjEbmVxBmACUQ RwEQ== X-Forwarded-Encrypted: i=1; AJvYcCXCdSkQnJrTWfyFgyEmSHw3yv0tDzejf90supB1q01nS1I0e/4EjjPK1ilI7U4+npAXrrv1/BrnC8a6VcUX@lists.postgresql.org X-Gm-Message-State: AOJu0Yy7mrGFjwuZBW+zucCkhvH+VRZK1VeXdljQxgAJmmdKYNiVqcQ0 WEZKpW+VPF2gpjKmO4085iSVUPfzZYwqDmuXfLgAcEpux9I6ZLjTeUpqlsA6o/zAw0zvRtmcS/A AqYGfhuwc72XQuTACMkvlUb5b4z8= X-Google-Smtp-Source: AGHT+IHwiFghJob4/Ht/ntvQxdpqilHtv65fcJiY6Ds5pLnKNA03whb0qMrURG6u3/8+6LAf/m7FEP6UzPZ9YFp/YZ4= X-Received: by 2002:a05:6902:b17:b0:e33:1ed4:2a00 with SMTP id 3f1490d57ef6-e331ed42cadmr712705276.27.1730533028796; Sat, 02 Nov 2024 00:37:08 -0700 (PDT) MIME-Version: 1.0 References: <20200924035637.GF28585@paquier.xyz> <20201001033824.GC8130@paquier.xyz> <51a9a68e8a998d04df17417d45c1dbd4@xs4all.nl> <89817942c99da01cd5e7850fe418436b@xs4all.nl> <56ca532c37eb0b540961f74a7bd5db39@xs4all.nl> <8181bd3abc647bdae5a4f78e71e62478a98c75f4.camel@cybertec.at> <9e67d49deb18270eddb95e602c83f02b98459843.camel@cybertec.at> <3b662dc5b615d4c20a55e8e2fbe6fc00fe00609d.camel@cybertec.at> <6996931e8c9edf3b82223e74e92326a7ed06c1d6.camel@cybertec.at> <67aa68a7e6dfb44c0cbbdf7f97cadfede4269ce5.camel@cybertec.at> <04ec666686e9e21cb515617df06885c66f3d34ce.camel@cybertec.at> <3850a85012d040827b10193189edbe2c23a64f8f.camel@cybertec.at> In-Reply-To: From: Pavel Stehule Date: Sat, 2 Nov 2024 08:36:30 +0100 Message-ID: Subject: Re: proposal: schema variables To: Laurenz Albe Cc: Erik Rijkers , Michael Paquier , Amit Kapila , DUVAL REMI , PostgreSQL Hackers Content-Type: multipart/alternative; boundary="0000000000007420fb0625e91ef0" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --0000000000007420fb0625e91ef0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable so 2. 11. 2024 v 6:46 odes=C3=ADlatel Laurenz Albe 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 =3D 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 =3D=3D 0 || svar->hashvalue =3D=3D hashvalue) <-><-->{ <-><--><-->svar->is_valid =3D false; +<-><--><-->needs_validation =3D 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 cle= an > up whenever session variables are accessed. > Calling remove_invalid_session_variables() during each COMMIT will affe= ct > 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 > --0000000000007420fb0625e91ef0 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
so 2. 11. 2024 v=C2=A06:46 odes=C3=AD= 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.<= br>
- The commit message is headed "memory cleaning after DROP VARIABLE&qu= ot;, but
=C2=A0 the rest of the commit message speaks of sinval messages.=C2=A0 Thes= e two
=C2=A0 things are independent, aren't they?=C2=A0 And both lead to the = need to validate
=C2=A0 the variables, right?

Maybe it c= an be formulated differently, but it is true. There are a lot of sinval mes= sages, but in this case
only sinval messages related to DROP VARI= ABLE are interesting.=C2=A0


=C2=A0 Then this code comment would for example be wrong:

=C2=A0 =C2=A0 =C2=A0/* true after accepted sinval message */
=C2=A0 =C2=A0 =C2=A0static bool needs_validation =3D false;

=C2=A0 It also becomes "true" after DROP VARIABLE, right?
=C2=A0 I am happy to fix the comment, but I want to understand the patch fi= rst.

sinval message can be raised by an= y operation over the pg_variable table.

=C2= =A0<-><-->if (hashvalue =3D=3D 0 || svar->hashvalue =3D=3D h= ashvalue)
=C2=A0<-><-->{
=C2=A0<-><--><--&= gt;svar->is_valid =3D false;
+<-><--><-->needs_vali= dation =3D 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 afte= r DROP VARIABLE */.
=C2=A0

- I see that the patch adds cleanup of invalid session variable to each
=C2=A0 COMMIT.=C2=A0 Is that a good idea?=C2=A0 I'd expect that it is g= ood enough to clean
=C2=A0 up whenever session variables are accessed.
=C2=A0 Calling remove_invalid_session_variables() during each COMMIT will a= ffect
=C2=A0 all transactions, and I don't see the benefit.
<= div>
If I remember well, there were two reasons why I did it.

1. Minimiz= e 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 ove= rhead 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 ini= tialized. I can imagine some usage some hooks there as alternative

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


=C2=A0 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.
<= br>
Regards

Pavel
=C2=A0

Yours,
Laurenz Albe
--0000000000007420fb0625e91ef0--