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 1t7tJu-000wLy-LJ for pgsql-hackers@arkaria.postgresql.org; Mon, 04 Nov 2024 09:24:22 +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 1t7tJq-004l08-U0 for pgsql-hackers@arkaria.postgresql.org; Mon, 04 Nov 2024 09:24:19 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1t7tJq-004l00-FS for pgsql-hackers@lists.postgresql.org; Mon, 04 Nov 2024 09:24:19 +0000 Received: from mail-wm1-x336.google.com ([2a00:1450:4864:20::336]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1t7tJo-0006Cd-EM for pgsql-hackers@lists.postgresql.org; Mon, 04 Nov 2024 09:24:17 +0000 Received: by mail-wm1-x336.google.com with SMTP id 5b1f17b1804b1-431548bd1b4so32085195e9.3 for ; Mon, 04 Nov 2024 01:24:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cybertec.at; s=cybertec.at; t=1730712254; x=1731317054; darn=lists.postgresql.org; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject :date:message-id:reply-to; bh=MAnLqj+1XPn2QXyq7ZP4ZX5hEsul57b0cK/PrCJ4z40=; b=J92vKw7MKlgIIL67pnY9S0pNS0EhFTohxkOallEje0yX5xRLpL+cjOG9Jn0VAYl74X nb6/UzIl19ZWXl7KSV2mQSxeeFupsuZu4ifCzK/henDS9YfBvnwDKFztckalGhMJFE1i PYQ6mwo6DwYqCsg+4ZKv+Kks7oaZqWG2WvW1Q= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730712254; x=1731317054; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=MAnLqj+1XPn2QXyq7ZP4ZX5hEsul57b0cK/PrCJ4z40=; b=iH7ganccVMb8BClwmYZ5Hby5iXGjHouE2aMwKAGJmHONH9RQngPvRIsSNfM1UNkJX+ 93VKMr7mgzRO9kPtUpI0akrjg0ZKb7m2nH8hAY0JaxgcdEW/HOaA2BwJcuvaH81cX/Tj NoHgWZq4dWZ/DbLapwkMnlQdoQWDCatL8rqWpspDki1tXa1NGoqSLFNyNJZQQ+/9pII5 xAwP1Kk61KwUAGZZ/aqozZFKJAkwp9+5N5zNzGfrmwm5c3A2sDcwXAYaTgK7bYJrENja VTGWkeoJZoaskhBFgz8v3LXj6h64+aOSwdu4Vs19TTzIAUVuWkyCaP8BIRx1XgrDbqrX NOmw== X-Forwarded-Encrypted: i=1; AJvYcCXWvmjiiQ/DyMbS6E90LftY2NZG6cfB9De2i5OvyEtqbRk4/BE241NHFC/xAMEcnHgDSMHNQA67w6XsEBj9@lists.postgresql.org X-Gm-Message-State: AOJu0YxKXpLPTDviNFgRXKso5+ArIKyzc+WKHR4XhrotsspP8Cww0Q2A LxkB9jWDkL1yJtr/c04ptEwuQaIHgRCpIemlJoq7ehP+mihmIeiuoAJFqXG+zlE= X-Google-Smtp-Source: AGHT+IGnkx5B4dsg/05+qHRZrs1EoOO6NAGClkrdqG0zfTT+1/Qu3teWYJe8zdfDAaAfRkICnIb5uA== X-Received: by 2002:a05:600c:3b25:b0:432:7c08:d11e with SMTP id 5b1f17b1804b1-4328323f5f4mr93578025e9.1.1730712254538; Mon, 04 Nov 2024 01:24:14 -0800 (PST) Received: from localhost.localdomain (ip-185-104-138-49.ptr.icomera.net. [185.104.138.49]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-431bd9a99d3sm178289965e9.38.2024.11.04.01.24.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Nov 2024 01:24:14 -0800 (PST) Message-ID: Subject: Re: proposal: schema variables From: Laurenz Albe To: Pavel Stehule Cc: Erik Rijkers , Michael Paquier , Amit Kapila , DUVAL REMI , PostgreSQL Hackers Date: Mon, 04 Nov 2024 10:24:09 +0100 In-Reply-To: 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> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.52.4 (3.52.4-2.fc40) MIME-Version: 1.0 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk On Sat, 2024-11-02 at 08:36 +0100, Pavel Stehule wrote: > so 2. 11. 2024 v=C2=A06:46 odes=C3=ADlatel Laurenz Albe napsal: > > - The commit message is headed "memory cleaning after DROP VARIABLE", b= ut > > =C2=A0 the rest of the commit message speaks of sinval messages.=C2=A0 = These two > > =C2=A0 things are independent, aren't they?=C2=A0 And both lead to the = need to validate > > =C2=A0 the variables, right? >=20 > Maybe it can be formulated differently, but it is true. There are a lot o= f sinval messages, but in this case > only sinval messages related to DROP VARIABLE are interesting. Okay... > > =C2=A0 Then this code comment would for example be wrong: > >=20 > > =C2=A0 =C2=A0 =C2=A0/* true after accepted sinval message */ > > =C2=A0 =C2=A0 =C2=A0static bool needs_validation =3D false; > >=20 > > =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 patc= h first. > >=20 >=20 > sinval message can be raised by any operation over the pg_variable table. I set a watchpoint on "needs_validation", and when I run DROP VARIABLE, it = is called directly from the command: Hardware watchpoint 2: needs_validation Old value =3D false New value =3D true 0x00000000006ad44c in SessionVariableDropPostprocess (varid=3D) at session_variable.c:169 169 svar->drop_lxid =3D MyProc->vxid.lxid; (gdb) where #0 0x00000000006ad44c in SessionVariableDropPostprocess (varid=3D) at session_variable.c:169 #1 0x0000000000625dec in DropVariableById (varid=3D) at pg_= variable.c:259 #2 0x00000000005fec57 in doDeletion (object=3D0x4ac85e8, flags=3D0) at dep= endency.c:1450 ... #8 0x00000000008bb7e0 in standard_ProcessUtility (pstmt=3D0x49ac700, query= String=3D0x49abbb0 "DROP VARIABLE a;", readOnlyTree=3D,=20 context=3DPROCESS_UTILITY_TOPLEVEL, params=3D0x0, queryEnv=3D0x0, dest= =3D0x49acac0, qc=3D0x7ffe19d2db60) at utility.c:1076 ... #12 0x00000000008b6742 in exec_simple_query (query_string=3D0x49abbb0 "DROP= VARIABLE a;") at postgres.c:1283 Later, there is a sinval message and "pg_variable_cache_callback()" is call= ed, which sets "needs_validation" again. Perhaps my confustion is as follows: if DROP VARIABLE sends an invalidation= message, and the handler sets "needs_validation", why is it necessary to set "needs_= validation" in SessionVariableDropPostprocess() too? If we didn't set "needs_validation" in SessionVariableDropPostprocess(), th= e comment would be true. > > - 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 wi= ll affect > > =C2=A0 all transactions, and I don't see the benefit. >=20 > If I remember well, there were two reasons why I did it. >=20 > 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 n= ear time. > The rule - allocated space is released, when in the next transaction y= ou use any > session variable looks a little bit crazy (although I think so there w= ill not be real > significant difference in functionality). > Correct me, if I am wrong, but I don't remember any memory (or resourc= e) 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 so= me hooks there as alternative I don't think that is a good enough reason. Memory used by an idle backend is not totally predictable for other reasons= as well: - the catalog cache - memory that PostgreSQL freed, but that is kept in the malloc arena so tha= t it can be reused for the next malloc() call I believe that the disadvantage of keeping some memory allocated across tra= nsaction is not as bad as the pain of going through all variables on each COMMIT. If you have set one or two session variables and run a lot of statements in autocommit mode, that is an unnecessary overhead. > 2. The main reason why it is implemented is implementation of temporal va= riables > with RESET or DROP on transaction end. Related code should be triggere= d at > commit time, it cannot be delayed. That makes sense. If I remember right, temporary variables are an optional feature. So I suggest that you move AtPreEOXact_SessionVariables() to the patch that= adds temporary session variables. > > =C2=A0 Also, do we need to call it during pg_session_variables()? >=20 > I think it can be removed. Originally pg_session_variables showed only va= lid variables, but it is not true now. Right. I'll wait for your reaction to my above suggestions before I start working = on the comments. Yours, Laurenz Albe