[HACKERS] Scaling shared buffer eviction

2014-05-14 Thread Amit Kapila
As mentioned previously about my interest in improving shared
buffer eviction especially by reducing contention around
BufFreelistLock, I would like to share my progress about the
same.

The test used for this work is mainly the case when all the
data doesn't fit in shared buffers, but does fit in memory.
It is mainly based on previous comparison done by Robert
for similar workload:
http://rhaas.blogspot.in/2012/03/performance-and-scalability-on-ibm.html

To start with, I have taken LWLOCK_STATS report to confirm
the contention around BufFreelistLock and the data for HEAD
is as follows:

M/c details
IBM POWER-7 16 cores, 64 hardware threads
RAM - 64GB
Test
scale factor = 3000
shared_buffers = 8GB
number_of_threads = 64
duration = 5mins
./pgbench -c 64 -j 64 -T 300 -S  postgres

LWLOCK_STATS data for BufFreeListLock
PID 11762 lwlock main 0: shacq 0 exacq 253988 blk 29023

Here the high *blk* count for scale factor 3000 clearly shows
that to find a usable buffer when data doesn't fit in shared buffers
it has to wait.

To solve this issue, I have implemented a patch which makes
sure that there are always enough buffers on freelist such that
the need for backend to run clock-sweep is minimal, the
implementation idea is more or less same as discussed
previously in below thread, so I will explain it at end of mail.
http://www.postgresql.org/message-id/006e01ce926c$c7768680$56639380$@kap...@huawei.com

LWLOCK_STATS data after Patch (test used is same as
used for HEAD):

BufFreeListLock
PID 7257 lwlock main 0: shacq 0 exacq 165 blk 18 spindelay 0

Here the low *exacq* and *blk* count shows that the need to
run clock sweep for backend has reduced significantly.

Performance Data
---
shared_buffers= 8GB
number of threads - 64
sc - scale factor

  sc  tps
Head300045569
Patch   300046457
Head100093037
Patch   100092711

Above data shows that there is no significant change in
performance or scalability even after the contention is
reduced significantly around BufFreelistLock.

I have analyzed the patch both with perf record and
LWLOCK_STATS, both indicates that there is a high
contention around BufMappingLocks.

Data With perf record -a -g
-

+  10.14%  swapper  [kernel.kallsyms]  [k]
.pseries_dedicated_idle_sleep
+   7.77% postgres  [kernel.kallsyms]  [k] ._raw_spin_lock
+   6.88% postgres  [kernel.kallsyms]  [k]
.function_trace_call
+   4.15%  pgbench  [kernel.kallsyms]  [k] .try_to_wake_up
+   3.20%  swapper  [kernel.kallsyms]  [k]
.function_trace_call
+   2.99%  pgbench  [kernel.kallsyms]  [k]
.function_trace_call
+   2.41% postgres  postgres   [.] AllocSetAlloc
+   2.38% postgres  [kernel.kallsyms]  [k] .try_to_wake_up
+   2.27%  pgbench  [kernel.kallsyms]  [k] ._raw_spin_lock
+   1.49% postgres  [kernel.kallsyms]  [k]
._raw_spin_lock_irq
+   1.36% postgres  postgres   [.]
AllocSetFreeIndex
+   1.09%  swapper  [kernel.kallsyms]  [k] ._raw_spin_lock
+   0.91% postgres  postgres   [.] GetSnapshotData
+   0.90% postgres  postgres   [.]
MemoryContextAllocZeroAligned


Expanded graph
--

-  10.14%  swapper  [kernel.kallsyms]  [k]
.pseries_dedicated_idle_sleep
   - .pseries_dedicated_idle_sleep
  - 10.13% .pseries_dedicated_idle_sleep
 - 10.13% .cpu_idle
- 10.00% .start_secondary
 .start_secondary_prolog
-   7.77% postgres  [kernel.kallsyms]  [k] ._raw_spin_lock
   - ._raw_spin_lock
  - 6.63% ._raw_spin_lock
 - 5.95% .double_rq_lock
- .load_balance
   - 5.95% .__schedule
  - .schedule
 - 3.27% .SyS_semtimedop
  .SyS_ipc
  syscall_exit
  semop
  PGSemaphoreLock
  LWLockAcquireCommon
- LWLockAcquire
   - 3.27% BufferAlloc
ReadBuffer_common
 - ReadBufferExtended
 - 3.27% ReadBuffer
- 2.73% ReleaseAndReadBuffer
   - 1.70% _bt_relandgetbuf
_bt_search
_bt_first
btgettuple


It shows BufferAlloc->LWLOCK as top contributor and we use
BufMappingLocks in BufferAlloc, I have checked other expanded
calls as well, StrategyGetBuffer is not present in top contributors.

Data with LWLOCK_STATS
--
BufMappingLocks

PID 7245 lwlock main 38: shacq 41117 exacq 34561 blk 36274 spindel

Re: [HACKERS] Freezing without write I/O

2014-05-14 Thread David Fetter
On Wed, May 14, 2014 at 05:46:49PM -0700, Jeff Janes wrote:
> On Monday, January 27, 2014, Robert Haas  wrote:
> 
> > On Mon, Jan 27, 2014 at 4:16 PM, Simon Riggs 
> > >
> > wrote:
> > > On 26 January 2014 12:58, Andres Freund 
> > > >
> > wrote:
> > >> On 2014-01-25 20:26:16 -0800, Peter Geoghegan wrote:
> > >>> Shouldn't this patch be in the January commitfest?
> > >>
> > >> I think we previously concluded that there wasn't much chance to get
> > >> this into 9.4 and there's significant work to be done on the patch
> > >> before new reviews are required, so not submitting it imo makes sense.
> > >
> > > I think we should make this a priority feature for 9.5
> >
> > +1.  I can't think of many things we might do that would be more important.
> >
> Can anyone guess how likely this approach is to make it into 9.5?
> I've been pondering some incremental improvements over what we have
> now, but if this revolutionary approach has a high chance of landing
> then any work on incremental improvements would be pointless.

How much work do you anticipate for the description and PoC of
whatever it is you've been pondering?

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Freezing without write I/O

2014-05-14 Thread Jeff Janes
On Monday, January 27, 2014, Robert Haas  wrote:

> On Mon, Jan 27, 2014 at 4:16 PM, Simon Riggs 
> >
> wrote:
> > On 26 January 2014 12:58, Andres Freund 
> > >
> wrote:
> >> On 2014-01-25 20:26:16 -0800, Peter Geoghegan wrote:
> >>> Shouldn't this patch be in the January commitfest?
> >>
> >> I think we previously concluded that there wasn't much chance to get
> >> this into 9.4 and there's significant work to be done on the patch
> >> before new reviews are required, so not submitting it imo makes sense.
> >
> > I think we should make this a priority feature for 9.5
>
> +1.  I can't think of many things we might do that would be more important.
>
>
Can anyone guess how likely this approach is to make it into 9.5?  I've
been pondering some incremental improvements over what we have now, but if
this revolutionary approach has a high chance of landing then any work on
incremental improvements would be pointless.

Thanks,

Jeff


Re: [HACKERS] [PATCH] Writing changes of decoding plugin in the memory context where data is kept?

2014-05-14 Thread Michael Paquier
On Thu, May 15, 2014 at 9:18 AM, Andres Freund  wrote:
> On 2014-05-15 09:11:15 +0900, Michael Paquier wrote:
>> When working on a decoder plugin, I have been pointed that it is
>> incorrect to write changes in an output plugin while not being in the
>> memory context where changes are written.
>
> That was a misunderstanding. It's perfectly alright to do so. The 'bug'
> you have is that you sometimes wrote empty messages. Which
> pg_recvlogical doesn't accept. There's little reason to allow doing so,
> but for consistency we could probably ok to allow it.
Yeah, thanks. Pushed a fix. Also I imagine that this is not really a
problem, but is there an impact on the plugin if
OutputPluginPrepareWrite is called and OutputPluginWrite is not for a
single change? By looking at the code I am guessing that we use
WalSndPrepareWrite for a plugin to prepare a message, and then
WalSndWriteData to finish the work, so we end up with an empty message
that is not sent to the backend. It doesn't matter much for
test_decoding as it always writes a change, but shouldn't we
discourage users to do so in the documentation? Something like a
big-fat warning mentioning that using OutputPluginPrepareWrite called
without OutputPluginWrite may result in stall messages stuck in the
queue?
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] buildfarm / handling (undefined) locales

2014-05-14 Thread Tom Lane
Tomas Vondra  writes:
> On 13.5.2014 23:07, Tom Lane wrote:
>> Attached is a draft patch that behaves like this:
>> 
>> $ initdb --locale=foo
>> The files belonging to this database system will be owned by user "postgres".
>> This user must also own the server process.
>> 
>> initdb: invalid locale name "foo"

> I see the committed patch actually behaves like this:

>   initdb: invalid locale name "cs_CZ.WIN-1250"
>   The files belonging to this database system will be owned by user
>   "pgbuild".
>   This user must also own the server process.

> (at least that's what magpie logged at [1]). I find that confusing,

Interesting. I guess stdout isn't getting fflush'd soon enough when
it's going to a file.  We should probably mark it as line-buffered
so the behavior is consistent with the interactive case.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] buildfarm / handling (undefined) locales

2014-05-14 Thread Tomas Vondra
On 13.5.2014 23:07, Tom Lane wrote:
> Andrew Dunstan  writes:
>> On 05/13/2014 04:14 PM, Tom Lane wrote:
>>> But independently of whether it's a fatal error or not: when there's
>>> no relevant command-line argument then we print the
>>>
>>> invalid locale name ""
>>>
>>> message which is surely pretty unhelpful.  It'd be better if we could
>>> finger the incorrect environment setting.  Unfortunately, we don't know
>>> for sure which environment variable(s) setlocale was looking at --- I
>>> believe it's somewhat platform specific.  We could probably print
>>> something like this instead:
>>>
>>> environment locale settings are invalid
>>>
>>> Thoughts?
> 
>> I'd also be tempted to add the settings for LC_ALL and LANG and note 
>> that they are possible sources of the problem, or maybe only do that if 
>> they match the locale being rejected.
> 
> Well, all we know is that we asked setlocale for the default locale from
> the environment and it failed.  We don't really know which variable(s)
> it looked at.  I think printing specific variables could easily be as
> misleading as it is helpful; and given the lack of prior complaints,
> I'm doubtful that it's worth going to the effort of trying to print a
> platform-specific collection of variables.
> 
> Attached is a draft patch that behaves like this:
> 
> $ initdb --locale=foo
> The files belonging to this database system will be owned by user "postgres".
> This user must also own the server process.
> 
> initdb: invalid locale name "foo"

