public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 1/3] bootstrap: convert Typ to a List*
5+ messages / 3 participants
[nested] [flat]
* [PATCH 1/3] bootstrap: convert Typ to a List*
@ 2020-11-20 02:48 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Justin Pryzby @ 2020-11-20 02:48 UTC (permalink / raw)
---
src/backend/bootstrap/bootstrap.c | 69 ++++++++++++++-----------------
1 file changed, 31 insertions(+), 38 deletions(-)
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 6f615e6622..18eb62ca47 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -159,7 +159,7 @@ struct typmap
FormData_pg_type am_typ;
};
-static struct typmap **Typ = NULL;
+static List *Typ = NIL; /* List of struct typmap* */
static struct typmap *Ap = NULL;
static Datum values[MAXATTR]; /* current row's attribute values */
@@ -597,7 +597,7 @@ boot_openrel(char *relname)
* pg_type must be filled before any OPEN command is executed, hence we
* can now populate the Typ array if we haven't yet.
*/
- if (Typ == NULL)
+ if (Typ == NIL)
populate_typ_array();
if (boot_reldesc != NULL)
@@ -688,7 +688,7 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
typeoid = gettype(type);
- if (Typ != NULL)
+ if (Typ != NIL)
{
attrtypes[attnum]->atttypid = Ap->am_oid;
attrtypes[attnum]->attlen = Ap->am_typ.typlen;
@@ -877,36 +877,25 @@ populate_typ_array(void)
Relation rel;
TableScanDesc scan;
HeapTuple tup;
- int nalloc;
- int i;
-
- Assert(Typ == NULL);
- nalloc = 512;
- Typ = (struct typmap **)
- MemoryContextAlloc(TopMemoryContext, nalloc * sizeof(struct typmap *));
+ Assert(Typ == NIL);
rel = table_open(TypeRelationId, NoLock);
scan = table_beginscan_catalog(rel, 0, NULL);
- i = 0;
while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Form_pg_type typForm = (Form_pg_type) GETSTRUCT(tup);
+ struct typmap *newtyp;
+ MemoryContext old;
- /* make sure there will be room for a trailing NULL pointer */
- if (i >= nalloc - 1)
- {
- nalloc *= 2;
- Typ = (struct typmap **)
- repalloc(Typ, nalloc * sizeof(struct typmap *));
- }
- Typ[i] = (struct typmap *)
- MemoryContextAlloc(TopMemoryContext, sizeof(struct typmap));
- Typ[i]->am_oid = typForm->oid;
- memcpy(&(Typ[i]->am_typ), typForm, sizeof(Typ[i]->am_typ));
- i++;
+ old = MemoryContextSwitchTo(TopMemoryContext);
+ newtyp = (struct typmap *) palloc(sizeof(struct typmap));
+ Typ = lappend(Typ, newtyp);
+ MemoryContextSwitchTo(old);
+
+ newtyp->am_oid = typForm->oid;
+ memcpy(&newtyp->am_typ, typForm, sizeof(newtyp->am_typ));
}
- Typ[i] = NULL; /* Fill trailing NULL pointer */
table_endscan(scan);
table_close(rel, NoLock);
}
@@ -925,16 +914,17 @@ populate_typ_array(void)
static Oid
gettype(char *type)
{
- if (Typ != NULL)
+ if (Typ != NIL)
{
- struct typmap **app;
+ ListCell *lc;
- for (app = Typ; *app != NULL; app++)
+ foreach (lc, Typ)
{
- if (strncmp(NameStr((*app)->am_typ.typname), type, NAMEDATALEN) == 0)
+ struct typmap *app = lfirst(lc);
+ if (strncmp(NameStr(app->am_typ.typname), type, NAMEDATALEN) == 0)
{
- Ap = *app;
- return (*app)->am_oid;
+ Ap = app;
+ return app->am_oid;
}
}
}
@@ -980,14 +970,17 @@ boot_get_type_io_data(Oid typid,
if (Typ != NULL)
{
/* We have the boot-time contents of pg_type, so use it */
- struct typmap **app;
- struct typmap *ap;
-
- app = Typ;
- while (*app && (*app)->am_oid != typid)
- ++app;
- ap = *app;
- if (ap == NULL)
+ struct typmap *ap = NULL;
+ ListCell *lc;
+
+ foreach (lc, Typ)
+ {
+ ap = lfirst(lc);
+ if (ap->am_oid == typid)
+ break;
+ }
+
+ if (!ap || ap->am_oid != typid)
elog(ERROR, "type OID %u not found in Typ list", typid);
*typlen = ap->am_typ.typlen;
--
2.26.2
--------------76B8D0DC8AE327D516738282
Content-Type: text/x-patch; charset=UTF-8;
name="0002-Allow-composite-types-in-bootstrap-20210108.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="0002-Allow-composite-types-in-bootstrap-20210108.patch"
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Truncate logs by max_log_size
@ 2024-11-28 14:22 Jim Jones <[email protected]>
2024-11-28 19:20 ` Re: Truncate logs by max_log_size Kirill Gavrilov <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Jim Jones @ 2024-11-28 14:22 UTC (permalink / raw)
To: Kirill Reshke <[email protected]>; Kirill Gavrilov <[email protected]>; +Cc: Andrey M. Borodin <[email protected]>; Euler Taveira <[email protected]>; [email protected]
On 25.11.24 17:52, Kirill Reshke wrote:
> Hello! Please deliver the most recent patch version and fix the issues
> Jim identified [0] as the current commitfest draws to a close.
> Do not forget to include both parts of this patch (This was actually
> developed off-list, and we are now using this on our cloud PostgreSQL
> distribution on versions 12–17).
>
> [0] https://www.postgresql.org/message-id/35096a36-04d4-480b-a7cd-a2d8151fb737%40uni-muenster.de
In addition to these points, this feature seems to fail with queries
containing special characters (more than one byte):
With this "max_log_size"..
postgres=# SHOW max_log_size;
max_log_size
--------------
20
(1 row)
... and this query ..
postgres=# SELECT "ÄÜÖ" FROM t;
ERROR: relation "t" does not exist
LINE 1: SELECT "ÄÜÖ" FROM t;
.. this is the [truncated] log entry we get ..
2024-11-28 14:58:57.912 CET [2258876] ERROR: relation "t" does not
exist at character 19
2024-11-28 14:58:57.912 CET [2258876] STATEMENT: SELECT "ÄÜÖ" FROM
... although the query originally had exactly 20 characters:
postgres=# SELECT length('SELECT "ÄÜÖ" FROM t;');
length
--------
20
(1 row)
postgres=# SELECT length('ÄÜÖ'::bytea), length('AUO'::bytea);
length | length
--------+--------
6 | 3
(1 row)
If it is supposed to be like this, it should be clearly stated so in the
docs.
--
Jim
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Truncate logs by max_log_size
2024-11-28 14:22 Re: Truncate logs by max_log_size Jim Jones <[email protected]>
@ 2024-11-28 19:20 ` Kirill Gavrilov <[email protected]>
2024-11-29 00:24 ` Re: Truncate logs by max_log_size Jim Jones <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Kirill Gavrilov @ 2024-11-28 19:20 UTC (permalink / raw)
To: Jim Jones <[email protected]>; +Cc: Kirill Reshke <[email protected]>; Andrey M. Borodin <[email protected]>; Euler Taveira <[email protected]>; [email protected]
On Thu, Nov 28, 2024 at 5:23 PM Jim Jones <[email protected]> wrote:
>
>
> On 25.11.24 17:52, Kirill Reshke wrote:
> > Hello! Please deliver the most recent patch version and fix the issues
> > Jim identified [0] as the current commitfest draws to a close.
> > Do not forget to include both parts of this patch (This was actually
> > developed off-list, and we are now using this on our cloud PostgreSQL
> > distribution on versions 12–17).
> >
> > [0]
> https://www.postgresql.org/message-id/35096a36-04d4-480b-a7cd-a2d8151fb737%40uni-muenster.de
>
> In addition to these points, this feature seems to fail with queries
> containing special characters (more than one byte):
>
> With this "max_log_size"..
>
> postgres=# SHOW max_log_size;
> max_log_size
> --------------
> 20
> (1 row)
>
> ... and this query ..
>
> postgres=# SELECT "ÄÜÖ" FROM t;
> ERROR: relation "t" does not exist
> LINE 1: SELECT "ÄÜÖ" FROM t;
>
> .. this is the [truncated] log entry we get ..
>
> 2024-11-28 14:58:57.912 CET [2258876] ERROR: relation "t" does not
> exist at character 19
> 2024-11-28 14:58:57.912 CET [2258876] STATEMENT: SELECT "ÄÜÖ" FROM
>
> ... although the query originally had exactly 20 characters:
>
> postgres=# SELECT length('SELECT "ÄÜÖ" FROM t;');
> length
> --------
> 20
> (1 row)
>
>
> postgres=# SELECT length('ÄÜÖ'::bytea), length('AUO'::bytea);
> length | length
> --------+--------
> 6 | 3
> (1 row)
>
> If it is supposed to be like this, it should be clearly stated so in the
> docs.
>
> --
> Jim
>
> Here is version 3 of this patch. I found another place where this
setting can be applied.
Also added some documentation and specified that this setting truncates
queries by size in bytes.
Attachments:
[application/octet-stream] V3-0001-parameter-max_log_size-to-truncate-logs.patch (8.5K, ../../CA+E0NR6tnLDanSNwcqZM_-ReBXitAOFsoaBAZBqXYEx0Di+fwQ@mail.gmail.com/3-V3-0001-parameter-max_log_size-to-truncate-logs.patch)
download | inline diff:
From 9439eb1c5c194d38a6aa388d8d8ca34350ae300f Mon Sep 17 00:00:00 2001
From: diphantxm <[email protected]>
Date: Thu, 28 Nov 2024 21:24:59 +0300
Subject: [PATCH] parameter max_log_size to truncate logs
There is no need to log the entire query, because it may be large and take lots of space on disk. Parameter max_log_size set the maximum length for logged query. Everything beyond that length is truncated. Value 0 disables the parameter.
---
doc/src/sgml/config.sgml | 15 ++++++++
src/backend/tcop/postgres.c | 10 ++++-
src/backend/utils/error/elog.c | 38 +++++++++++++++++++
src/backend/utils/misc/guc_tables.c | 11 ++++++
src/backend/utils/misc/postgresql.conf.sample | 2 +
src/bin/pg_ctl/t/004_logrotate.pl | 15 ++++++++
src/include/utils/elog.h | 2 +
7 files changed, 91 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 76ab72db96..5110b61ba5 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7913,6 +7913,21 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
</listitem>
</varlistentry>
+ <varlistentry id="guc-max-log-size" xreflabel="max_log_size">
+ <term><varname>max_log_size</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>max_log_size</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ If greater than zero, each query logged is truncated to this many bytes.
+ Zero disables the setting.
+ Default value is 5kB.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
<sect2 id="runtime-config-logging-csvlog">
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 4b985bd056..549be13e5b 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -70,6 +70,7 @@
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "tcop/utility.h"
+#include "utils/elog.h"
#include "utils/guc_hooks.h"
#include "utils/injection_point.h"
#include "utils/lsyscache.h"
@@ -1029,11 +1030,14 @@ exec_simple_query(const char *query_string)
bool was_logged = false;
bool use_implicit_block;
char msec_str[32];
+ char* query_log;
/*
* Report query to various monitoring facilities.
*/
debug_query_string = query_string;
+ bool copied = false;
+ query_log = build_query_log(query_string, &copied);
pgstat_report_activity(STATE_RUNNING, query_string);
@@ -1078,7 +1082,7 @@ exec_simple_query(const char *query_string)
if (check_log_statement(parsetree_list))
{
ereport(LOG,
- (errmsg("statement: %s", query_string),
+ (errmsg("statement: %s", query_log),
errhidestmt(true),
errdetail_execute(parsetree_list)));
was_logged = true;
@@ -1377,7 +1381,7 @@ exec_simple_query(const char *query_string)
case 2:
ereport(LOG,
(errmsg("duration: %s ms statement: %s",
- msec_str, query_string),
+ msec_str, query_log),
errhidestmt(true),
errdetail_execute(parsetree_list)));
break;
@@ -1388,6 +1392,8 @@ exec_simple_query(const char *query_string)
TRACE_POSTGRESQL_QUERY_DONE(query_string);
+ if (query_log && copied)
+ pfree(query_log);
debug_query_string = NULL;
}
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 289059435a..aa9712ba64 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -111,6 +111,7 @@ int Log_destination = LOG_DESTINATION_STDERR;
char *Log_destination_string = NULL;
bool syslog_sequence_numbers = true;
bool syslog_split_messages = true;
+int max_log_size = 0;
/* Processed form of backtrace_functions GUC */
static char *backtrace_function_list;
@@ -1693,6 +1694,10 @@ EmitErrorReport(void)
CHECK_STACK_DEPTH();
oldcontext = MemoryContextSwitchTo(edata->assoc_context);
+ const char* old_query_string = debug_query_string;
+ bool copied = false;
+ debug_query_string = build_query_log(debug_query_string, &copied);
+
/*
* Reset the formatted timestamp fields before emitting any logs. This
* includes all the log destinations and emit_log_hook, as the latter
@@ -1733,6 +1738,12 @@ EmitErrorReport(void)
MemoryContextSwitchTo(oldcontext);
recursion_depth--;
+
+ if (debug_query_string && copied)
+ {
+ pfree(debug_query_string);
+ debug_query_string = old_query_string;
+ }
}
/*
@@ -3773,3 +3784,30 @@ write_stderr(const char *fmt,...)
#endif
va_end(ap);
}
+
+/*
+ * Apply truncation to build query that will be logged.
+ *
+ * If query needs to be truncated, copied will be set to true
+ * and returned string must be freed
+*/
+char*
+build_query_log(const char* query, bool *copied)
+{
+ *copied = false;
+ if (!query)
+ return NULL;
+
+ size_t query_len = strlen(query);
+ if (max_log_size == 0 || query_len < max_log_size)
+ {
+ return query;
+ }
+
+ *copied = true;
+ size_t query_log_len = pg_mbcliplen(query, query_len, max_log_size);
+ char* query_log = (char*)palloc(query_log_len+1);
+ memcpy(query_log, query, query_log_len);
+ query_log[query_log_len] = '\0';
+ return query_log;
+}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 9845abd693..64f21377ad 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -3724,6 +3724,17 @@ struct config_int ConfigureNamesInt[] =
NULL, NULL, NULL
},
+ {
+ {"max_log_size", PGC_SUSET, LOGGING_WHAT,
+ gettext_noop("Sets max size in bytes of logged statement."),
+ NULL
+ },
+ &max_log_size,
+ 5 * (1024 * 1024),
+ 0, INT_MAX,
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 407cd1e08c..44971cecc6 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -617,6 +617,8 @@
# bind-parameter values to N bytes;
# -1 means print in full, 0 disables
#log_statement = 'none' # none, ddl, mod, all
+#max_log_size = 0 # max size of logged statement_timeout (in bytes)
+ # 0 disables the feature
#log_replication_commands = off
#log_temp_files = -1 # log temporary files equal or larger
# than the specified size in kilobytes;
diff --git a/src/bin/pg_ctl/t/004_logrotate.pl b/src/bin/pg_ctl/t/004_logrotate.pl
index eacca1a652..1c1f89cb0a 100644
--- a/src/bin/pg_ctl/t/004_logrotate.pl
+++ b/src/bin/pg_ctl/t/004_logrotate.pl
@@ -69,6 +69,7 @@ log_destination = 'stderr, csvlog, jsonlog'
# these ensure stability of test results:
log_rotation_age = 0
lc_messages = 'C'
+max_log_size = 32
));
$node->start();
@@ -135,6 +136,20 @@ check_log_pattern('stderr', $new_current_logfiles, 'syntax error', $node);
check_log_pattern('csvlog', $new_current_logfiles, 'syntax error', $node);
check_log_pattern('jsonlog', $new_current_logfiles, 'syntax error', $node);
+$node->psql('postgres', 'INSERT INTO SOME_NON_EXISTANT_TABLE VALUES (TEST)');
+for (my $attempts = 0; $attempts < $max_attempts; $attempts++)
+{
+ eval {
+ $current_logfiles = slurp_file($node->data_dir . '/current_logfiles');
+ };
+ last unless $@;
+ usleep(100_000);
+}
+die $@ if $@;
+check_log_pattern('stderr', $current_logfiles, 'INSERT INTO SOME_NON_EXISTANT_TA(?!(BLE VALUES \(TEST\)))', $node);
+check_log_pattern('csvlog', $current_logfiles, 'INSERT INTO SOME_NON_EXISTANT_TA(?!(BLE VALUES \(TEST\)))', $node);
+check_log_pattern('jsonlog', $current_logfiles, 'INSERT INTO SOME_NON_EXISTANT_TA(?!(BLE VALUES \(TEST\)))', $node);
+
$node->stop();
done_testing();
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index e54eca5b48..f6689796a5 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -502,6 +502,7 @@ extern PGDLLIMPORT int Log_destination;
extern PGDLLIMPORT char *Log_destination_string;
extern PGDLLIMPORT bool syslog_sequence_numbers;
extern PGDLLIMPORT bool syslog_split_messages;
+extern PGDLLIMPORT int max_log_size;
/* Log destination bitmap */
#define LOG_DESTINATION_STDERR 1
@@ -516,6 +517,7 @@ extern void log_status_format(StringInfo buf, const char *format,
extern void DebugFileOpen(void);
extern char *unpack_sql_state(int sql_state);
extern bool in_error_recursion_trouble(void);
+extern char* build_query_log(const char* query, bool *copied);
/* Common functions shared across destinations */
extern void reset_formatted_start_time(void);
--
2.39.2 (Apple Git-143)
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Truncate logs by max_log_size
2024-11-28 14:22 Re: Truncate logs by max_log_size Jim Jones <[email protected]>
2024-11-28 19:20 ` Re: Truncate logs by max_log_size Kirill Gavrilov <[email protected]>
@ 2024-11-29 00:24 ` Jim Jones <[email protected]>
2024-11-29 11:05 ` Re: Truncate logs by max_log_size Kirill Gavrilov <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Jim Jones @ 2024-11-29 00:24 UTC (permalink / raw)
To: Kirill Gavrilov <[email protected]>; +Cc: Kirill Reshke <[email protected]>; Andrey M. Borodin <[email protected]>; Euler Taveira <[email protected]>; [email protected]
On 28.11.24 20:20, Kirill Gavrilov wrote:
> Here is version 3 of this patch. I found another place where this
> setting can be applied.
> Also added some documentation and specified that this setting
> truncates queries by size in bytes.
Thanks. It is now possible to change the parameter using SET
postgres=# SET max_log_size TO 15;
SET
postgres=# SHOW max_log_size ;
max_log_size
--------------
15
(1 row)
In the postgresql.conf the default value is set to 0
#max_log_size = 0
But if we take a look at the parameter it shows something else
postgres=# SHOW max_log_size ;
max_log_size
--------------
5242880
(1 row)
Perhaps it would be best to keep the default value in the
postgresql.conf consistent with the documentation and the actual
max_log_size value. IMHO 0 (or -1) should disable it.
There are also several compilation issues:
postgres.c: In function ‘exec_simple_query’:
postgres.c:1040:9: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
1040 | bool copied = false;
| ^~~~
elog.c: In function ‘EmitErrorReport’:
elog.c:1697:9: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
1697 | const char* old_query_string = debug_query_string;
| ^~~~~
elog.c:1744:23: warning: passing argument 1 of ‘pfree’ discards ‘const’
qualifier from pointer target type [-Wdiscarded-qualifiers]
1744 | pfree(debug_query_string);
| ^~~~~~~~~~~~~~~~~~
In file included from ../../../../src/include/postgres.h:47,
from elog.c:55:
../../../../src/include/utils/palloc.h:86:25: note: expected ‘void *’
but argument is of type ‘const char *’
86 | extern void pfree(void *pointer);
| ~~~~~~^~~~~~~
elog.c: In function ‘build_query_log’:
elog.c:3798:9: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
3798 | size_t query_len = strlen(query);
| ^~~~~~
elog.c:3801:24: warning: return discards ‘const’ qualifier from pointer
target type [-Wdiscarded-qualifiers]
3801 | return query;
| ^~~~~
elog.c:3805:9: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
3805 | size_t query_log_len = pg_mbcliplen(query, query_len,
max_log_size);
| ^~~~~~
and there is a trailing whitespace at:
+ * If query needs to be truncated, copied will be set to true
/home/jim/patches/max_log_query/V3-0001-parameter-max_log_size-to-truncate-logs.patch:141:
trailing whitespace.
* If query needs to be truncated, copied will be set to true
Checking patch doc/src/sgml/config.sgml...
Hunk #1 succeeded at 7865 (offset -48 lines).
Checking patch src/backend/tcop/postgres.c...
Hunk #1 succeeded at 71 (offset 1 line).
Hunk #2 succeeded at 1031 (offset 1 line).
Hunk #3 succeeded at 1083 (offset 1 line).
Hunk #4 succeeded at 1382 (offset 1 line).
Hunk #5 succeeded at 1393 (offset 1 line).
Checking patch src/backend/utils/error/elog.c...
Hunk #4 succeeded at 3781 (offset -3 lines).
Checking patch src/backend/utils/misc/guc_tables.c...
Hunk #1 succeeded at 3714 (offset -10 lines).
Checking patch src/backend/utils/misc/postgresql.conf.sample...
Hunk #1 succeeded at 615 (offset -2 lines).
Checking patch src/bin/pg_ctl/t/004_logrotate.pl...
Checking patch src/include/utils/elog.h...
Applied patch doc/src/sgml/config.sgml cleanly.
Applied patch src/backend/tcop/postgres.c cleanly.
Applied patch src/backend/utils/error/elog.c cleanly.
Applied patch src/backend/utils/misc/guc_tables.c cleanly.
Applied patch src/backend/utils/misc/postgresql.conf.sample cleanly.
Applied patch src/bin/pg_ctl/t/004_logrotate.pl cleanly.
Applied patch src/include/utils/elog.h cleanly.
warning: 1 line adds whitespace errors.
--
Jim
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Truncate logs by max_log_size
2024-11-28 14:22 Re: Truncate logs by max_log_size Jim Jones <[email protected]>
2024-11-28 19:20 ` Re: Truncate logs by max_log_size Kirill Gavrilov <[email protected]>
2024-11-29 00:24 ` Re: Truncate logs by max_log_size Jim Jones <[email protected]>
@ 2024-11-29 11:05 ` Kirill Gavrilov <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Kirill Gavrilov @ 2024-11-29 11:05 UTC (permalink / raw)
To: Jim Jones <[email protected]>; +Cc: Kirill Reshke <[email protected]>; Andrey M. Borodin <[email protected]>; Euler Taveira <[email protected]>; [email protected]
On Fri, Nov 29, 2024 at 3:24 AM Jim Jones <[email protected]> wrote:
>
>
> On 28.11.24 20:20, Kirill Gavrilov wrote:
> > Here is version 3 of this patch. I found another place where this
> > setting can be applied.
> > Also added some documentation and specified that this setting
> > truncates queries by size in bytes.
>
> Thanks. It is now possible to change the parameter using SET
>
> postgres=# SET max_log_size TO 15;
> SET
> postgres=# SHOW max_log_size ;
> max_log_size
> --------------
> 15
> (1 row)
>
> In the postgresql.conf the default value is set to 0
>
> #max_log_size = 0
>
> But if we take a look at the parameter it shows something else
>
> postgres=# SHOW max_log_size ;
> max_log_size
> --------------
> 5242880
> (1 row)
>
> Perhaps it would be best to keep the default value in the
> postgresql.conf consistent with the documentation and the actual
> max_log_size value. IMHO 0 (or -1) should disable it.
>
> There are also several compilation issues:
>
> postgres.c: In function ‘exec_simple_query’:
> postgres.c:1040:9: warning: ISO C90 forbids mixed declarations and code
> [-Wdeclaration-after-statement]
> 1040 | bool copied = false;
> | ^~~~
> elog.c: In function ‘EmitErrorReport’:
> elog.c:1697:9: warning: ISO C90 forbids mixed declarations and code
> [-Wdeclaration-after-statement]
> 1697 | const char* old_query_string = debug_query_string;
> | ^~~~~
> elog.c:1744:23: warning: passing argument 1 of ‘pfree’ discards ‘const’
> qualifier from pointer target type [-Wdiscarded-qualifiers]
> 1744 | pfree(debug_query_string);
> | ^~~~~~~~~~~~~~~~~~
> In file included from ../../../../src/include/postgres.h:47,
> from elog.c:55:
> ../../../../src/include/utils/palloc.h:86:25: note: expected ‘void *’
> but argument is of type ‘const char *’
> 86 | extern void pfree(void *pointer);
> | ~~~~~~^~~~~~~
> elog.c: In function ‘build_query_log’:
> elog.c:3798:9: warning: ISO C90 forbids mixed declarations and code
> [-Wdeclaration-after-statement]
> 3798 | size_t query_len = strlen(query);
> | ^~~~~~
> elog.c:3801:24: warning: return discards ‘const’ qualifier from pointer
> target type [-Wdiscarded-qualifiers]
> 3801 | return query;
> | ^~~~~
> elog.c:3805:9: warning: ISO C90 forbids mixed declarations and code
> [-Wdeclaration-after-statement]
> 3805 | size_t query_log_len = pg_mbcliplen(query, query_len,
> max_log_size);
> | ^~~~~~
>
> and there is a trailing whitespace at:
>
> + * If query needs to be truncated, copied will be set to true
>
>
> /home/jim/patches/max_log_query/V3-0001-parameter-max_log_size-to-truncate-logs.patch:141:
> trailing whitespace.
> * If query needs to be truncated, copied will be set to true
> Checking patch doc/src/sgml/config.sgml...
> Hunk #1 succeeded at 7865 (offset -48 lines).
> Checking patch src/backend/tcop/postgres.c...
> Hunk #1 succeeded at 71 (offset 1 line).
> Hunk #2 succeeded at 1031 (offset 1 line).
> Hunk #3 succeeded at 1083 (offset 1 line).
> Hunk #4 succeeded at 1382 (offset 1 line).
> Hunk #5 succeeded at 1393 (offset 1 line).
> Checking patch src/backend/utils/error/elog.c...
> Hunk #4 succeeded at 3781 (offset -3 lines).
> Checking patch src/backend/utils/misc/guc_tables.c...
> Hunk #1 succeeded at 3714 (offset -10 lines).
> Checking patch src/backend/utils/misc/postgresql.conf.sample...
> Hunk #1 succeeded at 615 (offset -2 lines).
> Checking patch src/bin/pg_ctl/t/004_logrotate.pl...
> Checking patch src/include/utils/elog.h...
> Applied patch doc/src/sgml/config.sgml cleanly.
> Applied patch src/backend/tcop/postgres.c cleanly.
> Applied patch src/backend/utils/error/elog.c cleanly.
> Applied patch src/backend/utils/misc/guc_tables.c cleanly.
> Applied patch src/backend/utils/misc/postgresql.conf.sample cleanly.
> Applied patch src/bin/pg_ctl/t/004_logrotate.pl cleanly.
> Applied patch src/include/utils/elog.h cleanly.
> warning: 1 line adds whitespace errors.
>
> --
> Jim
>
>
Thanks for spotting mistakes.
Setting max_log_size to 0 already disables truncation.
I changed guc units to bytes, just to make it more human-friendly. Did
some refactoring and fixed all warnings. Set default value in
postgresql.conf.
Attachments:
[application/octet-stream] V4-0001-parameter-max_log_size-to-truncate-logs.patch (9.0K, ../../CA+E0NR7=V57gVR_-CHs-0W0XL7UrKdyGfT=ZiQcuCdsDcjN94g@mail.gmail.com/3-V4-0001-parameter-max_log_size-to-truncate-logs.patch)
download | inline diff:
From 978693e3375f8ac53a44e9a1496d085cbd99fa9e Mon Sep 17 00:00:00 2001
From: diphantxm <[email protected]>
Date: Fri, 29 Nov 2024 14:02:04 +0300
Subject: [PATCH] parameter max_log_size to truncate logs
There is no need to log the entire query, because it may be large and take lots of space on disk. Parameter max_log_size set the maximum length for logged query. Everything beyond that length is truncated. Value 0 disables the parameter.
---
doc/src/sgml/config.sgml | 15 ++++++
src/backend/tcop/postgres.c | 18 ++++++-
src/backend/utils/error/elog.c | 47 +++++++++++++++++++
src/backend/utils/misc/guc_tables.c | 12 +++++
src/backend/utils/misc/postgresql.conf.sample | 2 +
src/bin/pg_ctl/t/004_logrotate.pl | 15 ++++++
src/include/utils/elog.h | 4 ++
7 files changed, 111 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 76ab72db96..82dc458a4a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7913,6 +7913,21 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
</listitem>
</varlistentry>
+ <varlistentry id="guc-max-log-size" xreflabel="max_log_size">
+ <term><varname>max_log_size</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>max_log_size</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ If greater than zero, each query logged is truncated to this many bytes.
+ Zero disables the setting.
+ Default value is 5MB.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
<sect2 id="runtime-config-logging-csvlog">
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 4b985bd056..033de5862a 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -70,6 +70,7 @@
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "tcop/utility.h"
+#include "utils/elog.h"
#include "utils/guc_hooks.h"
#include "utils/injection_point.h"
#include "utils/lsyscache.h"
@@ -1029,11 +1030,22 @@ exec_simple_query(const char *query_string)
bool was_logged = false;
bool use_implicit_block;
char msec_str[32];
+ char* truncated_query = NULL;
+ const char* query_log;
/*
* Report query to various monitoring facilities.
*/
debug_query_string = query_string;
+ if (need_truncate_query_log(query_string))
+ {
+ truncated_query = truncate_query_log(query_string);
+ query_log = truncated_query;
+ }
+ else
+ {
+ query_log = query_string;
+ }
pgstat_report_activity(STATE_RUNNING, query_string);
@@ -1078,7 +1090,7 @@ exec_simple_query(const char *query_string)
if (check_log_statement(parsetree_list))
{
ereport(LOG,
- (errmsg("statement: %s", query_string),
+ (errmsg("statement: %s", query_log),
errhidestmt(true),
errdetail_execute(parsetree_list)));
was_logged = true;
@@ -1377,7 +1389,7 @@ exec_simple_query(const char *query_string)
case 2:
ereport(LOG,
(errmsg("duration: %s ms statement: %s",
- msec_str, query_string),
+ msec_str, query_log),
errhidestmt(true),
errdetail_execute(parsetree_list)));
break;
@@ -1388,6 +1400,8 @@ exec_simple_query(const char *query_string)
TRACE_POSTGRESQL_QUERY_DONE(query_string);
+ if (truncated_query)
+ pfree(truncated_query);
debug_query_string = NULL;
}
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 289059435a..8a1071677b 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -111,6 +111,7 @@ int Log_destination = LOG_DESTINATION_STDERR;
char *Log_destination_string = NULL;
bool syslog_sequence_numbers = true;
bool syslog_split_messages = true;
+int max_log_size = 0;
/* Processed form of backtrace_functions GUC */
static char *backtrace_function_list;
@@ -1688,11 +1689,18 @@ EmitErrorReport(void)
{
ErrorData *edata = &errordata[errordata_stack_depth];
MemoryContext oldcontext;
+ char* truncated_query = NULL;
recursion_depth++;
CHECK_STACK_DEPTH();
oldcontext = MemoryContextSwitchTo(edata->assoc_context);
+ if (need_truncate_query_log(debug_query_string))
+ {
+ truncated_query = truncate_query_log(debug_query_string);
+ debug_query_string = truncated_query;
+ }
+
/*
* Reset the formatted timestamp fields before emitting any logs. This
* includes all the log destinations and emit_log_hook, as the latter
@@ -1733,6 +1741,9 @@ EmitErrorReport(void)
MemoryContextSwitchTo(oldcontext);
recursion_depth--;
+
+ if (truncated_query)
+ pfree(truncated_query);
}
/*
@@ -3773,3 +3784,39 @@ write_stderr(const char *fmt,...)
#endif
va_end(ap);
}
+
+/*
+ * Apply truncation to build query that will be logged.
+ *
+ * If query needs to be truncated, copied will be set to true
+ * and returned string must be freed
+*/
+char*
+truncate_query_log(const char* query)
+{
+ size_t truncated_query_len;
+ char* truncatd_query;
+ size_t query_len;
+
+ if (!query)
+ return NULL;
+
+ query_len = strlen(query);
+ truncated_query_len = pg_mbcliplen(query, query_len, max_log_size);
+ truncatd_query = (char *) palloc(truncated_query_len+1);
+ memcpy(truncatd_query, query, truncated_query_len);
+ truncatd_query[truncated_query_len] = '\0';
+ return truncatd_query;
+}
+
+/*
+ * Checks if query should be truncated
+ * according to max_log_size
+*/
+bool
+need_truncate_query_log(const char* query)
+{
+ if (!query)
+ return false;
+ return !(max_log_size == 0 || strlen(query) < max_log_size);
+}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 9845abd693..8aa6e3c7a8 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -3724,6 +3724,18 @@ struct config_int ConfigureNamesInt[] =
NULL, NULL, NULL
},
+ {
+ {"max_log_size", PGC_SUSET, LOGGING_WHAT,
+ gettext_noop("Sets max size in bytes of logged statement."),
+ NULL,
+ GUC_UNIT_BYTE
+ },
+ &max_log_size,
+ 5 * (1024 * 1024),
+ 0, INT_MAX,
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 407cd1e08c..848067d87c 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -617,6 +617,8 @@
# bind-parameter values to N bytes;
# -1 means print in full, 0 disables
#log_statement = 'none' # none, ddl, mod, all
+#max_log_size = 5MB # max size of logged statement (in bytes)
+ # 0 disables the feature
#log_replication_commands = off
#log_temp_files = -1 # log temporary files equal or larger
# than the specified size in kilobytes;
diff --git a/src/bin/pg_ctl/t/004_logrotate.pl b/src/bin/pg_ctl/t/004_logrotate.pl
index eacca1a652..1c1f89cb0a 100644
--- a/src/bin/pg_ctl/t/004_logrotate.pl
+++ b/src/bin/pg_ctl/t/004_logrotate.pl
@@ -69,6 +69,7 @@ log_destination = 'stderr, csvlog, jsonlog'
# these ensure stability of test results:
log_rotation_age = 0
lc_messages = 'C'
+max_log_size = 32
));
$node->start();
@@ -135,6 +136,20 @@ check_log_pattern('stderr', $new_current_logfiles, 'syntax error', $node);
check_log_pattern('csvlog', $new_current_logfiles, 'syntax error', $node);
check_log_pattern('jsonlog', $new_current_logfiles, 'syntax error', $node);
+$node->psql('postgres', 'INSERT INTO SOME_NON_EXISTANT_TABLE VALUES (TEST)');
+for (my $attempts = 0; $attempts < $max_attempts; $attempts++)
+{
+ eval {
+ $current_logfiles = slurp_file($node->data_dir . '/current_logfiles');
+ };
+ last unless $@;
+ usleep(100_000);
+}
+die $@ if $@;
+check_log_pattern('stderr', $current_logfiles, 'INSERT INTO SOME_NON_EXISTANT_TA(?!(BLE VALUES \(TEST\)))', $node);
+check_log_pattern('csvlog', $current_logfiles, 'INSERT INTO SOME_NON_EXISTANT_TA(?!(BLE VALUES \(TEST\)))', $node);
+check_log_pattern('jsonlog', $current_logfiles, 'INSERT INTO SOME_NON_EXISTANT_TA(?!(BLE VALUES \(TEST\)))', $node);
+
$node->stop();
done_testing();
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index e54eca5b48..cafa8de3c0 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -502,6 +502,7 @@ extern PGDLLIMPORT int Log_destination;
extern PGDLLIMPORT char *Log_destination_string;
extern PGDLLIMPORT bool syslog_sequence_numbers;
extern PGDLLIMPORT bool syslog_split_messages;
+extern PGDLLIMPORT int max_log_size;
/* Log destination bitmap */
#define LOG_DESTINATION_STDERR 1
@@ -517,6 +518,9 @@ extern void DebugFileOpen(void);
extern char *unpack_sql_state(int sql_state);
extern bool in_error_recursion_trouble(void);
+extern bool need_truncate_query_log(const char* query);
+extern char* truncate_query_log(const char* query);
+
/* Common functions shared across destinations */
extern void reset_formatted_start_time(void);
extern char *get_formatted_start_time(void);
--
2.39.2 (Apple Git-143)
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2024-11-29 11:05 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 02:48 [PATCH 1/3] bootstrap: convert Typ to a List* Justin Pryzby <[email protected]>
2024-11-28 14:22 Re: Truncate logs by max_log_size Jim Jones <[email protected]>
2024-11-28 19:20 ` Re: Truncate logs by max_log_size Kirill Gavrilov <[email protected]>
2024-11-29 00:24 ` Re: Truncate logs by max_log_size Jim Jones <[email protected]>
2024-11-29 11:05 ` Re: Truncate logs by max_log_size Kirill Gavrilov <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox