Content-Type: multipart/alternative;
 boundary="------------rxAxpSKe0DX55xg1VTXG0AIz"
Message-ID: <e3d143e1-0eb3-48db-b16e-4c0aa18e6f53@postgrespro.ru>
Date: Wed, 15 Jan 2025 22:18:56 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Sample rate added to pg_stat_statements
To: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
Cc: "Andrey M. Borodin" <x4mmm@yandex-team.ru>,
 Sami Imseih <samimseih@gmail.com>, Alexander Korotkov
 <aekorotkov@gmail.com>, Michael Paquier <michael@paquier.xyz>,
 Greg Sabino Mullane <htamfids@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <fe99e0ca-e564-480e-b865-5f0cee30bc60@tantorlabs.com>
 <Zz0MFPq1s4WFxxpL@paquier.xyz>
 <CAPpHfdsTKAQqC3A48-MGQhrhfEamXZPb64w=utk7thQcOMNr7Q@mail.gmail.com>
 <320a31e6-4c91-438b-ab17-8a1d72384727@tantorlabs.com>
 <f202ed0f-28c3-45aa-884c-db0a78322342@tantorlabs.com>
 <fb3df2e0-2cd4-49ea-bfda-148bc43d613d@tantorlabs.com>
 <52e5e3aa-066a-4daf-8d65-77c34a2f0d98@tantorlabs.com>
 <6482A576-4465-4E46-A42C-F1767E8DDE30@yandex-team.ru>
 <CAA5RZ0s=JH4owVr=Tfr4DYpFnBrYJjKoq2d7NqdkBQKY5whiRQ@mail.gmail.com>
 <20994d10-ab44-4c6b-91bc-c4675cd2e58e@tantorlabs.com>
 <9D8A5BC8-A3A5-4DA9-9382-F30D343247D1@yandex-team.ru>
 <CAA5RZ0sBsajHdE_HPoJ-C5rp8QmfArd6a6yJo6bDMVpyW25E+Q@mail.gmail.com>
 <6cf6270b-e8fa-4d99-b74a-17e781cdf309@tantorlabs.com>
 <b40b76c3-fc54-4f11-a50f-e63cd6434560@postgrespro.ru>
 <3a27c830-b81f-4560-9774-a87d9b922fbd@tantorlabs.com>
 <CAA5RZ0vYu+dm3KKd5iaQ=LY0J2sjfxJG+-VCfkeVtWFuhSb-4A@mail.gmail.com>
 <cebfee9c-a894-46f1-b6bc-5bf4a0753b41@tantorlabs.com>
Content-Language: en-US
From: Alena Rybakina <a.rybakina@postgrespro.ru>
In-Reply-To: <cebfee9c-a894-46f1-b6bc-5bf4a0753b41@tantorlabs.com>
Archived-At: <https://www.postgresql.org/message-id/e3d143e1-0eb3-48db-b16e-4c0aa18e6f53%40postgrespro.ru>
Precedence: bulk

This is a multi-part message in MIME format.
--------------rxAxpSKe0DX55xg1VTXG0AIz
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 15.01.2025 12:47, Ilia Evdokimov wrote:
> On 06.01.2025 18:57, Andrey M. Borodin wrote:
>> 1. This code seems a little different from your patch. It is trying to avoid engaging PRNG. I'm not sure it's a good idea, but still. Also, it uses "<=", not "<".
>>
>>      xact_is_sampled = log_xact_sample_rate != 0 &&
>> (log_xact_sample_rate == 1 ||
>> pg_prng_double(&pg_global_prng_state) <= log_xact_sample_rate);
>
> Sorry for the delayed reply. Andrey was right about this 
> suggestion—first, it makes the code more readable for others, and 
> second, it avoids engaging the PRNG on edge values of 0.0 and 1.0. 
> I’ve attached patch v11 with these changes.
>
Patch looks fine. Thank you!

On 14.01.2025 15:00, Ilia Evdokimov wrote:
>
> Alena, Sami – I apologize for not including you in the previous email. 
> If you're interested in this approach, I'm open to any suggestions.
>
> [0]: 
> https://www.postgresql.org/message-id/1b13d748-5e98-479c-9222-3253a734a038%40tantorlabs.com 
>
>
This is a difficult question. I tend to agree with Alexander Korotkov's 
proposal to add a filter that saves information about queries whose 
statistical information satisfies the conditions of the configured 
filter. However, I don’t believe query execution time is a sufficient 
metric for this purpose. It is too unstable and influenced by many 
external factors, such as system load. For instance, various background 
processes like vacuum, checkpointer, or background writer mechanisms 
could be running simultaneously.

Additionally, a query may take a long time to execute because another 
large query is consuming most of the system's resources at the same 
time. In such cases, the long execution time of the triggered query may 
not indicate anything remarkable. Furthermore, statistics for 
resource-intensive queries may appear normal if the query simply 
processed a large volume of data, such as when it involves a Cartesian 
product or a full join.