I see the committed patch actually behaves like this:

  initdb: invalid locale name "cs_CZ.WIN-1250"
  The files belonging to this database system will be owned by user
  "pgbuild".
  This user must also own the server process.

(at least that's what magpie logged at [1]). I find that confusing,
because initdb logs the actual problem and then two more lines so it
seems like nothing happened.

I think we should keep the message explaining the problem last, i.e. not
log the two additional messages (which are quite pointless), switch the
order of messages or add another one.

[1]
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=magpie&dt=2014-05-14%2019%3A20%3A25

Tomas


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [PATCH] Writing changes of decoding plugin in the memory context where data is kept?

2014-05-14 Thread Andres Freund
On 2014-05-15 09:11:15 +0900, Michael Paquier wrote:
> When working on a decoder plugin, I have been pointed that it is
> incorrect to write changes in an output plugin while not being in the
> memory context where changes are written.

That was a misunderstanding. It's perfectly alright to do so. The 'bug'
you have is that you sometimes wrote empty messages. Which
pg_recvlogical doesn't accept. There's little reason to allow doing so,
but for consistency we could probably ok to allow it.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Cache invalidation bug in RelationGetIndexAttrBitmap()

2014-05-14 Thread Tom Lane
Tomas Vondra  writes:
> On 14.5.2014 22:29, Andres Freund wrote:
>> I'm afraid it's more in the year range from what i've seen. I.e. not
>> practical.

> Yeah, that wouldn't be very practical. I'll try to run it though and if
> it'd run more than a few days, I'll switch it to CLOBBER_CACHE_ALWAYS.

I think it might be interesting to try running CLOBBER_CACHE_RECURSIVELY
on some subset of the regression tests.  But I'm not sure which subset,
nor whether the buildfarm scripts could readily be coaxed to do that.
Let's see how far you get with a full run before you get bored ...

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] [PATCH] Writing changes of decoding plugin in the memory context where data is kept?

2014-05-14 Thread Michael Paquier
Hi all,

When working on a decoder plugin, I have been pointed that it is
incorrect to write changes in an output plugin while not being in the
memory context where changes are written.
In pg_decode_change@test_decoding.c, we do the following:
old = MemoryContextSwitchTo(data->context);
OutputPluginPrepareWrite(ctx, true);
[...]
MemoryContextSwitchTo(old);
MemoryContextReset(data->context);
OutputPluginWrite(ctx, true);

Wouldn't it be better to call OutputPluginWrite before switching back
to the old context? The attached patch for test_decoding does so.
Comments welcome.
Regards,
-- 
Michael
From 9bd2e09433573fbfd85105fca2c6862dba383eab Mon Sep 17 00:00:00 2001
From: Michael Paquier 
Date: Thu, 15 May 2014 09:01:28 +0900
Subject: [PATCH] Write changes of test_decoding while being in the data
 context

OutputPluginWrite was being called after switching to the old memory
context. This seems incorrect as the changes are written in a specific
memory context.
---
 contrib/test_decoding/test_decoding.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/contrib/test_decoding/test_decoding.c 
b/contrib/test_decoding/test_decoding.c
index 5ce052b..903b15b 100644
--- a/contrib/test_decoding/test_decoding.c
+++ b/contrib/test_decoding/test_decoding.c
@@ -400,8 +400,10 @@ pg_decode_change(LogicalDecodingContext *ctx, 
ReorderBufferTXN *txn,
Assert(false);
}
 
+   /* Write changes while in the context of the data */
+   OutputPluginWrite(ctx, true);
+
+   /* Switch back to old context */
MemoryContextSwitchTo(old);
MemoryContextReset(data->context);
-
-   OutputPluginWrite(ctx, true);
 }
-- 
1.9.3


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Sending out a request for more buildfarm animals?

2014-05-14 Thread Tomas Vondra
On 13.5.2014 20:42, Tomas Vondra wrote:
> On 10.5.2014 20:21, Tomas Vondra wrote:
>> On 9.5.2014 00:47, Tomas Vondra wrote:
>>
>> And I've requested 6 more animals - two for each compiler. One set for
>> tests with basic CLOBBER, one set for recursive CLOBBER.
> 
> Can someone please approve the animals I've requested a few days ago?
> I'm already running the clobber tests with '--nosend --nostatus' and
> it's already reporting some errors. Would be nice to get it to the
> buildfarm.

So the new animals:

CLOBBER_CACHE_ALWAYS (+ others)
 markhor  gcc
 tick clang/llvm
 leechicc

CLOBBER_CACHE_RECURSIVELY (+ others)
 addaxgcc
 mite clang/llvm
 barnacle icc

The builds trigger every hour, but it may take hours / days to get first
results.

Tomas


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Cache invalidation bug in RelationGetIndexAttrBitmap()

2014-05-14 Thread Tomas Vondra
On 14.5.2014 22:29, Andres Freund wrote:
> Hi,
> 
> On 2014-05-14 21:04:41 +0200, Tomas Vondra wrote:
>> On 14.5.2014 17:52, Andres Freund wrote:
>>> On 2014-05-14 15:17:39 +0200, Andres Freund wrote:
 On 2014-05-14 15:08:08 +0200, Tomas Vondra wrote:
> Apparently there's something wrong with 'test-decoding-check':

 Man. I shouldn't have asked... My code. There's some output in there
 that's probably triggered by the extraordinarily long runtimes, but
 there's definitely something else wrong.
 My gut feeling says it's in RelationGetIndexList().
>>>
>>> Nearly right. It's in RelationGetIndexAttrBitmap(). Fix attached.
>>>
>>> Tomas, thanks for that. I've never (and probably will never) run
>>> CLOBBER_CACHE_RECURSIVELY during development. Having a machine do that
>>> regularly is really helpful. How long does a single testrun take? It
>>> takes hundreds of seconds here to do a single UPDATE?
>>
>> Don't know yet, as it fails at the beginning.
> 
> test decoding is at the beginning? That's somewhat odd?

Judging from the timestamps of log files, it seems to be running after
pg_upgrade checks. Or maybe I'm reading that wrong.

>> But I suppose it will be
>> tens or possibly hundreds of hours. For example these are the logs from
>> regular build (no clobber etc.)
> 
>> May 14 19:00 SCM-checkout.log
>> May 14 19:00 githead.log
>> May 14 19:00 configure.log
>> May 14 19:00 config.log
>> May 14 19:05 make.log
>> May 14 19:05 check.log
>> May 14 19:06 make-contrib.log
>> May 14 19:06 make-install.log
>> May 14 19:06 install-contrib.log
>> May 14 19:07 check-pg_upgrade.log
>> May 14 19:08 test-decoding-check.log
>>
>> while these are the logs from recursive clobber:
>>
>> May 14 00:19 SCM-checkout.log
>> May 14 00:20 configure.log
>> May 14 00:20 config.log
>> May 14 00:26 make.log
>> May 14 03:12 check.log
>> May 14 03:13 make-contrib.log
>> May 14 03:13 make-install.log
>> May 14 03:13 install-contrib.log
>> May 14 08:25 check-pg_upgrade.log
>> May 14 09:07 test-decoding-check.log
>> May 14 09:07 web-txn.data
>>
>>
>> So with the regular build, it took <1 minute to do 'make check' and ~1
>> minute to test pg_upgrade, with recursive clobber it takes ~3 hours and
>> ~5 hours. That's a factor of ~300, although it's a very rough
>> estimate.
> 
> I seriously doubt that's recursive clobber. That should take *way* much
> longer. And indeed you have:
> 
>> -DCLOBBER_CACHE_ALWAYS -DCLOBBER_FREED_MEMORY -DMEMORY_CONTEXT_CHECKING
>> -DRANDOMIZE_ALLOCATED_MEMORY -DCLOBBER_CACHE_RECURSIVELY
>>
>> it does not happen with
>>
>> CPPFLAGS => '-DCLOBBER_CACHE_ALWAYS -DCLOBBER_FREED_MEMORY
>> -DMEMORY_CONTEXT_CHECKING -DRANDOMIZE_ALLOCATED_MEMORY',
> 
> #if defined(CLOBBER_CACHE_ALWAYS)
>   {
>   static bool in_recursion = false;
> 
>   if (!in_recursion)
>   {
>   in_recursion = true;
>   InvalidateSystemCaches();
>   in_recursion = false;
>   }
>   }
> #elif defined(CLOBBER_CACHE_RECURSIVELY)
>   InvalidateSystemCaches();
> #endif
> 
> i.e. you can't specifiy -DCLOBBER_CACHE_ALWAYS and
> -DCLOBBER_CACHE_RECURSIVELY together. The former will take precedence.

Oh, I see :-/

>> Without clobber the whole run (for a "C" locale) takes ~10 minutes, so
>> my estimate is ~50 hours for the recursive one. But I wouldn't be
>> surprised by 100 hours.
> 
> I'm afraid it's more in the year range from what i've seen. I.e. not
> practical.

Yeah, that wouldn't be very practical. I'll try to run it though and if
it'd run more than a few days, I'll switch it to CLOBBER_CACHE_ALWAYS.

Tomas


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Cache invalidation bug in RelationGetIndexAttrBitmap()

2014-05-14 Thread Andres Freund
Hi,

On 2014-05-14 21:04:41 +0200, Tomas Vondra wrote:
> On 14.5.2014 17:52, Andres Freund wrote:
> > On 2014-05-14 15:17:39 +0200, Andres Freund wrote:
> >> On 2014-05-14 15:08:08 +0200, Tomas Vondra wrote:
> >>> Apparently there's something wrong with 'test-decoding-check':
> >>
> >> Man. I shouldn't have asked... My code. There's some output in there
> >> that's probably triggered by the extraordinarily long runtimes, but
> >> there's definitely something else wrong.
> >> My gut feeling says it's in RelationGetIndexList().
> > 
> > Nearly right. It's in RelationGetIndexAttrBitmap(). Fix attached.
> > 
> > Tomas, thanks for that. I've never (and probably will never) run
> > CLOBBER_CACHE_RECURSIVELY during development. Having a machine do that
> > regularly is really helpful. How long does a single testrun take? It
> > takes hundreds of seconds here to do a single UPDATE?
> 
> Don't know yet, as it fails at the beginning.

test decoding is at the beginning? That's somewhat odd?

> But I suppose it will be
> tens or possibly hundreds of hours. For example these are the logs from
> regular build (no clobber etc.)

