(unnamed)
text/html
<div> </div><div> </div><div>10.08.2024, 16:40, "Kirill Reshke" <reshkekirill@gmail.com>:</div><blockquote><p>On Thu, 4 Jul 2024 at 21:46, Sergey Solovev<br /><<a href="mailto:sergey.soloviev@tantorlabs.ru" rel="noopener noreferrer">sergey.soloviev@tantorlabs.ru</a>> wrote:</p><blockquote><br /> Hi.<br /><br /> We encountered a problem with excessive logging when transaction is<br /> sampled.<br /> When it is getting sampled every statement is logged, event SELECT. This<br /> can<br /> lead to performance degradation and log polluting.<br /> I have added new setting to filter statements when transaction is<br /> sampled - log_transaction.<br /><br /> Overview<br /> ===================<br /> This parameter works very similar to log_statement, but it filters out<br /> statements only<br /> in sampled transaction. It has same values as log_statement, access<br /> rights (only superuser),<br /> setup in postgresql.conf or by superuser with SET.<br /> But by default it has value "all" in compliance with existing behaviour<br /> (to log every statement).<br /><br /> Example usage<br /> ==================<br /> Log every DDL, but only subset of other statements.<br /><br /> postgresql.conf<br /> > log_transaction = ddl<br /> > log_statement = all<br /> > log_transaction_sample_rate = 1<br /> > log_statement_sample_rate = 0.3<br /><br /> backend:<br /> > BEGIN;<br /> > CREATE TABLE t1(value text);<br /> > INSERT INTO t1(value) VALUES ('hello'), ('world');<br /> > SELECT * FROM t1;<br /> > DROP TABLE t1;<br /> > COMMIT;<br /><br /> logfile:<br /> > LOG: duration: 6.465 ms statement: create table t1(value text);<br /> > LOG: statement: insert into t1(value) values ('hello'), ('world');<br /> > LOG: duration: 6.457 ms statement: drop table t1;<br /><br /> As you can see CREATE and DROP were logged with duration (because it is<br /> DDL), but<br /> only INSERT was logged (without duration) - not SELECT.<br /><br /> Testing<br /> ===================<br /> All tests are passed - default configuration is compatible with existing<br /> behaviour.<br /> Honestly, I could not find any TAP/regress tests that would test logging<br /> - some<br /> tests only use logs to ensure test results.<br /> I did not find suitable place for such tests, so no tests provided<br /><br /> Implementation details<br /> ===================<br /> I have modified signature of check_log_duration function - this accepts<br /> List of<br /> statements that were executed. This is to decide whether we should log<br /> current statement if transaction is sampled.<br /> But this list can be empty, in that case we decide to log. NIL is passed<br /> only<br /> in fast path, PARSE and BIND functions - by default these are logged<br /> if transaction is sampled, so we are compliant with existing behaviour<br /> again.<br /> In first implementation version, there was boolean flag (instead of<br /> List), but<br /> it was replaced by List to defer determining (function call) whether it<br /> is worth logging.<br /><br /> -------------<br /> Best regards, Sergey Solovev</blockquote><p><br />Hi!<br /><br />As I understand, the need of this GUC variable comes from fact, that<br />is the transaction is sampled for logging, all statements within this<br />tx are logged and this is not configurable now, correct?<br />Well, if this is the case, why should we add a new GUC? Maybe we<br />should use `log_statement` in this case too, so there is a bug, that<br />log_statement is honored not during tx sampling?<br /><br /><br />Also, tests are failing[1]<br /><br /><br />[1] <a href="https://cirrus-ci.com/task/5645711230894080" rel="noopener noreferrer">https://cirrus-ci.com/task/5645711230894080</a><br /> </p>--<br />Best regards,<br />Kirill Reshke</blockquote><div>Hi, Kirill!</div><div> </div><div>Thanks for review.</div><div><blockquote><div><div>Also, tests are failing[1]</div></div></blockquote></div><div>That is my mistake: I have tested patch on REL_17BETA2, but set 18 version, and</div><div>it seems that I run tests without proper ./configure flags. I reproduced error and fixed</div><div>it in new patch.</div><blockquote><div><div>As I understand, the need of this GUC variable comes from fact, that<br />is the transaction is sampled for logging, all statements within this<br />tx are logged and this is not configurable now, correct?</div></div></blockquote><div>Yes. That's the point - all or nothing.</div><blockquote><div><div>Well, if this is the case, why should we add a new GUC? Maybe we<br />should use `log_statement` in this case too, so there is a bug, that<br />log_statement is honored not during tx sampling?</div></div></blockquote><div><div>That can be good solution, but I proceed from possibility of flexible logging setup.</div><div>Shown example (when all DDL statements were logged) is just one of such use cases.</div><div>Another example is security policy: when we must log every data modification -</div><div>set `log_transaction = mod`.</div></div>