Re: genbki.pl not quoting keywords in postgres.bki output

[Tom Lane]

Mark Dilger  writes:
> There are not yet any examples in the postgres sources where this
> oversight causes problems, but in genbki.pl, where it makes the
> decision whether to quote a token: ...
> it should also quote anything that is a keyword, such as "open", as
> otherwise you get a syntax error during initdb.

Good point.  Up to now, I'd have written that off as one of the many
undocumented gotchas involved in construction of DATA lines.  However,
I think we've made a conscious effort in the bootstrap conversion work
to eliminate undocumented special cases, so we oughta do something to
make such cases work without extra hacking.

> This patch is not that complicated, but it does create a new coding
> requirement to keep bootparse.y and genbki.pl from getting out of sync.
> It might be simpler to just change genbki.pl to quote everything rather
> than applying this patch.  I don't have an opinion on that.

I don't like adding a lot of unnecessary quoting to the .bki file.

The patch as you have it isn't that awful from a maintenance perspective;
I don't think we've added new keywords to bootparse.y very often, so we
could surely cope with keeping genbki.pl in sync.

However, really this ought to be solved in bootparse.y itself, I think:
it should be possible to have it treat keywords like identifiers in INSERT
commands, just as (most) keywords aren't reserved in the main grammar.
Let me go poke at that idea.  If it doesn't work nicely I'll use your
patch.

regards, tom lane

Re: Python 3.7 support

[Tom Lane]

Peter Eisentraut  writes:
> I have committed this now, since the release of Python 3.7 is soon.
> I'll let the build farm have a pass at it, then backport it for the
> upcoming minor releases.

If you're intending to push this into the back branches before Monday's
releases, please do it *today*, the sooner the better.  We are rapidly
closing in on the point where we'd not have a full set of buildfarm runs
done before the wrap.  That is not a good situation for a patch with
portability implications.

regards, tom lane

Compiler warnings with --enable-dtrace

[Thomas Munro]

Hi hackers,

--enable-dtrace produces compiler warnings about const correctness,
except on macOS.  That's because Apple's dtrace produces function
declarations in probes.h that take strings as const char * whereas
AFAIK on all other operating systems they take char * (you can see
that possibly recent difference in Apple's version of dt_header_decl()
in dt_program.c).  People have complained before[1].

Maybe we should do what the Perl people do[2] and post-process the
generated header file to add const qualifiers?  Please see attached.

I have just added --enable-dtrace to my build farm animal elver so
these warnings should appear at the next build.  I wonder if the
owners of damselfly, castoroides, protosciurus (CCed) would consider
adding it for them too so that we could get some coverage of this
build option on Illumos and Solaris.

[1] 
https://www.postgresql.org/message-id/flat/38D06FCCB225BA1C6699D4E7%40amenophis
[2] 
https://github.com/Perl/perl5/blob/a385812b685b3164e706880a72ee60c9cc9573e4/Makefile.SH#L870

-- 
Thomas Munro
http://www.enterprisedb.com


0001-Fix-const-warnings-when-building-with-enable-dtrace.patch
Description: Binary data

Re: Explain buffers wrong counter with parallel plans

[Amit Kapila]

 On Fri, May 4, 2018 at 10:35 PM, Robert Haas  wrote:
> On Wed, May 2, 2018 at 11:37 AM, Adrien Nayrat
>  wrote:
>> In 9.6 gather node reports sum of buffers for main process + workers. In 10,
>> gather node only reports buffers from the main process.
>
> Oh, really?  Well, that sounds like a bug.  Amit seems to think it's
> expected behavior, but I don't know why it should be.
>

The reason why I think the current behavior is okay because it is
coincidental that they were displayed correctly.  We have not made any
effort to percolate it to upper nodes.  For ex., before that commit
also, it was not being displayed for Gather Merge or Gather with some
kind of node like 'Limit' where we have to stop before reaching the
end of the result.

>  The commit
> message makes it sound like it's just refactoring, but in fact it
> seems to have made a significant behavior change that doesn't look
> very desirable.
>

I think it is below part of that commit which has made this
difference, basically shutting down the workers.