> May 14 19:00 SCM-checkout.log
> May 14 19:00 githead.log
> May 14 19:00 configure.log
> May 14 19:00 config.log
> May 14 19:05 make.log
> May 14 19:05 check.log
> May 14 19:06 make-contrib.log
> May 14 19:06 make-install.log
> May 14 19:06 install-contrib.log
> May 14 19:07 check-pg_upgrade.log
> May 14 19:08 test-decoding-check.log
> 
> while these are the logs from recursive clobber:
> 
> May 14 00:19 SCM-checkout.log
> May 14 00:20 configure.log
> May 14 00:20 config.log
> May 14 00:26 make.log
> May 14 03:12 check.log
> May 14 03:13 make-contrib.log
> May 14 03:13 make-install.log
> May 14 03:13 install-contrib.log
> May 14 08:25 check-pg_upgrade.log
> May 14 09:07 test-decoding-check.log
> May 14 09:07 web-txn.data
> 
> 
> So with the regular build, it took <1 minute to do 'make check' and ~1
> minute to test pg_upgrade, with recursive clobber it takes ~3 hours and
> ~5 hours. That's a factor of ~300, although it's a very rough
> estimate.

I seriously doubt that's recursive clobber. That should take *way* much
longer. And indeed you have:

> -DCLOBBER_CACHE_ALWAYS -DCLOBBER_FREED_MEMORY -DMEMORY_CONTEXT_CHECKING
> -DRANDOMIZE_ALLOCATED_MEMORY -DCLOBBER_CACHE_RECURSIVELY
> 
> it does not happen with
> 
> CPPFLAGS => '-DCLOBBER_CACHE_ALWAYS -DCLOBBER_FREED_MEMORY
> -DMEMORY_CONTEXT_CHECKING -DRANDOMIZE_ALLOCATED_MEMORY',

#if defined(CLOBBER_CACHE_ALWAYS)
{
static bool in_recursion = false;

if (!in_recursion)
{
in_recursion = true;
InvalidateSystemCaches();
in_recursion = false;
}
}
#elif defined(CLOBBER_CACHE_RECURSIVELY)
InvalidateSystemCaches();
#endif

i.e. you can't specifiy -DCLOBBER_CACHE_ALWAYS and
-DCLOBBER_CACHE_RECURSIVELY together. The former will take precedence.

> Without clobber the whole run (for a "C" locale) takes ~10 minutes, so
> my estimate is ~50 hours for the recursive one. But I wouldn't be
> surprised by 100 hours.

I'm afraid it's more in the year range from what i've seen. I.e. not
practical.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Cache invalidation bug in RelationGetIndexAttrBitmap()

2014-05-14 Thread Tomas Vondra
On 14.5.2014 17:52, Andres Freund wrote:
> On 2014-05-14 15:17:39 +0200, Andres Freund wrote:
>> On 2014-05-14 15:08:08 +0200, Tomas Vondra wrote:
>>> Apparently there's something wrong with 'test-decoding-check':
>>
>> Man. I shouldn't have asked... My code. There's some output in there
>> that's probably triggered by the extraordinarily long runtimes, but
>> there's definitely something else wrong.
>> My gut feeling says it's in RelationGetIndexList().
> 
> Nearly right. It's in RelationGetIndexAttrBitmap(). Fix attached.
> 
> Tomas, thanks for that. I've never (and probably will never) run
> CLOBBER_CACHE_RECURSIVELY during development. Having a machine do that
> regularly is really helpful. How long does a single testrun take? It
> takes hundreds of seconds here to do a single UPDATE?

Don't know yet, as it fails at the beginning. But I suppose it will be
tens or possibly hundreds of hours. For example these are the logs from
regular build (no clobber etc.)

May 14 19:00 SCM-checkout.log
May 14 19:00 githead.log
May 14 19:00 configure.log
May 14 19:00 config.log
May 14 19:05 make.log
May 14 19:05 check.log
May 14 19:06 make-contrib.log
May 14 19:06 make-install.log
May 14 19:06 install-contrib.log
May 14 19:07 check-pg_upgrade.log
May 14 19:08 test-decoding-check.log

while these are the logs from recursive clobber:

May 14 00:19 SCM-checkout.log
May 14 00:20 configure.log
May 14 00:20 config.log
May 14 00:26 make.log
May 14 03:12 check.log
May 14 03:13 make-contrib.log
May 14 03:13 make-install.log
May 14 03:13 install-contrib.log
May 14 08:25 check-pg_upgrade.log
May 14 09:07 test-decoding-check.log
May 14 09:07 web-txn.data


So with the regular build, it took <1 minute to do 'make check' and ~1
minute to test pg_upgrade, with recursive clobber it takes ~3 hours and
~5 hours. That's a factor of ~300, although it's a very rough estimate.

Without clobber the whole run (for a "C" locale) takes ~10 minutes, so
my estimate is ~50 hours for the recursive one. But I wouldn't be
surprised by 100 hours.

> 
> There were some more differences but those are all harmless and caused
> by the extraordinarily long runtime (autovacuums). I think we need to
> add a feature to test_decoding to suppress displaying transactions
> without changes. Ick.
> 

I expect to hit more timing-related issues with the recursive clobber
tests - not necessarily in the code/tests itself, but I guess the
buildfarm tooling doesn't really expect runs that long.

regards
Tomas


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] 9.4 release notes

2014-05-14 Thread Bruce Momjian
On Wed, May 14, 2014 at 05:40:04PM +0100, Dean Rasheed wrote:
> > Agreed.  Adjusted attached patch applied.  I was never happy with the
> > previous awkward auto-update wording.
> >
> 
> Thanks. There's a typo in the new text though --- "cals" should be "calls".

Thanks, fixed.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Cache invalidation bug in RelationGetIndexAttrBitmap()

2014-05-14 Thread Tom Lane
Andres Freund  writes:
> On 2014-05-14 12:15:27 -0400, Tom Lane wrote:
>> And why does the header
>> comment for RelationGetIndexList make no mention of this new side-effect?
>> Somebody did a seriously poor job of adding this functionality to
>> relcache.

> It's not like it's not documented: There's a comment about it in struct
> RelationData. I must have missed that rd_oidindex has a comment abou
> it's lifetime over RelationGetIndexList().

If rd_replidindex is being managed like rd_oidindex, then it should be
managed just like rd_oidindex, including getting reset in all the places
rd_oidindex is.  This might be just a matter of cleanliness but I think
it's important for readability and debuggability.

I notice also that rd_keyattr and rd_idattr have been implemented with
bad copies of the logic for rd_indexattr.  This is at least leading
to a permanent memory leak in CacheMemoryContext during every relcache
flush, and maybe worse things.  The bugs for rd_keyattr appear to predate
your patch though.

Working on a patch for this now.  One thing I'm wondering about is
RelationSetIndexList.  It's probably okay for it not to touch rd_keyattr
and rd_idattr, but I'm not too clear on what the use cases for those
attnum sets are.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Various cosmetic fixes

2014-05-14 Thread Euler Taveira
Hi,

While updating pt-br translation I noticed that some sentences could be
improved. I also fix some style glitches. A set of patches are attached.


-- 
   Euler Taveira   Timbira - http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
>From 87a499cd8fe6a1d0c491b3263c3daf66856cf6f1 Mon Sep 17 00:00:00 2001
From: Euler Taveira 
Date: Wed, 14 May 2014 14:17:00 -0300
Subject: [PATCH 01/11] Fix style.

Use the same messages from pg_receivexlog because they were copied from
it.
---
 src/bin/pg_basebackup/pg_recvlogical.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 194d10f..52dc839 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -965,14 +965,14 @@ main(int argc, char **argv)
 		}
 		else if (noloop)
 		{
-			fprintf(stderr, _("%s: disconnected.\n"), progname);
+			fprintf(stderr, _("%s: disconnected\n"), progname);
 			exit(1);
 		}
 		else
 		{
 			fprintf(stderr,
 			/* translator: check source for value for %d */
-	_("%s: disconnected. Waiting %d seconds to try again.\n"),
+	_("%s: disconnected; waiting %d seconds to try again\n"),
 	progname, RECONNECT_SLEEP_TIME);
 			pg_usleep(RECONNECT_SLEEP_TIME * 100);
 		}
-- 
2.0.0.rc0

>From 864aabd419ce3f4ae5afcf751ebeb22eb9f0e257 Mon Sep 17 00:00:00 2001
From: Euler Taveira 
Date: Wed, 14 May 2014 14:18:53 -0300
Subject: [PATCH 02/11] Don't hardwire default values.

Avoid hardwiring default values in help. Also, provide the unit
(seconds) in the parameter value (following style elsewhere).
---
 src/bin/pg_basebackup/pg_recvlogical.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 52dc839..e7d2e97 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -80,14 +80,14 @@ usage(void)
 	printf(_("  -w, --no-password  never prompt for password\n"));
 	printf(_("  -W, --password force password prompt (should happen automatically)\n"));
 	printf(_("\nReplication options:\n"));
-	printf(_("  -F  --fsync-interval=INTERVAL\n"
-			 " frequency of syncs to the output file (in seconds, defaults to 10)\n"));
+	printf(_("  -F  --fsync-interval=SECS\n"
+			 " frequency of syncs to the output file (default: %d)\n"), (fsync_interval / 1000));
 	printf(_("  -o, --option=NAME[=VALUE]\n"
 			 " Specify option NAME with optional value VALUE, to be passed\n"
 			 " to the output plugin\n"));
-	printf(_("  -P, --plugin=PLUGINuse output plugin PLUGIN (defaults to test_decoding)\n"));
-	printf(_("  -s, --status-interval=INTERVAL\n"
-			 " time between status packets sent to server (in seconds, defaults to 10)\n"));
+	printf(_("  -P, --plugin=PLUGINuse output plugin PLUGIN (default: %s)\n"), plugin);
+	printf(_("  -s, --status-interval=SECS\n"
+			 " time between status packets sent to server (default: %d)\n"), (standby_message_timeout / 1000));
 	printf(_("  -S, --slot=SLOTuse existing replication slot SLOT instead of starting a new one\n"));
 	printf(_("  -I, --startpos=PTR Where in an existing slot should the streaming start\n"));
 	printf(_("\nAction to be performed:\n"));
-- 
2.0.0.rc0

>From fc089e4946e2c8780eae34ac08d9b6839470e8b4 Mon Sep 17 00:00:00 2001
From: Euler Taveira 
Date: Wed, 14 May 2014 14:20:44 -0300
Subject: [PATCH 03/11] Fix typo.

s/pg_receivexlog/pg_recvlogical/
---
 doc/src/sgml/ref/pg_recvlogical.sgml | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/doc/src/sgml/ref/pg_recvlogical.sgml b/doc/src/sgml/ref/pg_recvlogical.sgml
