On 2017-12-21 14:49:28 +0800, Craig Ringer wrote: > +/* > + * Accumulate writes into the buffer in diag_request_buf, > + * for use with functions that expect a printf-like callback. > + */ > +static void > +printwrapper_stringinfo(void *extra, const char * fmt, ...) > +{ > + StringInfo out = extra; > + for (;;) > + { > + va_list args; > + int needed; > + va_start(args, fmt); > + needed = appendStringInfoVA(out, fmt, args); > + va_end(args); > + if (needed == 0) > + break; > + enlargeStringInfo(out, needed); > + } > }
Hm, so I'm not entirely sure it's ok to use something that ERRORs on OOM. There's plenty of scenarios with thousands of memory contexts, making this output fairly large. If we want to make this usable in production, I'm not sure it's ok to introduce additional ERRORs. I wonder if we can change this to emit a static message if collecting the output exhausted memory.
There tons of callers to enlargeStringInfo, so a 'noerror' parameter would be viable.
But I'm not convinced it's worth it personally. If we OOM in response to a ProcSignal request for memory context output, we're having pretty bad luck. The output is 8k in my test. But even if it were a couple of hundred kb, happening to hit OOM just then isn't great luck on modern systems with many gigabytes of RAM.
If that *does* happen, repalloc(...) will call MemoryContextStats(TopMemoryContext) before returning NULL. So we'll get our memory context dump anyway, albeit to stderr.