Re: Log a sample of transactions

On 3/26/19 1:54 AM, Kuroda, Hayato wrote:
> Dear David,
> 
> I have a will and already read the patch, but I thought it's not my turn.
> Sorry.
> 
> 

Hello,

> Adrien,
> 
>>   I did not find any test for log_min_duration that could help me. LCOV indicate
>> there is no tests on this part (look check_log_duration()):
>> https://coverage.postgresql.org/src/backend/tcop/postgres.c.gcov.html
> 
> I understand the unnecessarily of some test case. It's OK.
> 
> Finally, how do you think about the deviation of randomness?
> If this parameter is set very low, nobody may be output because of the deviation.
> we can avoid this phenomenon by counting up internal parameter for each transactions and output to log file if the
parameterbecomes  more than 1.
 

I don't understand what you want to do and I am afraid of the cost it 
could add.

There was a long discussion about random, maybe the thread could answer 
some of your questions?
https://www.postgresql.org/message-id/3859.1545849900%40sss.pgh.pa.us

Furthermore, I wonder if it is really necessary : the goal of this patch 
is to log a sample of transaction, not to perform precise stats 
computation. FIY we do not perform this kind of checks for 
log_statement_sample_rate.


> 
> After consideration for this case and rebasing, I think this patch is enough.
> Do I have to measure the change of throughput?
> 

I don't think it is necessary or maybe I am missing something. I already 
perform performance test for log_statement_sample_rate:
https://www.postgresql.org/message-id/8a608ccf-f78b-dfad-4c5f-8e6fd068c59c%40anayrat.info

It should be less expensive as we call random() for new transaction 
only. log_statement_sample_rate call it for each statement.

I don't know if there is cases where random() call could be slow?

There is still a point raised by Andres:


>> As far as I can tell xact_is_sampled is not properly reset in case of
>> errors?
>>
> 
> I am not sure if I should disable logging in case of errors. Actually we have:
> 
> postgres=# set log_transaction_sample_rate to 1;
> SET
> postgres=# set client_min_messages to 'LOG';
> LOG:  duration: 0.392 ms  statement: set client_min_messages to 'LOG';
> SET
> postgres=# begin;
> LOG:  duration: 0.345 ms  statement: begin;
> BEGIN
> postgres=# select 1;
> LOG:  duration: 0.479 ms  statement: select 1;
>  ?column?
> ----------
>         1
> (1 row)
> 
> postgres=# select * from missingtable;
> ERROR:  relation "missingtable" does not exist
> LINE 1: select * from missingtable;
>                       ^
> postgres=# select 1;
> ERROR:  current transaction is aborted, commands ignored until end of transaction block
> postgres=# rollback;
> LOG:  duration: 11390.295 ms  statement: rollback;
> ROLLBACK
> 
> If I reset xact_is_sampled (after the first error inside AbortTransaction if I am right), "rollback" statement will
notbe logged. I wonder if this "statement" should be logged?
 
> 
> If the answer is no, I will reset xact_is_sampled in AbortTransaction.


Thanks!