index 4170c8e..edc52c0 100644
--- a/doc/src/sgml/ref/pg_recvlogical.sgml
+++ b/doc/src/sgml/ref/pg_recvlogical.sgml
@@ -226,13 +226,12 @@ PostgreSQL documentation
   --fsync-interval=interval_seconds
   

-How often should
-pg_receivexlog
-issue sync commands to ensure the --outputfile
-is safely flushed to disk without being asked by the server to do
-so. Specifying an interval of 0 disables issuing
-fsyncs altogether, while still reporting progress the server.
-In this case, data may be lost in the event of a crash.
+How often should pg_recvlogical issue sync
+commands to ensure the --outputfile is safely
+flushed to disk without being asked by the server to do so. Specifying
+an interval of 0 disables issuing fsyncs altogether,
+while still reporting progress the server.  In this case, data may be
+lost in the event of a crash.

   
  
-- 
2.0.0.rc0

>From 068eedaef58f174de81589de2e50bcafde39dd46 Mon Sep 17

Re: [HACKERS] Cache invalidation bug in RelationGetIndexAttrBitmap()

2014-05-14 Thread Andres Freund
On 2014-05-14 13:32:43 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > On 2014-05-14 12:15:27 -0400, Tom Lane wrote:
> >> And why does the header
> >> comment for RelationGetIndexList make no mention of this new side-effect?
> >> Somebody did a seriously poor job of adding this functionality to
> >> relcache.
> 
> > It's not like it's not documented: There's a comment about it in struct
> > RelationData. I must have missed that rd_oidindex has a comment abou
> > it's lifetime over RelationGetIndexList().
> 
> If rd_replidindex is being managed like rd_oidindex, then it should be
> managed just like rd_oidindex, including getting reset in all the places
> rd_oidindex is.  This might be just a matter of cleanliness but I think
> it's important for readability and debuggability.

Agreed. I am not against resetting it. I think I might not have been
aware of rd_oidindex when writing that code...

> I notice also that rd_keyattr and rd_idattr have been implemented with
> bad copies of the logic for rd_indexattr.  This is at least leading
> to a permanent memory leak in CacheMemoryContext during every relcache
> flush, and maybe worse things.  The bugs for rd_keyattr appear to predate
> your patch though.

Hm. Yes, the bitmapsets should be freed. I guess I copied the logic for
keyattr and didn't find any relevant places that touch it. rd_keyattr
should go back to 9.3.

> Working on a patch for this now.  One thing I'm wondering about is
> RelationSetIndexList.  It's probably okay for it not to touch rd_keyattr
> and rd_idattr, but I'm not too clear on what the use cases for those
> attnum sets are.

rd_keyattr is used to determine whether a heap_update() changed any keys
that could be referenced by a foreign key. That's then used to determine
which locklevel an update requires.
rd_idattr does something similar. It decides whether the configured
REPLICA IDENTITY key has changed so whether to log the old primary key
for logical decoding or not.

I can't see why either would need to care about forced index lists right
now, but will do a scan of the sources to see if I am wrong.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] 9.4 release notes

2014-05-14 Thread Dean Rasheed
On 14 May 2014 15:07, Bruce Momjian  wrote:
> On Wed, May 14, 2014 at 08:33:15AM +0100, Dean Rasheed wrote:
>> On 4 May 2014 13:46, Bruce Momjian  wrote:
>> > I have completed the initial version of the 9.4 release notes.  You can
>> > view them here:
>> >
>> > http://www.postgresql.org/docs/devel/static/release-9-4.html
>> >
>> > I will be adding additional markup in the next few days.
>> >
>> > Feedback expected and welcomed.  I expect to be modifying this until we
>> > release 9.4 final.  I have marked items where I need help with question
>> > marks.
>> >
>>
>> In a few places, I think "updateable" should be spelled "updatable"
>> for consistency with the rest of the documentation (although I think
>> both spellings are actually correct).
>>
>>
>> ===
>> Allow the updating of views where only some columns are
>> auto-updateable (Dean Rasheed)
>>
>> Previously the presence of a non-auto-updateable column prevented all
>> columns from being auto-updated. Deletes are now supported on suitable
>> views even if no auto-updateable columns are present.
>> ===
>>
>> I think that puts too much emphasis on deletes, and could be
>> misinterpreted. How about something like this:
>>
>> Allow views to be automatically updatable even if they contain some
>> non-updatable columns (Dean Rasheed)
>>
>> Previously the presence of non-updatable columns such as expressions,
>> literals and functions prevented automatic updates.  Now INSERTs,
>> UPDATEs and DELETEs are supported, provided that they do not attempt
>> to assign new values to any of the non-updatable columns.
>
> Agreed.  Adjusted attached patch applied.  I was never happy with the
> previous awkward auto-update wording.
>

Thanks. There's a typo in the new text though --- "cals" should be "calls".

Regards,
Dean


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Typo in release notes

2014-05-14 Thread Bruce Momjian
fOn Wed, May 14, 2014 at 07:40:03AM +0400, Sergey Muraviov wrote:
> Hi.
> 
> Here's a patch that fixes it.

Thanks, applied.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Cache invalidation bug in RelationGetIndexAttrBitmap()

2014-05-14 Thread Andres Freund
On 2014-05-14 12:15:27 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > On 2014-05-14 15:17:39 +0200, Andres Freund wrote:
> >> My gut feeling says it's in RelationGetIndexList().
> 
> > Nearly right. It's in RelationGetIndexAttrBitmap(). Fix attached.
> 
> TBH, I don't believe this patch at all.  Where exactly is rd_replidindex
> reset?  If it's supposed to have similar lifespan to, say, rd_oidindex,
> why isn't it being handled like rd_oidindex? 

I don't see why it'd have a different lifespan than rd_oidindex at all?
If the latter were used inside the loop it'd be a bug as well.

> And why does the header
> comment for RelationGetIndexList make no mention of this new side-effect?
> Somebody did a seriously poor job of adding this functionality to
> relcache.

It's not like it's not documented: There's a comment about it in struct
RelationData. I must have missed that rd_oidindex has a comment abou
it's lifetime over RelationGetIndexList().
I personally actually prefer the struct as the location for the
lifetime. I can send a patch to commonalize the location in either place
with the other location pointing to it.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] btree_gist valgrind warnings about uninitialized memory

2014-05-14 Thread Tom Lane
Andres Freund  writes:
> On 2014-05-14 10:07:18 -0400, Tom Lane wrote:
>> I think that's an OK restriction as long as we warn people about it
>> (you could update a replication pair as long as you shut them both
>> down cleanly at the same time, right?).  Can the WAL replay routine
>> be made to detect incompatible records?

> We could just bump the wal version. Somewhat surprisingly that works if
> both nodes are shutdown cleanly (primary first)... But the errors about
> it are really ugly (will moan about unusable checkpoints), so it's
> probably not a good idea. Especially as it'll make it an issue for all
> users, not just the ones creating spgist indexes.

Yeah, I don't think we want to bump the WAL version code post-beta1.

Probably better to assign the modified spgist record a new xl_info ID
number, so that a beta1 slave would throw an error for it.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Cache invalidation bug in RelationGetIndexAttrBitmap()

2014-05-14 Thread Tom Lane
Andres Freund  writes:
> On 2014-05-14 15:17:39 +0200, Andres Freund wrote:
>> My gut feeling says it's in RelationGetIndexList().

> Nearly right. It's in RelationGetIndexAttrBitmap(). Fix attached.

TBH, I don't believe this patch at all.  Where exactly is rd_replidindex
reset?  If it's supposed to have similar lifespan to, say, rd_oidindex,
why isn't it being handled like rd_oidindex?  And why does the header
comment for RelationGetIndexList make no mention of this new side-effect?
Somebody did a seriously poor job of adding this functionality to
relcache.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] btree_gist valgrind warnings about uninitialized memory

2014-05-14 Thread Andres Freund
On 2014-05-14 10:07:18 -0400, Tom Lane wrote:
> Heikki Linnakangas  writes:
> > On 05/13/2014 05:13 PM, Andres Freund wrote:
> >> What's your plans with your spgist fix? Commit it once 9.5 is branched?
> 
> > Good question. I don't know. I would still like to commit it to 9.4. It 
> > doesn't require catalog changes, but it's an incompatible change in the 
> > WAL record format. If we commit it to 9.4, it means that you cannot 
> > replicate between 9.4beta1 and 9.4beta2. I think that's OK, but how do 
> > others feel about that?
> 
> I think that's an OK restriction as long as we warn people about it
> (you could update a replication pair as long as you shut them both
> down cleanly at the same time, right?).  Can the WAL replay routine
> be made to detect incompatible records?

We could just bump the wal version. Somewhat surprisingly that works if
both nodes are shutdown cleanly (primary first)... But the errors about
it are really ugly (will moan about unusable checkpoints), so it's
probably not a good idea. Especially as it'll make it an issue for all
users, not just the ones creating spgist indexes.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Wanna help PostgreSQL

2014-05-14 Thread Merlin Moncure
On Wed, May 14, 2014 at 10:58 AM, Tom Lane  wrote:
> Merlin Moncure  writes:
>> On Wed, May 14, 2014 at 9:44 AM, Tom Lane  wrote:
>>> The reason that that project has gone untouched for upwards of ten years
>>> is that it's not just a large coding project, but involves a lot of
>>> complex API design with uncertain goals.  It's not very clear what
>>> features people would want from a "pg_dump library", though one capability
>>> that gets mentioned often is the ability to extract the SQL definition
>>> for a single object.
>
>> Personally I'd prefer the creation of definitional SQL be moved out of
>> pg_dump and into the database proper via something like
>> 'pg_sql_definition(oid)' or something like that.
>
> Well, that's just a different way of packaging a library, no?  It doesn't
> make the library-API problems any less difficult.  If anything, it makes
> things even harder, because now you have to consider version skew between
> pg_dump and the server.  And if you get any API details wrong you have
> no ability to change them till the next major release cycle.
>
> While we might someday do it like that, I'd think it foolish to proceed
> with such an approach until we had a proven library API design on the
> client side.  The costs of iterating there are a lot less.

Yeah -- Andres said it even more cleanly: for forward upgrades you'd
need to communicate with both versioned backends to produce a dump.
That's not a complete deal breaker but definitely a lot more complex
than I was thinking.

merlin


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Wanna help PostgreSQL

2014-05-14 Thread Tom Lane
Merlin Moncure  writes:
> On Wed, May 14, 2014 at 9:44 AM, Tom Lane  wrote:
>> The reason that that project has gone untouched for upwards of ten years
>> is that it's not just a large coding project, but involves a lot of
>> complex API design with uncertain goals.  It's not very clear what
>> features people would want from a "pg_dump library", though one capability
>> that gets mentioned often is the ability to extract the SQL definition
>> for a single object.