if (readerdone)
{
Assert(!tup);
..
if (gatherstate->nreaders == 0)
-   {
-   ExecShutdownGatherWorkers(gatherstate);
return NULL;
-   }


The reason why we were getting different results due to above code is
that because while shutting down workers, we gather the buffer usage
of all workers in 'pgBufferUsage' via InstrAccumParallelQuery and then
upper-level node say Gather in this case would get chance to use that
stats via ExecProcNodeInstr->InstrStopNode.  However, this won't be
true in other cases where we need to exit before reaching the end of
results like in 'Limit' node case as in such cases after shutting down
the workers via ExecShutdownNode we won't do InstrStopNode for
upper-level nodes.  I think if we want that all the stats being
collected by workers should percolate to Gather and nodes above it,
then we need to somehow ensure that we always shutdown
Gather/GatherMerge before we do InstrStopNode for those nodes.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Clock with Adaptive Replacement

[Yura Sokolov]

05.05.2018 09:16, Andrey Borodin пишет:
> Hi!
> 
>> 4 мая 2018 г., в 16:05, Юрий Соколов 
>> написал(а):
>> 
>> I didn't suggest log scale of usages, but rather
>> "replacement-period based" increment: usage count could be
>> incremented at most once in NBlocks/32 replaced items. Once it is
>> incremented, its "replacement time" is remembered, and next
>> NBlocks/32 replacements usage count of this buffer doesn't
>> increment. This way, increment is synchronized with replacement
>> activity.
> 
> But you loose difference between "touched once" and "actively used".
> Log scale of usage solves this: usage count grows logarithmically,
> but drains linearly.
No, I didn't loose difference. But instead of absolute value (log scale
or linear) I count how often in time block is used:
- if buffer were touched 1000 times just after placing into
shared_buffers should it live 500 times longer than neighbor that were
touched only 2 times? or 10 times longer? or 5 times longer?
- but what if that "1000 times" buffer were not touched in next period
of time, but neighbor were touched again 2 times?
All effective algorithms answers: "1000 times" buffer should be evicted
first, but its neighbor is a really hot buffer that should be saved for
longer period.

Log scale doesn't solve this. But increment "once in period" solves.
Especially if block is placed first with zero count (instead of 1 as
currently).

>> Digging further, I suggest as improvement of GClock algorithm: -
>> placing new buffer with usage count = 0 (and next NBlock/32
>> replacements its usage count doesn't increased)
>> - increment not by 1, but by 8 (it simulates "hot queue" of
>> popular algorithms) with limit 32.
>> - scan at most 25 buffers for eviction. If no buffer with zero
>> usage count found, the least used buffer (among scanned 25) is evicted.
>> (new buffers are not evicted during their first NBlock/32
>> replacements).
>> 
> 
> I do not understand where these numbers come from...

I found this number by testing with several artificial traces found in web.
I don't claim this number are best. Even on that traces best values may
vary on cache size: for small cache size increment and limit tends to be
higher, for huge cache - smaller. But this were most balanced.

And I don't claim those traces are representative for PostgreSQL, that
is why I'm pushing this discussion to collect more real-world PostgreSQL
traces and make them public.

And I believe my algorithm is not the best. Clock-Pro and ARC shows
better results on that traces. Tiny-LFU - cache admission algorithm -
may be even more efficient (in term of evictions). But results should be
rechecked with PostgreSQL traces.

My algorithm will be just least invasive for current code, imho.

With regards,
Sokolov Yura



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Clock with Adaptive Replacement

[Andrey Borodin]

Hi!

> 4 мая 2018 г., в 16:05, Юрий Соколов  написал(а):
> 
> I didn't suggest log scale of usages, but rather "replacement-period based" 
> increment: usage count could be incremented at most once in NBlocks/32 
> replaced items. Once it is incremented, its "replacement time" is remembered, 
> and next NBlocks/32 replacements usage count of this buffer doesn't increment.
> This way, increment is synchronized with replacement activity.
But you loose difference between "touched once" and "actively used". Log scale 
of usage solves this: usage count grows logarithmically, but drains linearly.

> 
> Digging further, I suggest as improvement of GClock algorithm:
> - placing new buffer with usage count = 0 (and next NBlock/32 replacements 
> its usage count doesn't increased)
> - increment not by 1, but by 8 (it simulates "hot queue" of popular 
> algorithms) with limit 32.
> - scan at most 25 buffers for eviction. If no buffer with zero usage count 
> found, the least used buffer (among scanned 25) is evicted.
> (new buffers are not evicted during their first NBlock/32 replacements).
> 

I do not understand where these numbers come from...

Best regards, Andrey Borodin.