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 1td7S9-004xc6-R9 for pgsql-hackers@arkaria.postgresql.org; Wed, 29 Jan 2025 12:45:58 +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 1td7S7-006rSj-UL for pgsql-hackers@arkaria.postgresql.org; Wed, 29 Jan 2025 12:45:55 +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 1td7S7-006rSZ-Jh for pgsql-hackers@lists.postgresql.org; Wed, 29 Jan 2025 12:45:55 +0000 Received: from mail-lf1-x131.google.com ([2a00:1450:4864:20::131]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1td7S4-002AZ3-22 for pgsql-hackers@postgresql.org; Wed, 29 Jan 2025 12:45:54 +0000 Received: by mail-lf1-x131.google.com with SMTP id 2adb3069b0e04-53e389d8dc7so7309108e87.0 for ; Wed, 29 Jan 2025 04:45:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738154751; x=1738759551; darn=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=hV2cFthu9pvU8vb/QDNKAoFKID7WIwdMfiTKq8PfUOA=; b=D4gmc42sWkcMutEKVfqK/0+z/dxVLQVqc0LtbuiyhFgqu2TBs5UJHsXOOUlnae8f1y uyVCeDPIv7CDHX3RFPfcB5d4dtbKDJVrF7G5sYc88BiS2Y72+J6M9d68NyphBt771c95 tZ5PQk4Lb+36RhthiBJXkvXw0n3WxhmPC8RG/O6FZl4QlXIBF+TzlOJZYnS9TjRvdB0r +buJPLByNQ+M8u4Mi/hci2AsTz+6ykXidqBSvCgS8orDHbt6s49hogMKFGUMZvB/fvK1 tX3uD3v8LRBGI3m+UFwxo7kglhK+4yDLRg+sEu53B8jzyCthfVdUvVGaEUm7gGzyH8Aq TgPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738154751; x=1738759551; 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=hV2cFthu9pvU8vb/QDNKAoFKID7WIwdMfiTKq8PfUOA=; b=gJeE9YIbctvrAfKf97t/ceo3bpWld3s7DPmGQx243VObELQDaGuedYJNwKO3OuV/Y/ rKpQz2UVKwTuISTii73+ov3J4baDnWcijWFHFHh2N0IvhE4ZU5K6OtYXjuUoNxhRct03 Vk0eJFbwBaFEH4uuP38yf1gD8aSaBpjD2WVD/dYzZP1hlJ5odjk9bz3B8RZkeOpz5Okc D3ooQCwqjaXZvhBHwj/6ZsF1+GK8t5rgH78FmSBQamxTATJjhGyaibuYLRRjU/C93S51 Dn6x6kbAjLhzj/zKn3DTdr4U+aFqsRXyT0+GZtZ62GZKT7goEwcABC15UB6sd28iujHK nAVA== X-Gm-Message-State: AOJu0YzNh00WvZyqdgJoVETutyop4pNFyTRugc/ntUXmOqOjFqJnIO0c 0KUl5EiyNIY5LdVphfrrT6sviKaVW/zs5Sp1sJSkspqeVIzN35ro/Yvog7Zj8D8SVLTLfqkTD5O 0vFRztsMMMwZNr4qeybbzfVC777k= X-Gm-Gg: ASbGncsAUVuJ7Ux6nJqHDKiTt8EitK7Jm0e2MfXYCajmc1UFpBtMRp+Lk87symnSS2A 8GjJHqfN5uKguRtygf8CZnzpisuAYNuk9mWtYIdbwi4H2OrGbOV5Zkw/jaMYO87NjB/uAwaluUn +Ed8rXSRoZD/i5jNOUd7fstFQrJgbr X-Google-Smtp-Source: AGHT+IEPWJidIULGWNFnPZPfjQ8r4HvKv1YTjBNDyaYw0AarEhS/wwyZvOWAJd5cFr39AbXX+Qu7D1RCo1sI1C1gmWM= X-Received: by 2002:a05:6512:2344:b0:542:1b63:141b with SMTP id 2adb3069b0e04-543e4c2b0b4mr1130941e87.32.1738154750904; Wed, 29 Jan 2025 04:45:50 -0800 (PST) MIME-Version: 1.0 References: <202411141148.vqxmwtn2ln25@alvherre.pgsql> <7d4db5cf-8d72-4ee1-8f2e-f88ef9615f27@vondra.me> <637b3ae3-b9eb-4d83-a942-5f7483911ea4@vondra.me> <29ced66e28ec696d1ac29747bd901df2@oss.nttdata.com> <5a899047-65fc-47c5-9c35-1035f44956c6@vondra.me> <70146bd0-da0f-48ee-bece-1e35536189e0@oss.nttdata.com> <2eb8fd76-89f0-42e8-bc79-152d50c63016@vondra.me> In-Reply-To: <2eb8fd76-89f0-42e8-bc79-152d50c63016@vondra.me> From: Rahila Syed Date: Wed, 29 Jan 2025 18:15:38 +0530 X-Gm-Features: AWEUYZnNF-6vgofakd6hIj38tJP5aRdbkBYfPKcfaIuA7TJP1f2UAaW0RY_NQuU Message-ID: Subject: Re: Enhancing Memory Context Statistics Reporting To: Tomas Vondra Cc: PostgreSQL-development , Fujii Masao Content-Type: multipart/alternative; boundary="0000000000007e0e6b062cd7b00b" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --0000000000007e0e6b062cd7b00b Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, On Sat, Jan 25, 2025 at 3:50=E2=80=AFAM Tomas Vondra wrot= e: > > > On 1/24/25 14:47, Rahila Syed wrote: > > > > Hi, > > > > > > Just idea; as an another option, how about blocking new requests to > > the target process (e.g., causing them to fail with an error or > > returning NULL with a warning) if a previous request is still > pending? > > Users can simply retry the request if it fails. IMO failing quickly > > seems preferable to getting stuck for a while in cases with > concurrent > > requests. > > > > Thank you for the suggestion. I agree that it is better to fail > > early and avoid waiting for a timeout in such cases. I will add a > > "pending request" tracker for this in shared memory. This approach > > will help prevent sending a concurrent request if a request for the > > same backend is still being processed. > > > > AFAIK these failures should be extremely rare - we're only talking about > that because the workload I used for testing is highly concurrent, i.e. > it requests memory context info extremely often. I doubt anyone sane is > going to do that in practice ... Yes, that makes sense. > > > IMO, one downside of throwing an error in such cases is that the > > users might wonder if they need to take a corrective action, even > > though the issue is actually going to solve itself and they just > > need to retry. Therefore, issuing a warning or displaying previously > > updated statistics might be a better alternative to throwing an > > error. > > > > Wouldn't this be mostly mitigated by adding proper detail/hint to the > error message? Sure, the user can always ignore that (especially when > calling this from a script), but well ... we can only do so much. > OK. All this makes me think about how we shared pgstat data before the shmem > approach was introduced in PG15. Until then the process signaled pgstat > collector, and the collector wrote the statistics into a file, with a > timestamp. And the process used the timestamp to decide if it's fresh > enough ... Wouldn't the same approach work here? > > I imagined it would work something like this: > > requesting backend: > ------------------- > * set request_ts to current timestamp > * signal the target process, to generate memory context info > * wait until the DSA gets filled with stats_ts > request_ts > * return the data, don't erase anything > > target backend > -------------- > * clear the signal > * generate the statistics > * set stats_ts to current timestamp > * wait all the backends waiting for the stats (through CV) > > I see v11 does almost this, except that it accepts somewhat stale data. > That's correct. > But why would that be necessary? I don't think it's needed, and I don't > think we should accept data from before the process sends the signal. > > This is done in an attempt to avoid concurrent requests from timing out. In such cases, data in response to another request is likely to already be in the dynamic shared memory. Hence instead of waiting for the latest data and risking a timeout, the approach displays available statistics that are newer than a defined threshold. Additionally, since we can't distinguish between sequential and concurrent requests, we accept somewhat stale data for all requests. I realize this approach has some issues, mainly regarding how to determine an appropriate threshold value or a limit for old data. Therefore, I agree that it makes sense to display the data that is published after the request is made. If such data can't be published due to concurrent requests or other delays, the function should detect this and return as soon as possible. Thank you, Rahila Syed --0000000000007e0e6b062cd7b00b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi,

On Sat, Jan 25, 2025 at 3:50= =E2=80=AFAM Tomas Vondra <tomas@vondr= a.me> wrote:


On 1/24/25 14:47, Rahila Syed wrote:
>
> Hi,
>
>
>=C2=A0 =C2=A0 =C2=A0Just idea; as an another option, how about blocking= new requests to
>=C2=A0 =C2=A0 =C2=A0the target process (e.g., causing them to fail with= an error or
>=C2=A0 =C2=A0 =C2=A0returning NULL with a warning) if a previous reques= t is still pending?
>=C2=A0 =C2=A0 =C2=A0Users can simply retry the request if it fails. IMO= failing quickly
>=C2=A0 =C2=A0 =C2=A0seems preferable to getting stuck for a while in ca= ses with concurrent
>=C2=A0 =C2=A0 =C2=A0requests.
>
> Thank you for the suggestion. I agree that it is better to fail
> early and avoid waiting for a timeout in such cases. I will add a
> "pending request" tracker for this in shared memory. This ap= proach
> will help prevent sending a concurrent request if a request for the > same backend is still being processed.
>

AFAIK these failures should be extremely rare - we're only talking abou= t
that because the workload I used for testing is highly concurrent, i.e.
it requests memory context info extremely often. I doubt anyone sane is
going to do that in practice ...
Yes, that makes sense.=C2= =A0
=C2=A0
> IMO, one downside of throwing an error in such cases is that the
> users might wonder if they need to take a corrective action, even
> though the issue is actually going to solve itself and they just
> need to retry. Therefore, issuing a warning or displaying previously > updated statistics might be a better alternative to throwing an
> error.
>

Wouldn't this be mostly mitigated by adding proper detail/hint to the error message? Sure, the user can always ignore that (especially when
calling this from a script), but well ... we can only do so much.
=C2=A0
=C2=A0OK.