> Personally I'd prefer the creation of definitional SQL be moved out of
> pg_dump and into the database proper via something like
> 'pg_sql_definition(oid)' or something like that.

Well, that's just a different way of packaging a library, no?  It doesn't
make the library-API problems any less difficult.  If anything, it makes
things even harder, because now you have to consider version skew between
pg_dump and the server.  And if you get any API details wrong you have
no ability to change them till the next major release cycle.

While we might someday do it like that, I'd think it foolish to proceed
with such an approach until we had a proven library API design on the
client side.  The costs of iterating there are a lot less.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Wanna help PostgreSQL

2014-05-14 Thread Andres Freund
On 2014-05-14 10:49:05 -0500, Merlin Moncure wrote:
> On Wed, May 14, 2014 at 9:44 AM, Tom Lane  wrote:
> > The reason that that project has gone untouched for upwards of ten years
> > is that it's not just a large coding project, but involves a lot of
> > complex API design with uncertain goals.  It's not very clear what
> > features people would want from a "pg_dump library", though one capability
> > that gets mentioned often is the ability to extract the SQL definition
> > for a single object.
> 
> Personally I'd prefer the creation of definitional SQL be moved out of
> pg_dump and into the database proper via something like
> 'pg_sql_definition(oid)' or something like that.  There are lot of
> reasons applications (especially administrative ones like pgadmin and
> psql but also end user applications in some cases) would want to do
> that and forcing everything through pg_dump et al is awkward.  The
> less magic in the external applications the better.

That'd be a separate feature from pg_dump though. pg_dump needs to be
cross-version compatible and the above prevents that...

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Cache invalidation bug in RelationGetIndexAttrBitmap()

2014-05-14 Thread Andres Freund
On 2014-05-14 15:17:39 +0200, Andres Freund wrote:
> On 2014-05-14 15:08:08 +0200, Tomas Vondra wrote:
> > Apparently there's something wrong with 'test-decoding-check':
> 
> Man. I shouldn't have asked... My code. There's some output in there
> that's probably triggered by the extraordinarily long runtimes, but
> there's definitely something else wrong.
> My gut feeling says it's in RelationGetIndexList().

Nearly right. It's in RelationGetIndexAttrBitmap(). Fix attached.

Tomas, thanks for that. I've never (and probably will never) run
CLOBBER_CACHE_RECURSIVELY during development. Having a machine do that
regularly is really helpful. How long does a single testrun take? It
takes hundreds of seconds here to do a single UPDATE?

There were some more differences but those are all harmless and caused
by the extraordinarily long runtime (autovacuums). I think we need to
add a feature to test_decoding to suppress displaying transactions
without changes. Ick.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From 9dfe879d2b6940b6072e277b0104e9cbe4af691e Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Wed, 14 May 2014 17:12:57 +0200
Subject: [PATCH] Fix cache invalidation hazard in
 RelationGetIndexAttrBitmap().

When a cache invalidation arrived inside RelationGetIndexAttrBitmap()
inbetween the call to RelationGetIndexList() and one of the
index_open() calls relation->rd_replidindex would be reset leading to
a corrupted INDEX_ATTR_BITMAP_IDENTITY_KEY return value. That in turn
could lead to the old REPLICA IDENTITY not being logged if set to
DEFAULT or INDEX.
---
 src/backend/utils/cache/relcache.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 5ff0d9e..4fc18b5 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -3976,6 +3976,7 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
 	List	   *indexoidlist;
 	ListCell   *l;
 	MemoryContext oldcxt;
+	Oid			relreplindex;
 
 	/* Quick exit if we already computed the result. */
 	if (relation->rd_indexattr != NULL)
@@ -4005,6 +4006,12 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
 		return NULL;
 
 	/*
+	 * Store after computing the index list above, to be safe against cache
+	 * flushes inside index_open() below.
+	 */
+	relreplindex = relation->rd_replidindex;
+
+	/*
 	 * For each index, add referenced attributes to indexattrs.
 	 *
 	 * Note: we consider all indexes returned by RelationGetIndexList, even if
@@ -4038,7 +4045,7 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
 			indexInfo->ii_Predicate == NIL;
 
 		/* Is this index the configured (or default) replica identity? */
-		isIDKey = indexOid == relation->rd_replidindex;
+		isIDKey = indexOid == relreplindex;
 
 		/* Collect simple attribute references */
 		for (i = 0; i < indexInfo->ii_NumIndexAttrs; i++)
-- 
2.0.0.rc2.4.g1dc51c6.dirty


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Wanna help PostgreSQL

2014-05-14 Thread Merlin Moncure
On Wed, May 14, 2014 at 9:44 AM, Tom Lane  wrote:
> The reason that that project has gone untouched for upwards of ten years
> is that it's not just a large coding project, but involves a lot of
> complex API design with uncertain goals.  It's not very clear what
> features people would want from a "pg_dump library", though one capability
> that gets mentioned often is the ability to extract the SQL definition
> for a single object.

Personally I'd prefer the creation of definitional SQL be moved out of
pg_dump and into the database proper via something like
'pg_sql_definition(oid)' or something like that.  There are lot of
reasons applications (especially administrative ones like pgadmin and
psql but also end user applications in some cases) would want to do
that and forcing everything through pg_dump et al is awkward.  The
less magic in the external applications the better.

merlin


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..

2014-05-14 Thread Robert Haas
On Sun, May 11, 2014 at 12:47 PM, Tom Lane  wrote:
> Simon Riggs  writes:
>> On 11 May 2014 11:18, Andres Freund  wrote:
>>> I don't know. I'd find UPDATE/DELETE ORDER BY something rather
>>> useful.
>
>> Perhaps if an index exists to provide an ordering that makes it clear
>> what this means, then yes.
>
> The $64 question is whether we'd accept an implementation that fails
> if the target table has children (ie, is partitioned).

I'd say "no".  Partitioning is important, and we need to make it more
seamless and better-integrated, not add new warts.

> That seems
> to me to not be up to the project's usual quality expectations, but
> maybe if there's enough demand for a partial solution we should do so.

I like this feature, but if I were searching for places where it makes
sense to loosen our project's usual quality expectations, this isn't
where I'd start.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] buildfarm / handling (undefined) locales

2014-05-14 Thread Andrew Dunstan


On 05/14/2014 11:02 AM, Tom Lane wrote:

I wrote:

As far as Windows goes, we could certainly use #ifdef WIN32 to print
some different text, if anyone can write down what it should be.

Some googling suggests that on Windows, setlocale pays no attention
to environment variables but gets a value that the user can set via
the Control Panel.


Indeed. There is a comment in plperl.c about that, because we have to 
deal with it in a somewhat ugly fashion.




This probably means that an invalid setting is
impossible, or at least that we have no clear idea how the user could
screw it up.  So I'm thinking we do not need a Windows-specific
phrasing of the message, until such time as we see evidence of how
you could provoke this case on Windows.




Right. I'd just leave Windows out of it for now.

cheers

andrew



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Re: popen and pclose redefinitions causing many warning in Windows build

2014-05-14 Thread Noah Misch
On Wed, May 14, 2014 at 05:51:24PM +0300, Heikki Linnakangas wrote:
> On 05/14/2014 05:37 PM, Noah Misch wrote:
> >On Wed, May 14, 2014 at 03:15:38PM +0300, Heikki Linnakangas wrote:
> >>On 05/09/2014 02:56 AM, Noah Misch wrote:
> >>>MinGW: 
> >>>http://sourceforge.net/p/mingw/mingw-org-wsl/ci/master/tree/include/stdio.h#l467
> >>>MinGW-w64: 
> >>>http://sourceforge.net/p/mingw-w64/code/HEAD/tree/trunk/mingw-w64-headers/crt/stdio.h#l496
> >>>
> >>>Building with any recent MinGW-w64, 32-bit or 64-bit, gets the reported
> >>>warnings; building with MinGW proper does not.
> >>
> >>Hmm. The MinGW-w64 header does this:
> >>
> >>>#if !defined(NO_OLDNAMES) && !defined(popen)
> >>>#define popen _popen
> >>>#define pclose _pclose
> >>>#endif
> >>
> >>So if we defined popen() before including stdio.h, that would get
> >>rid of the warning. But we don't usually do things in that order.
> >
> >True.  I have no strong preference between that and use of #undef.
> 
> I think I would prefer #undef. The risk with that is if some
> platform has #defined popen() to something else entirely, for some
> good reason, we would be bypassing that hypothetical wrapper. But I
> guess we'll cross that bridge if we get there.

Works for me.  Since "(popen)(x, y)" shall behave the same as "popen(x, y)",
such a hypothetical system header would be buggy, anyway.

> >>Could we define NO_OLDNAMES? I couldn't find any documentation on
> >>it, but it seems to a bunch of lot of wrapper functions and defines.
> >>If we can get away without them, that seems like a good thing...
> >
> >That's a bit like compiling with "gcc -std=c89" on Unix.  It would lead us to
> >add "#define strdup(x) _strdup(x)" and similar.  I wouldn't do that.
> 
> Ugh. I can't believe they marked strdup(x) as deprecated.

That reflected the function's absence from ISO C, not a value judgement.

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] buildfarm / handling (undefined) locales

2014-05-14 Thread Tom Lane
I wrote:
> As far as Windows goes, we could certainly use #ifdef WIN32 to print
> some different text, if anyone can write down what it should be.

Some googling suggests that on Windows, setlocale pays no attention
to environment variables but gets a value that the user can set via
the Control Panel.  This probably means that an invalid setting is
impossible, or at least that we have no clear idea how the user could
screw it up.  So I'm thinking we do not need a Windows-specific
phrasing of the message, until such time as we see evidence of how
you could provoke this case on Windows.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Re: popen and pclose redefinitions causing many warning in Windows build

2014-05-14 Thread Heikki Linnakangas

On 05/14/2014 05:37 PM, Noah Misch wrote:

On Wed, May 14, 2014 at 03:15:38PM +0300, Heikki Linnakangas wrote:

On 05/09/2014 02:56 AM, Noah Misch wrote:

MinGW: 
http://sourceforge.net/p/mingw/mingw-org-wsl/ci/master/tree/include/stdio.h#l467
MinGW-w64: 
http://sourceforge.net/p/mingw-w64/code/HEAD/tree/trunk/mingw-w64-headers/crt/stdio.h#l496

Building with any recent MinGW-w64, 32-bit or 64-bit, gets the reported
warnings; building with MinGW proper does not.


Hmm. The MinGW-w64 header does this:


