A client was getting some hard to diagnose out of memory errors. What made this especially confusing was that there was no context reported at all, other than the (enormous) statement that triggered the error.

At first I thought the lack of context indicated a palloc had failed during ereport() (since we apparently just toss the previous error when that happens), but it turns out there's some error reporting in pg_stat_statements that's less than ideal. Attached patch fixes, though I'm not sure if %lld is portable or not.

I'll also argue that this is a bug and should be backpatched, but I'm not hell-bent on that.

At the same time I looked for other messages that don't explicitly reference pg_stat_statements; the only others are in pg_stat_statements_internal() complaining about being called in an inappropriate function context. Presumably at that point there's a reasonable error context stack so I didn't bother with them.

This still seems a bit fragile to me though. Anyone working in here has to notice that most every errmsg mentions pg_stat_statements and decide there's a good reason for that. ISTM it'd be better to push a new ErrorContextCallback onto the stack any time we enter the module. If folks think that's a good idea I'll pursue it as a separate patch.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c 
b/contrib/pg_stat_statements/pg_stat_statements.c
index 59b8a2e..06a0912 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -1893,14 +1893,24 @@ qtext_load_file(Size *buffer_size)
 
        /* Allocate buffer; beware that off_t might be wider than size_t */
        if (stat.st_size <= MaxAllocSize)
-               buf = (char *) malloc(stat.st_size);
-       else
-               buf = NULL;
-       if (buf == NULL)
        {
+               buf = (char *) malloc(stat.st_size);
+
+               if (buf == NULL)
+               {
+                       ereport(LOG,
+                                       (errcode(ERRCODE_OUT_OF_MEMORY),
+                                        errmsg("out of memory attempting to 
pg_stat_statement file"),
+                                errdetail("file \"%s\": size %lld", 
PGSS_TEXT_FILE, stat.st_size)));
+                       CloseTransientFile(fd);
+                       return NULL;
+               }
+       } else {
                ereport(LOG,
+                               /* Is there a better code to use? IE: SQLSTATE 
53000, 53400 or 54000 */
                                (errcode(ERRCODE_OUT_OF_MEMORY),
-                                errmsg("out of memory")));
+                                errmsg("pg_stat_statement file is too large to 
process"),
+                                errdetail("file \"%s\": size %lld", 
PGSS_TEXT_FILE, stat.st_size)));
                CloseTransientFile(fd);
                return NULL;
        }
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to