All this makes me think about how we shared pgstat data before the shmem approach was introduced in PG15. Until then the process signaled pgstat
collector, and the collector wrote the statistics into a file, with a
timestamp. And the process used the timestamp to decide if it's fresh enough ... Wouldn't the same approach work here?

I imagined it would work something like this:

requesting backend:
-------------------
* set request_ts to current timestamp
* signal the target process, to generate memory context info
* wait until the DSA gets filled with stats_ts > request_ts
* return the data, don't erase anything

target backend
--------------
* clear the signal
* generate the statistics
* set stats_ts to current timestamp
* wait all the backends waiting for the stats (through CV)

I see v11 does almost this, except that it accepts somewhat stale data.
=
That's correct.
=C2=A0
But why would that be necessary? I don't think it's needed, and I d= on't
think we should accept data from before the process sends the signal.
This is done in an=C2=A0attempt to avoid concurrent req= uests from timing out.
In such cases, data in response to another reques= t is likely to already be in the=C2=A0
dynamic shared memory. Hen= ce instead of waiting for the latest data and risking a=C2=A0
tim= eout, the approach displays available statistics that are newer than a defi= ned
threshold. Additionally, since we can't distinguish between sequ= ential and=C2=A0
concurrent requests, we accept somewhat stale da= ta for all requests.

I realize this approach has some issues,= mainly regarding how to determine
an appropriate threshold value= or a limit for old data.

Therefore, I agree that it makes sense to = display the data that is published=C2=A0
after the request is mad= e. If such data can't be published due to concurrent=C2=A0
re= quests or other delays, the=C2=A0function should detect this and return as = soon as=C2=A0
possible.


Thank you,
Rahila Syed
--0000000000007e0e6b062cd7b00b--