#if !defined(NO_OLDNAMES) && !defined(popen)
#define popen _popen
#define pclose _pclose
#endif


So if we defined popen() before including stdio.h, that would get
rid of the warning. But we don't usually do things in that order.


True.  I have no strong preference between that and use of #undef.


I think I would prefer #undef. The risk with that is if some platform 
has #defined popen() to something else entirely, for some good reason, 
we would be bypassing that hypothetical wrapper. But I guess we'll cross 
that bridge if we get there.



Could we define NO_OLDNAMES? I couldn't find any documentation on
it, but it seems to a bunch of lot of wrapper functions and defines.
If we can get away without them, that seems like a good thing...


That's a bit like compiling with "gcc -std=c89" on Unix.  It would lead us to
add "#define strdup(x) _strdup(x)" and similar.  I wouldn't do that.


Ugh. I can't believe they marked strdup(x) as deprecated.

- Heikki


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Wanna help PostgreSQL

2014-05-14 Thread Tom Lane
Zhe-Wei Jiang  writes:
>> I read the GSoC ideas page and think the only one I'm possibly to help for
>> now is
>> "Rewrite (add) pg_dump and pg_restore utilities as libraries (.so, .dll &
>> .dylib)",
>> while I'm still not clear with the whole postgreSQL project.
>> 
>> If my understanding is correct, taking pg_dump as an example, this should
>> include refactoring the pg_dump.c and rewrite the Makefile to make it as
>> shared libraries.
>> Do I miss anything?

The reason that that project has gone untouched for upwards of ten years
is that it's not just a large coding project, but involves a lot of
complex API design with uncertain goals.  It's not very clear what
features people would want from a "pg_dump library", though one capability
that gets mentioned often is the ability to extract the SQL definition
for a single object.  So before anything else you'd need to identify a
satisfactory set of library capabilities.  The next nasty problem is that
pg_dump has a large set of odd behaviors that have evolved for good and
sufficient reason, and that we'd not want to give up, but that it's not
clear whether anyone else would want --- and they'd complicate any API
definition quite a bit.  One example is that pg_dump knows how to dump
objects in a safe order to avoid forward references.  If there turn out
to be circular references (which arise in more cases than you might think)
it even knows how to split certain kinds of objects into multiple
commands so as to break the circularity.  How would we expose all that?
The features for parallel pg_dump and pg_restore are another thing that
doesn't seem to fit all that well into a clean library API.  But how much
of this should actually be in a library, rather than in the wrapper
programs?

So while this is certainly a worthwhile task, it's not one to
underestimate the scope and difficulty of.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Re: popen and pclose redefinitions causing many warning in Windows build

2014-05-14 Thread Noah Misch
On Wed, May 14, 2014 at 03:15:38PM +0300, Heikki Linnakangas wrote:
> On 05/09/2014 02:56 AM, Noah Misch wrote:
> >MinGW: 
> >http://sourceforge.net/p/mingw/mingw-org-wsl/ci/master/tree/include/stdio.h#l467
> >MinGW-w64: 
> >http://sourceforge.net/p/mingw-w64/code/HEAD/tree/trunk/mingw-w64-headers/crt/stdio.h#l496
> >
> >Building with any recent MinGW-w64, 32-bit or 64-bit, gets the reported
> >warnings; building with MinGW proper does not.
> 
> Hmm. The MinGW-w64 header does this:
> 
> >#if !defined(NO_OLDNAMES) && !defined(popen)
> >#define popen _popen
> >#define pclose _pclose
> >#endif
> 
> So if we defined popen() before including stdio.h, that would get
> rid of the warning. But we don't usually do things in that order.

True.  I have no strong preference between that and use of #undef.

> Could we define NO_OLDNAMES? I couldn't find any documentation on
> it, but it seems to a bunch of lot of wrapper functions and defines.
> If we can get away without them, that seems like a good thing...

That's a bit like compiling with "gcc -std=c89" on Unix.  It would lead us to
add "#define strdup(x) _strdup(x)" and similar.  I wouldn't do that.

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Wanna help PostgreSQL

2014-05-14 Thread Zhe-Wei Jiang
Thanks Jeff.

2014-05-14 2:19 GMT+08:00 Jeff Janes :

> On Tue, May 13, 2014 at 10:12 AM, Zhe-Wei Jiang wrote:
>
>> Hi everyone,
>>
>> I'm new to the open source community, and wanna help in some
>> implementation work.
>> However, when I looked into the TODO list, I found most of the items are
>> very old.
>>
>
> So that is one thing you can do to help, try to improve the todo page!
>  I've thought we should create a new badge for staleness (in addition to
> the Easy and Done ones) and add it to everything currently on the page,
> then remove it from ones that were recently evaluated and still seem to be
> relevant.
>
> And in the process, we should clarify why they haven't been done yet.
>  Often what really needs to be done is not the implementation, but rather
> the evaluation of whether it is worth the trade offs involved.
>

Aren't I too unfamiliar with the postgreSQL project to help on this?
I think this should be coordinated by a more experienced contributor.


>
>
>> Therefore, I'd like to know if there is any entry-level implementation
>> needed by PostgreSQL now?
>>
>
> Did anything on the todo list strike your fancy, especially anything
> marked Easy?  Even if it is old, it still might be valid. Then discuss it
> here on the hackers list, and if it not still valid we can remove it from
> the list.  No point putting the sour milk back in the fridge.
>
>
The following two look easies to me:
[E] [image: Incomplete
item]Add
full object name to the tag field. eg. for operators we need '=(integer,
integer)', instead of just '='. [E] [image: Incomplete
item]Modify
pg_dump to create skeleton views for reload (which are then updated via
CREATE OR REPLACE VIEW) when views have circular dependencies. This should
eliminate the need for the CREATE RULE "_RETURN" hack currently used to
address this issue.

Are they still in need?

Besides, for the following:
[E] [image: Incomplete
item]Remove
warnings created by -Wcast-align

I tried to add -Wcast-align in the latest git source but found no warning.
Should this be removed from the TODO page?


> How much experience do you have using PostgreSQL (or for that matter,
> coding in C)?
>
>
Actually I never used PostgreSQL before.
I used to run mySQL but also not much experience.
I coded C++ for ~10 years and I guess C should not bring too much trouble.


> Cheers,
>
> Jeff
>

Regards,
Zhe-Wei Jiang


Re: [HACKERS] Wanna help PostgreSQL

2014-05-14 Thread Zhe-Wei Jiang
See, that's beyond my imagination.
Then I have to do more research first before I can understand the other
ideas.

Regards,
Zhe-Wei Jiang



2014-05-14 22:25 GMT+08:00 Stephen Frost :

> Zhe-Wei,
>
> * Zhe-Wei Jiang (jrreinha...@gmail.com) wrote:
> > I read the GSoC ideas page and think the only one I'm possibly to help
> for
> > now is
> > "Rewrite (add) pg_dump and pg_restore utilities as libraries (.so, .dll &
> > .dylib)",
> > while I'm still not clear with the whole postgreSQL project.
>
> It's not clear to me that we actually want that one, and even if we do,
> it's actually a huge amount of work...
>
> Thanks,
>
> Stephen
>


Re: [HACKERS] buildfarm / handling (undefined) locales

2014-05-14 Thread Tomas Vondra
On 14 Květen 2014, 16:26, Heikki Linnakangas wrote:
> On 05/14/2014 05:19 PM, Tom Lane wrote:
>
>> Anyway, given that we seem to have consensus on doing this (modulo
>> exact text of the error message), the next question is whether we
>> want to change this only in HEAD, or consider it a back-patchable
>> bug fix.  I lean to the former but can see that there's some
>> argument for the latter.
>
> HEAD-only. There might be scripts out there that do initdb with invalid
> LANG. They might be perfectly happy with getting the C locale, and
> backpatching this would make them unhappy.

Not sure if 'happy with broken config' is the right argument here, but it
certainly is a behaviour change / not a bugfix, so +1 for HEAD-only from
me too.

Tomas



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] buildfarm / handling (undefined) locales

2014-05-14 Thread Heikki Linnakangas

On 05/14/2014 05:19 PM, Tom Lane wrote:

Heikki Linnakangas  writes:

On 05/14/2014 12:07 AM, Tom Lane wrote:

Well, all we know is that we asked setlocale for the default locale from
the environment and it failed.  We don't really know which variable(s)
it looked at.



Printing the values of the environment variables sounds complicated, but
I think a generic hint along these lines would be good:



initdb: environment locale settings are invalid
HINT: This usually means that the LANG, LC_ALL or LC_* environment
variable has an invalid value.


Hm.  Keep in mind this is not backend code so we don't have a HINT
formalism.  How about

initdb: invalid locale settings; check LANG and LC_* environment variables


Works for me. I note that we have used the HINT: wording in pg_ctl, too:

pg_ctl: server does not shut down
HINT: The "-m fast" option immediately disconnects sessions rather than
waiting for session-initiated disconnection.

That's the only client program that does that, though, so perhaps we 
want to reconsider that in pg_ctl.



Anyway, given that we seem to have consensus on doing this (modulo
exact text of the error message), the next question is whether we
want to change this only in HEAD, or consider it a back-patchable
bug fix.  I lean to the former but can see that there's some
argument for the latter.


HEAD-only. There might be scripts out there that do initdb with invalid 
LANG. They might be perfectly happy with getting the C locale, and 
backpatching this would make them unhappy.


- Heikki


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Re: popen and pclose redefinitions causing many warning in Windows build

2014-05-14 Thread Andrew Dunstan


On 05/14/2014 08:15 AM, Heikki Linnakangas wrote:

On 05/09/2014 02:56 AM, Noah Misch wrote:

On Thu, May 08, 2014 at 12:14:44PM -0400, Tom Lane wrote:

Andrew Dunstan  writes:

I'm pretty sure we need this on Mingw - this SYSTEMQUOTE stuff dates
back well before 8.3, IIRC, which is when we first got full MSVC 
support.


I tried googling for some info on this, and got a number of hits
suggesting that mingw didn't emulate popen at all till pretty recently.
For instance this:
https://lists.fedoraproject.org/pipermail/fedora-mingw/2009-September/002087.html 


Jones is an ex-coworker of mine, and I'm pretty sure that if he said
it wasn't there then it wasn't there.


I doubt MinGW has overridden popen() at runtime; that would be 
contrary to its
design criteria.  The headers, however, are MinGW territory. MinGW 
declares
both _popen() and popen() as functions.  MinGW-w64, a project more 
distinct

from MinGW than it sounds, uses "#define popen _popen":