Therefore, I think the idea of using filters is more promising, 
especially if multiple filters are implemented. For example, we could 
add filters for buffer usage (pages read or modified), differences in 
cardinality (predicted vs. actual), username, application name, and 
other criteria. These filters would help reduce the volume of queries 
tracked by the pg_stat_statements extension. However, there might be 
challenges in keeping this state up to date, given the volatile and 
unstable nature of the system.

Your proposed parameter could also reduce the volume of queries for 
which statistics need to be saved, but it is too unpredictable for 
analysis. This unpredictability makes it unclear how the data would be 
interpreted in the future. For instance, a query that genuinely impacted 
performance might not be processed by pg_stat_statements simply due to 
randomness, while a small, insignificant query could be selected 
instead. Analyzing such statistical information might lead to misleading 
conclusions.

-- 
Regards,
Alena Rybakina
Postgres Professional

--------------rxAxpSKe0DX55xg1VTXG0AIz
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">On 15.01.2025 12:47, Ilia Evdokimov
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:cebfee9c-a894-46f1-b6bc-5bf4a0753b41@tantorlabs.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <div class="moz-cite-prefix">On 06.01.2025 18:57, Andrey M.
        Borodin wrote:<span style="white-space: pre-wrap">
</span><span style="white-space: pre-wrap">
</span></div>
      <blockquote type="cite"
        cite="mid:6482A576-4465-4E46-A42C-F1767E8DDE30@yandex-team.ru">
        <pre class="moz-quote-pre" wrap="">1. This code seems a little different from your patch. It is trying to avoid engaging PRNG. I'm not sure it's a good idea, but still. Also, it uses "&lt;=", not "&lt;".

    xact_is_sampled = log_xact_sample_rate != 0 &amp;&amp;
(log_xact_sample_rate == 1 ||
pg_prng_double(&amp;pg_global_prng_state) &lt;= log_xact_sample_rate);
</pre>
      </blockquote>
      <p>Sorry for the delayed reply. Andrey was right about this
        suggestion—first, it makes the code more readable for others,
        and second, it avoids engaging the PRNG on edge values of 0.0
        and 1.0. I’ve attached patch v11 with these changes.</p>
    </blockquote>
    <p>Patch looks fine. Thank you!</p>
    <div class="moz-cite-prefix">On 14.01.2025 15:00, Ilia Evdokimov
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:3a27c830-b81f-4560-9774-a87d9b922fbd@tantorlabs.com"> <br>
      Alena, Sami – I apologize for not including you in the previous
      email. If you're interested in this approach, I'm open to any
      suggestions. <br>
      <br>
      [0]:
<a class="moz-txt-link-freetext" href="https://www.postgresql.org/message-id/1b13d748-5e98-479c-9222-3253a734a038%40tantorlabs.com">https://www.postgresql.org/message-id/1b13d748-5e98-479c-9222-3253a734a038%40tantorlabs.com</a>
      <br>
      <br>
    </blockquote>
    This is a difficult question. I tend to agree with Alexander
    Korotkov's proposal to add a filter that saves information about
    queries whose statistical information satisfies the conditions of
    the configured filter. However, I don’t believe query execution time
    is a sufficient metric for this purpose. It is too unstable and
    influenced by many external factors, such as system load. For
    instance, various background processes like vacuum, checkpointer, or
    background writer mechanisms could be running simultaneously.<br>
    <br>
    Additionally, a query may take a long time to execute because
    another large query is consuming most of the system's resources at
    the same time. In such cases, the long execution time of the
    triggered query may not indicate anything remarkable. Furthermore,
    statistics for resource-intensive queries may appear normal if the
    query simply processed a large volume of data, such as when it
    involves a Cartesian product or a full join.<br>
    <br>
    Therefore, I think the idea of using filters is more promising,
    especially if multiple filters are implemented. For example, we
    could add filters for buffer usage (pages read or modified),
    differences in cardinality (predicted vs. actual), username,
    application name, and other criteria. These filters would help
    reduce the volume of queries tracked by the pg_stat_statements
    extension. However, there might be challenges in keeping this state
    up to date, given the volatile and unstable nature of the system.<br>
    <br>
    Your proposed parameter could also reduce the volume of queries for
    which statistics need to be saved, but it is too unpredictable for
    analysis. This unpredictability makes it unclear how the data would
    be interpreted in the future. For instance, a query that genuinely
    impacted performance might not be processed by pg_stat_statements
    simply due to randomness, while a small, insignificant query could
    be selected instead. Analyzing such statistical information might
    lead to misleading conclusions.<br>
    <p></p>
    <p></p>
    <pre class="moz-signature" cols="72">-- 
Regards,
Alena Rybakina
Postgres Professional</pre>
  </body>
</html>

--------------rxAxpSKe0DX55xg1VTXG0AIz--