MinGW: 
http://sourceforge.net/p/mingw/mingw-org-wsl/ci/master/tree/include/stdio.h#l467
MinGW-w64: 
http://sourceforge.net/p/mingw-w64/code/HEAD/tree/trunk/mingw-w64-headers/crt/stdio.h#l496


Building with any recent MinGW-w64, 32-bit or 64-bit, gets the reported
warnings; building with MinGW proper does not.


Hmm. The MinGW-w64 header does this:


#if !defined(NO_OLDNAMES) && !defined(popen)
#define popen _popen
#define pclose _pclose
#endif


So if we defined popen() before including stdio.h, that would get rid 
of the warning. But we don't usually do things in that order.


Could we define NO_OLDNAMES? I couldn't find any documentation on it, 
but it seems to a bunch of lot of wrapper functions and defines. If we 
can get away without them, that seems like a good thing...






Why don't we simply undefine it if it's defined before we redefine it? 
We don't need or want their definition, as our implementation calls 
_popen directly.


cheers

andrew




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] buildfarm / handling (undefined) locales

2014-05-14 Thread Tom Lane
Heikki Linnakangas  writes:
> On 05/14/2014 12:07 AM, Tom Lane wrote:
>> Well, all we know is that we asked setlocale for the default locale from
>> the environment and it failed.  We don't really know which variable(s)
>> it looked at.

> Printing the values of the environment variables sounds complicated, but 
> I think a generic hint along these lines would be good:

> initdb: environment locale settings are invalid
> HINT: This usually means that the LANG, LC_ALL or LC_* environment 
> variable has an invalid value.

Hm.  Keep in mind this is not backend code so we don't have a HINT
formalism.  How about

initdb: invalid locale settings; check LANG and LC_* environment variables

> Specifically mentioning those environment variables by name would be 
> very helpful to the user. They are specified by POSIX: 
> http://pubs.opengroup.org/onlinepubs/7908799/xbd/envvar.html, so they're 
> not that platform-specific. Windows is a different story, though.

I seem to recall having heard that some implementations check LANGUAGE
and perhaps other spellings as well.  Still, this might be better than
assuming the user has a clue what to check.

As far as Windows goes, we could certainly use #ifdef WIN32 to print
some different text, if anyone can write down what it should be.

Anyway, given that we seem to have consensus on doing this (modulo
exact text of the error message), the next question is whether we
want to change this only in HEAD, or consider it a back-patchable
bug fix.  I lean to the former but can see that there's some
argument for the latter.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Wanna help PostgreSQL

2014-05-14 Thread Zhe-Wei Jiang
Forgot to include pgsql-hackers.

2014-05-14 22:11 GMT+08:00 Zhe-Wei Jiang :

> Thanks Stephen.
>
> I read the GSoC ideas page and think the only one I'm possibly to help for
> now is
> "Rewrite (add) pg_dump and pg_restore utilities as libraries (.so, .dll &
> .dylib)",
> while I'm still not clear with the whole postgreSQL project.
>
> If my understanding is correct, taking pg_dump as an example, this should
> include refactoring the pg_dump.c and rewrite the Makefile to make it as
> shared libraries.
> Do I miss anything?
>
> Regards,
> Zhe-Wei Jiang
>
>
>
> 2014-05-14 1:49 GMT+08:00 Stephen Frost :
>
> * Zhe-Wei Jiang (jrreinha...@gmail.com) wrote:
>> > Therefore, I'd like to know if there is any entry-level implementation
>> > needed by PostgreSQL now?
>> > Any advise is appreciated.
>>
>> You might take a look at the GSoC ideas page instead:
>>
>> https://wiki.postgresql.org/wiki/GSoC_2014
>>
>> Thanks,
>>
>> Stephen
>>
>
>


Re: [HACKERS] btree_gist valgrind warnings about uninitialized memory

2014-05-14 Thread Tom Lane
Heikki Linnakangas  writes:
> On 05/13/2014 05:13 PM, Andres Freund wrote:
>> What's your plans with your spgist fix? Commit it once 9.5 is branched?

> Good question. I don't know. I would still like to commit it to 9.4. It 
> doesn't require catalog changes, but it's an incompatible change in the 
> WAL record format. If we commit it to 9.4, it means that you cannot 
> replicate between 9.4beta1 and 9.4beta2. I think that's OK, but how do 
> others feel about that?

I think that's an OK restriction as long as we warn people about it
(you could update a replication pair as long as you shut them both
down cleanly at the same time, right?).  Can the WAL replay routine
be made to detect incompatible records?

What worries me more is that post-beta1 fixes will, by definition,
get noticeably less beta testing than anything that went out in beta1.
So how confident are you in this fix?  Is it something we'd consider
back-patching in the absence of a WAL-format issue?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] 9.4 release notes

2014-05-14 Thread Bruce Momjian
On Wed, May 14, 2014 at 08:33:15AM +0100, Dean Rasheed wrote:
> On 4 May 2014 13:46, Bruce Momjian  wrote:
> > I have completed the initial version of the 9.4 release notes.  You can
> > view them here:
> >
> > http://www.postgresql.org/docs/devel/static/release-9-4.html
> >
> > I will be adding additional markup in the next few days.
> >
> > Feedback expected and welcomed.  I expect to be modifying this until we
> > release 9.4 final.  I have marked items where I need help with question
> > marks.
> >
> 
> In a few places, I think "updateable" should be spelled "updatable"
> for consistency with the rest of the documentation (although I think
> both spellings are actually correct).
> 
> 
> ===
> Allow the updating of views where only some columns are
> auto-updateable (Dean Rasheed)
> 
> Previously the presence of a non-auto-updateable column prevented all
> columns from being auto-updated. Deletes are now supported on suitable
> views even if no auto-updateable columns are present.
> ===
> 
> I think that puts too much emphasis on deletes, and could be
> misinterpreted. How about something like this:
> 
> Allow views to be automatically updatable even if they contain some
> non-updatable columns (Dean Rasheed)
> 
> Previously the presence of non-updatable columns such as expressions,
> literals and functions prevented automatic updates.  Now INSERTs,
> UPDATEs and DELETEs are supported, provided that they do not attempt
> to assign new values to any of the non-updatable columns.

Agreed.  Adjusted attached patch applied.  I was never happy with the
previous awkward auto-update wording.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +
diff --git a/doc/src/sgml/release-9.4.sgml b/doc/src/sgml/release-9.4.sgml
new file mode 100644
index cabfbdd..eb0b4f8
*** a/doc/src/sgml/release-9.4.sgml
--- b/doc/src/sgml/release-9.4.sgml
***
*** 1097,1118 
  

 
! Allow the updating of views
! where only some columns are auto-updateable (Dean Rasheed)
 
  
 
! Previously the presence of a non-auto-updateable column prevented
! all columns from being auto-updated.  Deletes are now supported
! on suitable views even if no auto-updateable columns are present.
 

  

 
  Allow control over whether INSERTs and
! UPDATEs can add rows to an auto-updateable view that
  would no longer appear in the view (Dean Rasheed)
 
  
--- 1097,1121 
  

 
! Allow views to be automatically
! updated even if they contain some non-updatable columns
! (Dean Rasheed)
 
  
 
! Previously the presence of non-updatable columns such as
! expressions, literals, and function cals prevented automatic
! updates.  Now INSERTs, UPDATEs and
! DELETEs are supported, provided that they do not
! attempt to assign new values to any of the non-updatable columns.
 

  

 
  Allow control over whether INSERTs and
! UPDATEs can add rows to an auto-updatable view that
  would no longer appear in the view (Dean Rasheed)
 
  
***
*** 1125,1131 

 
  Allow security barrier views
! to be automatically updateable (Dean Rasheed)
 

  
--- 1128,1134 

 
  Allow security barrier views
! to be automatically updatable (Dean Rasheed)
 

  

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Sending out a request for more buildfarm animals?

2014-05-14 Thread Andres Freund
On 2014-05-14 15:08:08 +0200, Tomas Vondra wrote:
> On 14 Květen 2014, 13:51, Andres Freund wrote:
> > On 2014-05-13 20:42:16 +0200, Tomas Vondra wrote:
> >> Can someone please approve the animals I've requested a few days ago?
> >> I'm already running the clobber tests with '--nosend --nostatus' and
> >> it's already reporting some errors. Would be nice to get it to the
> >> buildfarm.
> >
> > Can you provide some details about those failures until then?
> 
> Sure.

Thanks.

> Apparently there's something wrong with 'test-decoding-check':

Man. I shouldn't have asked... My code. There's some output in there
that's probably triggered by the extraordinarily long runtimes, but
there's definitely something else wrong.
My gut feeling says it's in RelationGetIndexList().

> This only happens on animals executed with
> 
> -DCLOBBER_CACHE_ALWAYS -DCLOBBER_FREED_MEMORY -DMEMORY_CONTEXT_CHECKING
> -DRANDOMIZE_ALLOCATED_MEMORY -DCLOBBER_CACHE_RECURSIVELY
> 
> it does not happen with
> 
> CPPFLAGS => '-DCLOBBER_CACHE_ALWAYS -DCLOBBER_FREED_MEMORY
> -DMEMORY_CONTEXT_CHECKING -DRANDOMIZE_ALLOCATED_MEMORY',
> 
> So clearly this is about CLOBBER_CACHE_RECURSIVELY. Also, it fails on all
> three animals (one for each compiler - gcc, icc, clang).

I tested it with CLOBBER_CACHE_ALWAYS, but not RECURSIVELY... So it's
entirely possible that i've missed something.


Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Sending out a request for more buildfarm animals?

2014-05-14 Thread Tomas Vondra
On 14 Květen 2014, 13:51, Andres Freund wrote:
> On 2014-05-13 20:42:16 +0200, Tomas Vondra wrote:
>> Can someone please approve the animals I've requested a few days ago?
>> I'm already running the clobber tests with '--nosend --nostatus' and
>> it's already reporting some errors. Would be nice to get it to the
>> buildfarm.
>
> Can you provide some details about those failures until then?

Sure.

Apparently there's something wrong with 'test-decoding-check':

== running regression test queries==
test ddl  ... FAILED
test rewrite  ... ok
test toast... FAILED
test permissions  ... ok
test decoding_in_xact ... ok
test binary   ... ok
== shutting down postmaster   ==

The whole logfile is attached, complete logs are available at
http://www.fuzzy.cz/tmp/buildlogs.tgz

This only happens on animals executed with

-DCLOBBER_CACHE_ALWAYS -DCLOBBER_FREED_MEMORY -DMEMORY_CONTEXT_CHECKING
-DRANDOMIZE_ALLOCATED_MEMORY -DCLOBBER_CACHE_RECURSIVELY

it does not happen with

CPPFLAGS => '-DCLOBBER_CACHE_ALWAYS -DCLOBBER_FREED_MEMORY
-DMEMORY_CONTEXT_CHECKING -DRANDOMIZE_ALLOCATED_MEMORY',

So clearly this is about CLOBBER_CACHE_RECURSIVELY. Also, it fails on all
three animals (one for each compiler - gcc, icc, clang).

regards
Tomas
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] btree_gist valgrind warnings about uninitialized memory

2014-05-14 Thread Andres Freund
On 2014-05-14 14:55:38 +0300, Heikki Linnakangas wrote:
> On 05/13/2014 05:13 PM, Andres Freund wrote:
> >That leaves the spgist thing and some infrastructure till we can setup a
> >valgrind animal... From what we've caught with it so far that seems
> >rather worthwile.
> >What's your plans with your spgist fix? Commit it once 9.5 is branched?
> 
> Good question. I don't know. I would still like to commit it to 9.4. It
> doesn't require catalog changes, but it's an incompatible change in the WAL
> record format. If we commit it to 9.4, it means that you cannot replicate
> between 9.4beta1 and 9.4beta2. I think that's OK, but how do others feel
> about that?

I personally think that's not too bad.

Alternatively we could come up with a second way that's not breaking the
WAL format. I'd feel better if we had a back patchable fix...

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Re: popen and pclose redefinitions causing many warning in Windows build

2014-05-14 Thread Heikki Linnakangas

On 05/09/2014 02:56 AM, Noah Misch wrote:

On Thu, May 08, 2014 at 12:14:44PM -0400, Tom Lane wrote:

Andrew Dunstan  writes:

I'm pretty sure we need this on Mingw - this SYSTEMQUOTE stuff dates
back well before 8.3, IIRC, which is when we first got full MSVC support.


I tried googling for some info on this, and got a number of hits
suggesting that mingw didn't emulate popen at all till pretty recently.
For instance this:
https://lists.fedoraproject.org/pipermail/fedora-mingw/2009-September/002087.html
Jones is an ex-coworker of mine, and I'm pretty sure that if he said
it wasn't there then it wasn't there.


I doubt MinGW has overridden popen() at runtime; that would be contrary to its
design criteria.  The headers, however, are MinGW territory.  MinGW declares
both _popen() and popen() as functions.  MinGW-w64, a project more distinct
from MinGW than it sounds, uses "#define popen _popen":

MinGW: 
http://sourceforge.net/p/mingw/mingw-org-wsl/ci/master/tree/include/stdio.h#l467
MinGW-w64: 
http://sourceforge.net/p/mingw-w64/code/HEAD/tree/trunk/mingw-w64-headers/crt/stdio.h#l496

Building with any recent MinGW-w64, 32-bit or 64-bit, gets the reported
warnings; building with MinGW proper does not.


Hmm. The MinGW-w64 header does this:


#if !defined(NO_OLDNAMES) && !defined(popen)
#define popen _popen
#define pclose _pclose
#endif


So if we defined popen() before including stdio.h, that would get rid of 
the warning. But we don't usually do things in that order.


Could we define NO_OLDNAMES? I couldn't find any documentation on it, 
but it seems to a bunch of lot of wrapper functions and defines. If we 
can get away without them, that seems like a good thing...


- Heikki


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] btree_gist valgrind warnings about uninitialized memory

2014-05-14 Thread Heikki Linnakangas

On 05/13/2014 05:13 PM, Andres Freund wrote:

That leaves the spgist thing and some infrastructure till we can setup a
valgrind animal... From what we've caught with it so far that seems
rather worthwile.
What's your plans with your spgist fix? Commit it once 9.5 is branched?


Good question. I don't know. I would still like to commit it to 9.4. It 
doesn't require catalog changes, but it's an incompatible change in the 
WAL record format. If we commit it to 9.4, it means that you cannot 
replicate between 9.4beta1 and 9.4beta2. I think that's OK, but how do 
others feel about that?


- Heikki


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Sending out a request for more buildfarm animals?

2014-05-14 Thread Andres Freund
On 2014-05-13 20:42:16 +0200, Tomas Vondra wrote:
> Can someone please approve the animals I've requested a few days ago?
> I'm already running the clobber tests with '--nosend --nostatus' and
> it's already reporting some errors. Would be nice to get it to the
> buildfarm.

Can you provide some details about those failures until then?

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] buildfarm / handling (undefined) locales

2014-05-14 Thread Heikki Linnakangas

On 05/14/2014 12:07 AM, Tom Lane wrote:

Andrew Dunstan  writes:

On 05/13/2014 04:14 PM, Tom Lane wrote:

But independently of whether it's a fatal error or not: when there's
no relevant command-line argument then we print the

invalid locale name ""

message which is surely pretty unhelpful.  It'd be better if we could
finger the incorrect environment setting.  Unfortunately, we don't know
for sure which environment variable(s) setlocale was looking at --- I
believe it's somewhat platform specific.  We could probably print
something like this instead:

environment locale settings are invalid

Thoughts?



I'd also be tempted to add the settings for LC_ALL and LANG and note
that they are possible sources of the problem, or maybe only do that if
they match the locale being rejected.


Well, all we know is that we asked setlocale for the default locale from
the environment and it failed.  We don't really know which variable(s)
it looked at.  I think printing specific variables could easily be as
misleading as it is helpful; and given the lack of prior complaints,
I'm doubtful that it's worth going to the effort of trying to print a
platform-specific collection of variables.


Printing the values of the environment variables sounds complicated, but 
I think a generic hint along these lines would be good:


$ LANG=foo initdb
The files belonging to this database system will be owned by user 
"postgres".

This user must also own the server process.

initdb: environment locale settings are invalid
HINT: This usually means that the LANG, LC_ALL or LC_* environment 
variable has an invalid value.


Specifically mentioning those environment variables by name would be 
very helpful to the user. They are specified by POSIX: 
http://pubs.opengroup.org/onlinepubs/7908799/xbd/envvar.html, so they're 
not that platform-specific. Windows is a different story, though.


- Heikki


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] gettimeofday is at the end of its usefulness?

2014-05-14 Thread Robert Haas
On Tue, May 13, 2014 at 11:34 PM, Greg Stark  wrote:
> I always assumed the kernel used rdtsc to implement some of the high
> performance timers. It can save the current time in a mapped page when
> it schedules a process and then in the vdso syscall (ie in user-space)
> it can use rdtsc to calculate the offset needed to adjust that
> timestamp to the current time. This seems consistent with your
> calculations that showed the 40ns overhead with +/- 10ns precision.

Crazy idea: Instead of trying to time precisely the amount of time we
spend in each node, configure a very-high frequency timer interrupt
(or background thread?) that does:

SomeGlobalVariablePointingToTheCurrentNode->profiling_counter++;

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] gettimeofday is at the end of its usefulness?

2014-05-14 Thread Ants Aasma
On Wed, May 14, 2014 at 6:34 AM, Greg Stark  wrote:
> I always assumed the kernel used rdtsc to implement some of the high
> performance timers. It can save the current time in a mapped page when
> it schedules a process and then in the vdso syscall (ie in user-space)
> it can use rdtsc to calculate the offset needed to adjust that
> timestamp to the current time. This seems consistent with your
> calculations that showed the 40ns overhead with +/- 10ns precision.

Both gettimeofday and clock_gettime do exactly that. [1]
clock_gettime(CLOCK_MONOTONIC) is the mode of operation we would want
to use here.

> I actually think it would be more interesting if we could measure the
> overhead and adjust for it. I don't think people are really concerned
> with how long EXPLAIN ANALYZE takes to run if they could get accurate
> numbers out of it.

Measuring would also be a good idea so we can automatically turn on
performance counters like IO timing when we know it's not obscenely
expensive.

However, subtracting the overhead will still skew the numbers somewhat
by giving more breathing time for memory and IO prefetching
mechanisms. Another option to consider would be to add a sampling
based mechanism for low overhead time attribution. It would be even
better if we could distinguish between time spent waiting on locks vs.
waiting on IO vs. waiting to be scheduled vs. actually executing.

[1] 
https://github.com/torvalds/linux/blob/master/arch/x86/vdso/vclock_gettime.c#L223

Regards,
Ants Aasma
-- 
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Error in running DBT2

2014-05-14 Thread Rohit Goyal
Hi Peter,

I tried the solution suggested by you. Please problem still persists.

I run the the *dbt2-pgsql-build-db -w 1 *

but, after some time, I faced this error

*/home/abhi/dbt2_install/bin/dbt2-pgsql-load-stored-procs: 45: [: c:
unexpected operator*
*/home/abhi/dbt2_install/bin/dbt2-pgsql-load-stored-procs: 53: [: c:
unexpected operator*
*unknown stored function type: c*

and script ends!!

moreover, I see that DB has been created and also 9 tables are there in
Database dbt2. Please suggest how to proceed.

Regards,
rohit Goyal


On Tue, May 13, 2014 at 9:44 PM, Peter Geoghegan  wrote:

>
> On Tue, May 13, 2014 at 12:36 PM, Rohit Goyal  wrote:
>
>> This pattern the above found many times. Please guide me through!!!
>>
>
> IIRC, people have been working around this by setting
> standard_conforming_strings to "off". It really ought to be fixed in a
> principled way, though -- the real issue here is that dbt2 has severe
> bit-rot.
>
> --
> Peter Geoghegan
>



-- 
Regards,
Rohit Goyal


Re: [HACKERS] 9.4 release notes

2014-05-14 Thread Dean Rasheed
On 4 May 2014 13:46, Bruce Momjian  wrote:
> I have completed the initial version of the 9.4 release notes.  You can
> view them here:
>
> http://www.postgresql.org/docs/devel/static/release-9-4.html
>
> I will be adding additional markup in the next few days.
>
> Feedback expected and welcomed.  I expect to be modifying this until we
> release 9.4 final.  I have marked items where I need help with question
> marks.
>

In a few places, I think "updateable" should be spelled "updatable"
for consistency with the rest of the documentation (although I think
both spellings are actually correct).


===
Allow the updating of views where only some columns are
auto-updateable (Dean Rasheed)

Previously the presence of a non-auto-updateable column prevented all
columns from being auto-updated. Deletes are now supported on suitable
views even if no auto-updateable columns are present.
===

I think that puts too much emphasis on deletes, and could be
misinterpreted. How about something like this:

Allow views to be automatically updatable even if they contain some
non-updatable columns (Dean Rasheed)

Previously the presence of non-updatable columns such as expressions,
literals and functions prevented automatic updates.  Now INSERTs,
UPDATEs and DELETEs are supported, provided that they do not attempt
to assign new values to any of the non-updatable columns.

Regards,
Dean


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers