Re: [HACKERS] Another swing at JSON

[Bernd Helmle]

--On 16. Juni 2011 17:38:07 -0400 Tom Lane  wrote:


After reading Joseph's comment upthread, I don't see any consensus
wether the existing pre-9.1 support is required or even desired. Maybe
i missed it, but do we really expect an extension (or contrib module)
to be backwards compatible to earlier major releases, when shipped in
contrib/ ?


No, we don't.  You won't find any attempt in any contrib module to build
against prior releases.  There's not much point, since they're shipped
with a specific release of the core.


Okay, then we should remove this code. It doesn't do any complicated, but it 
seems a waste of code in this case (and from a maintenance point of view).


Joseph, are you able to remove the compatibility code for this CF?

--
Thanks

Bernd

--
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] procpid?

[Magnus Hagander]

On Fri, Jun 17, 2011 at 06:39, Greg Smith  wrote:
> On 06/16/2011 05:27 PM, Bruce Momjian wrote:
>>
>> Greg Smith wrote:
>>
>>>
>>> -It is still useful to set current_query to descriptive text in the
>>> cases where the transaction is  etc.
>>>
>>
>> Uh, if we are going to do that, why not just add the boolean columns to
>> the existing view?  Clearly renaming procpid isn't worth creating
>> another view.
>>
>
> I'm not completely set on this either way; that's why I suggested a study
> that digs into typical monitoring system queries would be useful.  Even the
> current view is pushing the limits for how much you can put into something
> that intends to be human-readable though.  Adding a new pile of columns to
> it has some downsides there.

Is it intended for human-readable? And for human readable without
specifying which part you want? It's already way too wide to fit in
most terminals - and has been for years. You need to use \x unless you
specify the fields.

And if you want a "simpler version", why not just add all the columns
to the existing one we need, and then create a regular VIEW over it
that shows just the most common columns? But I still think you're
going to find a hard time making even that narrow enough to  be easily
consumable - but you could certainly remove things like usesysid and
datid which are mainly useful only for JOINing to other stuff.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] SSI work for 9.1

[Dan Ports]

On Fri, Jun 17, 2011 at 12:32:46AM -0400, Robert Haas wrote:
> Perhaps it would be best to remove the general item and replace it
> with a list of more specific things that need doing - which might just
> mean #5.

Done.

-- 
Dan R. K. Ports  MIT CSAILhttp://drkp.net/

-- 
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] procpid?

[Greg Smith]

On 06/16/2011 05:27 PM, Bruce Momjian wrote:

Greg Smith wrote:
   

-It is still useful to set current_query to descriptive text in the
cases where the transaction is  etc.
 

Uh, if we are going to do that, why not just add the boolean columns to
the existing view?  Clearly renaming procpid isn't worth creating
another view.
   


I'm not completely set on this either way; that's why I suggested a 
study that digs into typical monitoring system queries would be useful.  
Even the current view is pushing the limits for how much you can put 
into something that intends to be human-readable though.  Adding a new 
pile of columns to it has some downsides there.


I hadn't ever tried to write down everything I'd like to see changed 
here until this week, so there may be further column churn that 
justifies a new view too.  I think the whole idea needs to get chewed on 
a bit more.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us



--
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] SSI work for 9.1

[Robert Haas]

On Fri, Jun 17, 2011 at 12:30 AM, Dan Ports  wrote:
> On Thu, Jun 16, 2011 at 11:49:48PM -0400, Robert Haas wrote:
>> Does this mean that the open item "more SSI loose ends" can now be
>> marked resolved?
>
> I was just looking at it and contemplating moving it to the non-blockers
> list. Of the five items:
>  - (1) and (4) are resolved
>  - (2) isn't an issue -- maybe we want to add a comment, someplace but
>   I'm not convinced even that is necessary
>  - (3) is a regression test, and is already on the list separately
>  - (5) is a doc issue only
>
> There are no open issues with the code that I'm aware of.

Perhaps it would be best to remove the general item and replace it
with a list of more specific things that need doing - which might just
mean #5.

-- 
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] SSI work for 9.1

[Dan Ports]

On Thu, Jun 16, 2011 at 11:49:48PM -0400, Robert Haas wrote:
> Does this mean that the open item "more SSI loose ends" can now be
> marked resolved?

I was just looking at it and contemplating moving it to the non-blockers
list. Of the five items:
 - (1) and (4) are resolved
 - (2) isn't an issue -- maybe we want to add a comment, someplace but
   I'm not convinced even that is necessary
 - (3) is a regression test, and is already on the list separately
 - (5) is a doc issue only

There are no open issues with the code that I'm aware of.

Dan

-- 
Dan R. K. Ports  MIT CSAILhttp://drkp.net/

-- 
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] pg_upgrade using appname to lock out other users

[Bruce Momjian]

Robert Haas wrote:
> On Thu, Jun 16, 2011 at 11:47 PM, Bruce Momjian  wrote:
> > Robert Haas wrote:
> >> > We can pick different options for 9.0, 9.1, and 9.2. ?(For PG 9.0
> >> > probably only #1 is appropriate.)
> >>
> >> I don't like any of these options as well as what I already proposed.
> >> I proposed a complicated approach that actually fixes the problem for
> >> real; you're proposing a whole bunch of simpler approaches all of
> >> which have pretty obvious holes. ?We already have something that only
> >> sorta works; replacing it with a different system that only sorta
> >> works is not going to be a great leap forward.
> >
> > What is your proposal? ?Write a password into a file that is read by the
> > postmaster on startup and used for connections? ?That would remove the
> > "modify pg_hba.conf to 'trust'" step, but again only for new servers.
> 
> Yeah, as noted upthread, I'd probably create a binary_upgrade.conf
> that works like recovery.conf, if it were me.

Well, I know exactly where the data directories are.  We will still have
a problem for anyone upgrading from pre-9.2.

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

  + It's impossible for everything to be true. +

-- 
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] pg_upgrade using appname to lock out other users

[Robert Haas]

On Thu, Jun 16, 2011 at 11:47 PM, Bruce Momjian  wrote:
> Robert Haas wrote:
>> > We can pick different options for 9.0, 9.1, and 9.2. ?(For PG 9.0
>> > probably only #1 is appropriate.)
>>
>> I don't like any of these options as well as what I already proposed.
>> I proposed a complicated approach that actually fixes the problem for
>> real; you're proposing a whole bunch of simpler approaches all of
>> which have pretty obvious holes.  We already have something that only
>> sorta works; replacing it with a different system that only sorta
>> works is not going to be a great leap forward.
>
> What is your proposal?  Write a password into a file that is read by the
> postmaster on startup and used for connections?  That would remove the
> "modify pg_hba.conf to 'trust'" step, but again only for new servers.

Yeah, as noted upthread, I'd probably create a binary_upgrade.conf
that works like recovery.conf, if it were me.

-- 
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] SSI work for 9.1

[Robert Haas]

On Wed, Jun 15, 2011 at 5:10 AM, Heikki Linnakangas
 wrote:
> On 14.06.2011 17:57, Kevin Grittner wrote:
>>
>> Heikki Linnakangas  wrote:
>>
>>> I did some further changes, refactoring SkipSerialization so that
>>> it's hopefully more readable, and added a comment about the
>>> side-effects. See attached. Let me know if I'm missing something.
>>
>> I do think the changes improve readability.  I don't see anything
>> missing, but there's something we can drop.  Now that you've split
>> the read and write tests, this part can be dropped from the
>> SerializationNeededForWrite function:
>>
>> +
>> +       /* Check if we have just become "RO-safe". */
>> +       if (SxactIsROSafe(MySerializableXact))
>> +       {
>> +               ReleasePredicateLocks(false);
>> +               return false;
>> +       }
>>
>> If it's doing a write, it can't be a read-only transaction
>
> Ah, good point. Committed with that removed.

Does this mean that the open item "more SSI loose ends" can now be
marked resolved?

http://wiki.postgresql.org/wiki/PostgreSQL_9.1_Open_Items

-- 
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] pg_upgrade using appname to lock out other users

[Bruce Momjian]

Robert Haas wrote:
> > We can pick different options for 9.0, 9.1, and 9.2. ?(For PG 9.0
> > probably only #1 is appropriate.)
> 
> I don't like any of these options as well as what I already proposed.
> I proposed a complicated approach that actually fixes the problem for
> real; you're proposing a whole bunch of simpler approaches all of
> which have pretty obvious holes.  We already have something that only
> sorta works; replacing it with a different system that only sorta
> works is not going to be a great leap forward.

What is your proposal?  Write a password into a file that is read by the
postmaster on startup and used for connections?  That would remove the
"modify pg_hba.conf to 'trust'" step, but again only for new servers.

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

  + It's impossible for everything to be true. +

-- 
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] [GENERAL] Issues with generate_series using integer boundaries

[Robert Haas]

On Wed, Feb 9, 2011 at 4:50 AM, Thom Brown  wrote:
> On 9 February 2011 02:11, Robert Haas  wrote:
>> On Tue, Feb 8, 2011 at 8:30 PM, Andrew Dunstan  wrote:
>>> Quite right, but the commitfest manager isn't meant to be a substitute for
>>> one. Bug fixes aren't subject to the same restrictions of feature changes.
>>
>> Another option would be to add this here:
>>
>> http://wiki.postgresql.org/wiki/PostgreSQL_9.1_Open_Items
>
> I've removed it from the commitfest because it really doesn't belong
> there, and I've added it to the open items list.

So, I finally got around to look at this, and I think there is a
simpler solution.  When an overflow occurs while calculating the next
value, that just means that the value we're about to return is the
last one that should be generated.  So we just need to frob the
context state so that the next call will decide we're done.  There are
any of number of ways to do that; I just picked what looked like the
easiest one.

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


generate-series-overflow.patch
Description: Binary data

-- 
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] crash-safe visibility map, take five

[Noah Misch]

Robert,

I took a look at this patch.  To summarize its purpose as I understand it, it
does two independent but synergistic things:

1. Move the INSERT/UPDATE/DELETE clearing of visibility map bits from after
the main operation to before it.  This has no affect on crash recovery.  It
closes a race condition described in the last paragraphs of visibilitymap.c's
header comment.

2. In the words of a comment added by the patch:
 * The critical integrity requirement here is that we must never end up with
 * a situation where the visibility map bit is set, and the page-level
 * PD_ALL_VISIBLE bit is clear.  If that were to occur, then a subsequent
 * page modification would fail to clear the visibility map bit.
It does this by WAL-logging the operation of setting a vm bit.  This also has
the benefit of getting vm bits set correctly on standbys.

On Mon, May 23, 2011 at 03:54:44PM -0400, Robert Haas wrote:
> On Wed, May 11, 2011 at 8:46 AM, Pavan Deolasee
>  wrote:
> > Why do the set and clear functions need pass-by-reference (Buffer *)
> > argument ? I don't see them modifying the argument at all. This patch adds
> > the clear function, but the existing set function also suffers from that.
> 
> Yep, I just copied the style of the existing function, in the interest
> of changing no more than necessary.  But maybe it'd be better to
> change them both to take just the Buffer, instead of a pointer.
> Anyone else want to weigh in?

+1 for Buffer over Buffer * when the value isn't mutated for the caller.


I suggest revisiting the suggestion in
http://archives.postgresql.org/message-id/27743.1291135...@sss.pgh.pa.us and
including a latestRemovedXid in xl_heap_visible.  The range of risky xids is
the same for setting a visibility map bit as for deleting a dead tuple, and
the same operation (VACUUM) produces both conflicts.  The system that has
fueled my reports related to standby conflict would hit a snapshot conflict
more or less instantly without vacuum_defer_cleanup_age.  (With 9.1, we'll use
hot_standby_feedback and never look back.)  Adding visibility map bits as a
source of conflict costs nothing to a system like that.  You might not like it
on an INSERT-only system whose VACUUMs only update visibility.  Such systems
might like a GUC on the standby that disables both index-only scans and
conflict on xl_heap_visible.lastestRemovedXid.  For normal read/write systems,
there will be no advantage.

The patch follows a design hashed in Nov-Dec 2010.  Currently, it adds
overhead to VACUUM with thin practical benefit.  However, there's broad
community acceptance that this is a step on a deterministic path to success at
implementing index-only scans, for worthy benefit.


lazy_scan_heap() has two calls to visibilitymap_set(), but the patch only
changed the recptr argument for one of them.  This has the effect that we only
emit WAL for empty pages and pages that happened to have pd_lsn == {0,0}, such
as those not modified since the transaction creating the table.  I fixed this
before testing further.

Next, for no especially good reason, I set a breakpoint on the closing brace
of visibilitymap_set, ran a VACUUM that called it, and instigated a PANIC from
gdb.  Recovery failed like this:

3614 2011-06-16 20:05:18.118 EDT LOG:  0: redo starts at 0/172ABA0
3614 2011-06-16 20:05:18.118 EDT LOCATION:  StartupXLOG, xlog.c:6494
3614 2011-06-16 20:05:18.119 EDT FATAL:  XX000: lock 148 is not held
3614 2011-06-16 20:05:18.119 EDT CONTEXT:  xlog redo visible: rel 
1663/16384/24588; blk 0
3614 2011-06-16 20:05:18.119 EDT LOCATION:  LWLockRelease, lwlock.c:587
TRAP: FailedAssertion("!(PrivateRefCount[i] == 0)", File: "bufmgr.c", Line: 
1695)
3613 2011-06-16 20:05:18.119 EDT LOG:  0: startup process (PID 3614) was 
terminated by signal 6: Aborted
3613 2011-06-16 20:05:18.119 EDT LOCATION:  LogChildExit, postmaster.c:2881
3613 2011-06-16 20:05:18.119 EDT LOG:  0: aborting startup due to startup 
process failure
3613 2011-06-16 20:05:18.119 EDT LOCATION:  reaper, postmaster.c:2376

This happens due to heap_xlog_redo() calling UnlockReleaseBuffer() despite
having taken no buffer content lock.  I added
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
before the "if" to get past this.

I proceeded to do some immediate-shutdown tests to see if we get the bits set
as expected.  I set up a database like this:
CREATE EXTENSION pageinspect;
CREATE TABLE t (c int);
INSERT INTO t VALUES (2);
CHECKPOINT;
I normally cleared bits with "UPDATE t SET c = 1; CHECKPOINT;" and set them
with "VACUUM t".  I checked bits with this query:
SELECT to_hex(get_byte(get_raw_page('t', 'vm', 0), 24)),
   to_hex(flags::int) FROM page_header(get_raw_page('t', 0));
The row from that query should generally be 0,1 (bits unset) or 1,5 (bits
set).  0,5 is fine after a crash.  1,1 means we've broken our contract: the VM
bit is set while PD_ALL_VISIBLE is not set.

First test was to clear bits, checkpoint, th

Re: [HACKERS] pg_upgrade using appname to lock out other users

[Robert Haas]

On Thu, Jun 16, 2011 at 5:16 PM, Bruce Momjian  wrote:
> Robert Haas wrote:
>> On Wed, Jun 15, 2011 at 1:35 PM, Bruce Momjian  wrote:
>> > I now believe we are overthinking all this. ?pg_upgrade has always
>> > supported specification of a port number. ?Why not just tell users to
>> > specify an unused port number > 1023, and not to use the default value?
>>
>> 1. Because it shouldn't be the user's problem to figure out a good
>> choice of port number.
>>
>> 2. Because we also really ought to be ignoring the contents of
>> pg_hba.conf during an upgrade, and instead have some mechanism that
>> allows pg_upgrade to be sure of getting in (without creating a
>> security hole in the process).
>>
>> I agree that back-patching these changes wouldn't be a wonderful
>> thing, but we are going to do a lot more releases that have pg_upgrade
>> in them in the future than we've already done in the past.  It's not a
>> bad thing to try to start improving on the basic mechanism, even if
>> takes a while for versions that support that mechanism to become
>> commonplace.  Limiting what we're willing to do the server to improve
>> the pg_upgrade experience in the future to what we're willing to
>> back-patch is not going to be a winning strategy.
>
> OK, well, we have three possible directions:
>
> 1.  Just document that people should use -p and -P on unused ports to
> avoid user connections
>
> 2.  Do #1 and also require -p and -P to be used (no defaults)
>
> 3.  Have pg_upgrade default to use a typically unused port number, e.g.
> 25432 (on Unix, it might only be using unix domain sockets anyway)
>
> 4.  Require appname to match 'binary-upgrade' when in -b (binary-upgrade)
> mode
>
> 5.  Disable TCP when in -b mode on Unix (not possible on Windows)
>
> We can pick different options for 9.0, 9.1, and 9.2.  (For PG 9.0
> probably only #1 is appropriate.)

I don't like any of these options as well as what I already proposed.
I proposed a complicated approach that actually fixes the problem for
real; you're proposing a whole bunch of simpler approaches all of
which have pretty obvious holes.  We already have something that only
sorta works; replacing it with a different system that only sorta
works is not going to be a great leap forward.

-- 
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] time-delayed standbys

[Robert Haas]

On Thu, Jun 16, 2011 at 10:10 PM, Fujii Masao  wrote:
>>> According to the above page, one purpose of time-delayed replication is to
>>> protect against user mistakes on master. But, when an user notices his wrong
>>> operation on master, what should he do next? The WAL records of his wrong
>>> operation might have already arrived at the standby, so neither "promote" 
>>> nor
>>> "restart" doesn't cancel that wrong operation. Instead, probably he should
>>> shutdown the standby, investigate the timestamp of XID of the operation
>>> he'd like to cancel, set recovery_target_time and restart the standby.
>>> Something like this procedures should be documented? Or, we should
>>> implement new "promote" mode which finishes a recovery as soon as
>>> "promote" is requested (i.e., not replay all the available WAL records)?
>>
>> I like the idea of a new promote mode;
>
> Are you going to implement that mode in this CF? or next one?

I wasn't really planning on it - I thought you might want to take a
crack at it.  The feature is usable without that, just maybe a bit
less cool.  Certainly, it's too late for any more formal submissions
to this CF, but I wouldn't mind reviewing a patch if you want to write
one.

-- 
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] ALTER TABLE lock strength reduction patch is unsafe

[Robert Haas]

On Thu, Jun 16, 2011 at 6:54 PM, Tom Lane  wrote:
> I believe that this is fundamentally unavoidable so long as we use
> SnapshotNow to read catalogs --- which is something we've talked about
> changing, but it will require a pretty major R&D effort to make it
> happen.  In the meantime, we have to go back to using
> AccessExclusiveLock for table alterations.  It doesn't help to have
> a lower lock level if that means that concurrent transactions will
> unpredictably fail instead of waiting.

Ouch.

I wonder if we could avoid this anomaly by taking a throwaway MVCC
snapshot at the beginning of each system catalog scan and using it
just for the duration of that scan.  If nothing that has touched the
catalog commits while the scan is open, then this is logically
equivalent to SnapshotNow.  If something does commit in mid-scan, then
we might not get the latest version of the row, but we should end up
with exactly one.  If it's not the latest one, we'll do the rebuild
again upon seeing the next sinval message; in the meantime, the
version we're using mustn't be too intolerably bad or it was an error
not to use AccessExclusiveLock in the first place.

IIUC, the problem with this approach is not correctness but
performance.  Taking snapshots is (currently) expensive.

-- 
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] time-delayed standbys

[Fujii Masao]

On Fri, Jun 17, 2011 at 3:29 AM, Robert Haas  wrote:
> Even if that were not an issue, I'm still more or less of the opinion
> that trying to solve the time synchronization problem is a rathole
> anyway.  To really solve this problem well, you're going to need the
> standby to send a message containing a timestamp, get a reply back
> from the master that contains that timestamp and a master timestamp,
> and then compute based on those two timestamps plus the reply
> timestamp the maximum and minimum possible lag between the two
> machines.  Then you're going to need to guess, based on several cycles
> of this activity, what the actual lag is, and adjust it over time (but
> not too quckly, unless of course a large manual step has occurred) as
> the clocks potentially drift apart from each other.  This is basically
> what ntpd does, except that it can be virtually guaranteed that our
> implementation will suck by comparison.  Time synchronization is
> neither easy nor our core competency, and I think trying to include it
> in this feature is going to result in a net loss of reliability.

Agreed. You've already added the note about time synchronization into
the document. That's enough, I think.

>>> errmsg("parameter \"%s\" requires a temporal value", "recovery_time_delay"),
>>
>> We should s/"a temporal"/"an Integer"?
>
> It seems strange to ask for an integer when what we want is an amount
> of time in seconds or minutes...

OK.

>> http://forge.mysql.com/worklog/task.php?id=344
>> According to the above page, one purpose of time-delayed replication is to
>> protect against user mistakes on master. But, when an user notices his wrong
>> operation on master, what should he do next? The WAL records of his wrong
>> operation might have already arrived at the standby, so neither "promote" nor
>> "restart" doesn't cancel that wrong operation. Instead, probably he should
>> shutdown the standby, investigate the timestamp of XID of the operation
>> he'd like to cancel, set recovery_target_time and restart the standby.
>> Something like this procedures should be documented? Or, we should
>> implement new "promote" mode which finishes a recovery as soon as
>> "promote" is requested (i.e., not replay all the available WAL records)?
>
> I like the idea of a new promote mode;

Are you going to implement that mode in this CF? or next one?

> and documenting the other
> approach you mention doesn't sound bad either.  Either one sounds like
> a job for a separate patch, though.
>
> The other option is to pause recovery and run pg_dump...

Yes, please.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] planinstr, showing planner time on EXPLAIN

[Robert Haas]

On Thu, Jun 16, 2011 at 9:10 PM, Hitoshi Harada  wrote:
> 2011/6/17 Robert Haas :
>> On Tue, Jun 14, 2011 at 11:18 PM, Hitoshi Harada  
>> wrote:
>>> Yesterday on PGXN I just released the first version of planinstr, a
>>> plugin module to append planner time to EXPLAIN. I post this here
>>> since it is mostly for developers.
>>
>> Wow, that is awesome.  I sorta wish we had something like that in core
>> -- and I don't even think it should be optional, I think it should
>> just always give you that additional piece of information.
>
> Thanks for a positive feedback. Current design passes instrument time
> as global variable from planner to executor. To get it in core we need
> to add a field to the planner for that, which I don't think is too
> difficult. Anyway I still like its way as it doesn't need any change
> in core; free for anyone to use, even in the older versions.

Yeah, definitely a nice tool.

-- 
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] Re: patch review : Add ability to constrain backend temporary file space

[Mark Kirkwood]

On 17/06/11 13:08, Mark Kirkwood wrote:

On 17/06/11 09:49, Cédric Villemain wrote:


I have issues applying it.
Please can you remove trailing space?
Also, you can generate a cool patch like this :

get git-external-diff from postgres/src/tools to /usr/lib/git-core/
chmod +x it
export GIT_EXTERNAL_DIFF=git-external-diff
git format-patch --ext-diff origin


I think I have the trailing spaces removed, and patch is updated for the 
variable renaming recently done in fd.c


I have no idea why I can't get the git apply to work (obviously I have 
exceeded by git foo by quite a ways), but it should apply for you I hope 
(as it patches fine).


regards

Mark


temp-files-v5.1.patch.gz
Description: GNU Zip compressed data

-- 
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] Boolean operators without commutators vs. ALL/ANY

[Alvaro Herrera]

Excerpts from Tom Lane's message of jue jun 16 17:33:17 -0400 2011:
> Peter Eisentraut  writes:
> > I don't really agree that visual correlation needs to trump everything.
> > If say
> > foo =~ bar
> > and
> > foo ~= bar
> > were to produce completely different results, this would introduce bugs
> > all over the place.
> 
> Huh?  That's about like arguing that standard mathematical notation is
> broken because a < b and a > b don't produce the same result.

The difference is that the mnemonic for > and < is very simple and in
widespread knowledge; not something I would say for =~'s rule of "the ~
is on the side of the regexp".  I know I used to get it wrong in Perl
(i.e. I wrote ~= occasionally).
To make matters worse, our delimiters for regexes are the same as for
strings, the single quote.  So you get

foo =~ 'bar'/* foo is the text column, bar is the regex */
'bar' =~ foo/* no complaint but it's wrong */

'bar' ~= foo/* okay */
'foo' ~= bar/* no complaint but it's wrong */

How do I tell which is the regex here?  If we used, say, /, that would
be a different matter:

foo =~ /bar/
/bar/ ~= foo/* both okay */

If we had that and you get it wrong, the parser would immediately barf
at you if you got it wrong:

/bar/ =~ foo/* wrong: LHS wanted text, got regex */
foo ~= /bar//* wrong: LHS wanted regex, got text */

(Note: I'm not suggesting we use / as delimiter.  This is just an
example.)

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] planinstr, showing planner time on EXPLAIN

[Hitoshi Harada]

2011/6/17 Robert Haas :
> On Tue, Jun 14, 2011 at 11:18 PM, Hitoshi Harada  wrote:
>> Yesterday on PGXN I just released the first version of planinstr, a
>> plugin module to append planner time to EXPLAIN. I post this here
>> since it is mostly for developers.
>
> Wow, that is awesome.  I sorta wish we had something like that in core
> -- and I don't even think it should be optional, I think it should
> just always give you that additional piece of information.

Thanks for a positive feedback. Current design passes instrument time
as global variable from planner to executor. To get it in core we need
to add a field to the planner for that, which I don't think is too
difficult. Anyway I still like its way as it doesn't need any change
in core; free for anyone to use, even in the older versions.

Regards,


-- 
Hitoshi Harada

-- 
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] Re: patch review : Add ability to constrain backend temporary file space

[Mark Kirkwood]

On 17/06/11 09:49, Cédric Villemain wrote:


I have issues applying it.
Please can you remove trailing space?
Also, you can generate a cool patch like this :

get git-external-diff from postgres/src/tools to /usr/lib/git-core/
chmod +x it
export GIT_EXTERNAL_DIFF=git-external-diff
git format-patch --ext-diff origin


my log:
$ git apply  temp-files-v5.patch
temp-files-v5.patch:22: trailing whitespace.
 defaults to unlimited (-1). Values larger than zero
temp-files-v5.patch:23: trailing whitespace.
 constraint the temporary file space usage to be that number of
temp-files-v5.patch:28: trailing whitespace.
 the total space used by all the files produced by one backend is
temp-files-v5.patch:35: trailing whitespace.
 constrain disk space used for temporary table storage. However if
temp-files-v5.patch:105: trailing whitespace.
 /*
error: patch failed: src/backend/storage/file/fd.c:132
error: src/backend/storage/file/fd.c: patch does not apply
error: src/test/regress/expected/resource.out: No such file or directory
error: src/test/regress/sql/resource.sql: No such file or directory



I can generate a patch that way no problem - however it does not apply 
using the above method:


$ git apply  /data0/download/postgres/patches/temp/temp-files-v5.1.patch
error: doc/src/sgml/config.sgml: already exists in working directory
error: src/backend/storage/file/fd.c: already exists in working directory
error: src/backend/utils/misc/guc.c: already exists in working directory
error: src/backend/utils/misc/postgresql.conf.sample: already exists in 
working directory

error: src/include/utils/guc.h: already exists in working directory
error: src/include/utils/guc_tables.h: already exists in working directory
error: src/test/regress/serial_schedule: already exists in working directory

However it applies just fine using the "normal" method:

$ patch --dry-run -p1 < 
/data0/download/postgres/patches/temp/temp-files-v5.1.patch

patching file doc/src/sgml/config.sgml
patching file src/backend/storage/file/fd.c
patching file src/backend/utils/misc/guc.c
patching file src/backend/utils/misc/postgresql.conf.sample
patching file src/include/utils/guc.h
patching file src/include/utils/guc_tables.h
patching file src/test/regress/expected/resource.out
patching file src/test/regress/serial_schedule
patching file src/test/regress/sql/resource.sql

Any idea?

regards

Mark





--
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] per-column generic option

[David Fetter]

On Tue, Jun 14, 2011 at 05:56:05PM +0900, Shigeru Hanada wrote:
> Hi,
> 
> I would like to propose support for per-column generic option, which
> is defined in the SQL/MED standard.  In 9.0 release, support for
> foreign tables and per-table generic option have been added, but
> support for per-column generic option hasn't.
> 
> Please examine the description below and attached patch
> per_column_option_v1.patch.  Any comments or questions are welcome.

Sorry not to respond sooner.

First, the per-column generic options are a great thing for us to
have. :)

I have an idea I've been using for the next release of DBI-Link that
has varying levels of data type mapping.  In general, these mappings
would be units of executable code, one in-bound, and one out-bound,
for each of:

Universe (everything, default "mapping" is the identity map, i.e. a no-op)
Database type (e.g. MySQL)
Instance (e.g. mysql://foo.bar.com:5432)
Database
Schema
Table
Column

I didn't include row in the hierarchy because I couldn't think of a
way to identify rows across DBMSs and stable over time.

The finest-grain transformation that's been set would be the one
actually used.

Here's an example of a non-trivial mapping.

Database type:
MySQL
Foreign data type:
datetime
PostgreSQL data type:
timestamptz
Transformation direction:
Import
Transformation:
CASE
WHEN DATA = '-00-00 00:00:00'
THEN NULL
ELSE DATA
END

Here, I'm making the simplifying assumption that there is a bijective
mapping between data types.

Is there some way to fit the per-column part of such a mapping into
this scheme?  We'd need to do some dependency tracking in order to be
able to point to the appropriate code...

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] ALTER TABLE lock strength reduction patch is unsafe

[Simon Riggs]

On Thu, Jun 16, 2011 at 11:54 PM, Tom Lane  wrote:

> In typical cases where both versions of the row are on the same page,
> the window for the concurrent commit to happen is very narrow --- that's
> why you need so many clients to make it happen easily.  With enough
> clients there's a good chance of losing the CPU between tuple visits.
> But of course Murphy's Law says this will happen in production
> situations even if the load isn't so high.

Thanks for doing the research on this. Much better we know now than
enter production like this.

> I believe that this is fundamentally unavoidable so long as we use
> SnapshotNow to read catalogs --- which is something we've talked about
> changing, but it will require a pretty major R&D effort to make it
> happen.  In the meantime, we have to go back to using
> AccessExclusiveLock for table alterations.  It doesn't help to have
> a lower lock level if that means that concurrent transactions will
> unpredictably fail instead of waiting.

If we were to change ALTER TABLE so it takes a session lock rather
than a normal lock, then we can commit the change and then wait until
the relcache invalidations have been received before we release the
lock. That way we would be able to avoid the concurrent issues you
describe.

Or alternatively we could just re-scan if we can't find a valid row
when building the cache. We have time in the failure path...

Do you have stomach for any this in 9.1?

-- 
 Simon Riggs   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] flexible array members

[Brar Piening]

On Thu, 16 Jun 2011 22:49:45 +0300, Peter Eisentraut 
 wrote:

This macro is provided by Autoconf and it appears to be using the
standard's terminology.


commit dbbba5279f66f95805c1e084e6f646d174931e56 refs/heads/master
Author: Peter Eisentraut 
Date:   Thu Jun 16 22:39:09 2011 +0300

Periodically checking my VS2010 patch I noticed that this one broke 
Visual Studio builds.


At least mine and "chough 
" 
in the build farm - I expect others to follow once they rebuild.


error C2065: 'FLEXIBLE_ARRAY_MEMBER' : undeclared identifier
error C2057: expected constant expression

Regards,

Brar

Re: [HACKERS] Boolean operators without commutators vs. ALL/ANY

[Florian Pflug]

On Jun16, 2011, at 21:49 , Tom Lane wrote:
> All three of these are massive overkill.  What we need is a general
> policy that providing commutators is a good idea.  We do not need to try
> to make it 100.00% with an enforcement mechanism.

What parts of (1) do you think are overkill exactly, then?

> As for #2, what's
> your plan for automatically selecting a commutator operator name?

I figured we'd name it "COMMUTATOR " or something along this line.
That'd mean it'd only be useable with the OPERATOR() syntax, but that's
way better than nothing. Or we could even make the COMMUTATOR argument
mandatory for binary operators returning boolean. After all, if a 
commutator doesn't require a second function, than I fail to see why
you'd ever want to define a predicate without a commutator.

In any case, yeah, (2) is pretty hand-weavy. I included so that we'd
have all the options on the table, not because I think it's particularly
elegant, easy, or interesting to implement (actually, it's probably
none of these).

> (Having said that, I *was* thinking of adding an opr_sanity test ... but
> not expecting that we'd get it to find zero rows.)

Well, as long as there is some regression test failure for
missing commutators of newly added binary boolean operators, I'm
content.

best regards,
Florian Pflug




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

[HACKERS] ALTER TABLE lock strength reduction patch is unsafe

[Tom Lane]

If you set up a pgbench test case that hits the database with a lot of
concurrent selects and non-exclusive-locking ALTER TABLEs, 9.1 soon
falls over.  For example:

$ cat foo.script
alter table pgbench_accounts set (fillfactor = 100);
SELECT abalance FROM pgbench_accounts WHERE aid = 525212;

$ createdb bench
$ pgbench -i -s 10 bench
...
$ pgbench -c 50 -t 100 -f foo.script bench
starting vacuum...end.
Client 10 aborted in state 0: ERROR:  relation "pgbench_accounts" does not exist
Client 5 aborted in state 1: ERROR:  cache lookup failed for relation 46260
Client 44 aborted in state 0: ERROR:  relation "pgbench_accounts" does not exist
Client 3 aborted in state 1: ERROR:  relation "pgbench_accounts" does not exist
LINE 1: SELECT abalance FROM pgbench_accounts WHERE aid = 525212;
 ^
Client 45 aborted in state 1: ERROR:  could not open relation with OID 46260
LINE 1: SELECT abalance FROM pgbench_accounts WHERE aid = 525212;
 ^
Client 15 aborted in state 1: ERROR:  cache lookup failed for relation 46260
Client 34 aborted in state 1: ERROR:  could not open relation with OID 46260
LINE 1: SELECT abalance FROM pgbench_accounts WHERE aid = 525212;
 ^
Client 43 aborted in state 1: ERROR:  cache lookup failed for relation 46260
Client 49 aborted in state 1: ERROR:  relation "pgbench_accounts" does not exist
LINE 1: SELECT abalance FROM pgbench_accounts WHERE aid = 525212;
 ^
Client 12 aborted in state 0: ERROR:  relation "pgbench_accounts" does not exist
Client 23 aborted in state 0: ERROR:  relation "pgbench_accounts" does not exist
Client 14 aborted in state 0: ERROR:  relation "pgbench_accounts" does not exist
Client 6 aborted in state 1: ERROR:  could not open relation with OID 46260
LINE 1: SELECT abalance FROM pgbench_accounts WHERE aid = 525212;
 ^
Client 11 aborted in state 1: ERROR:  could not open relation with OID 46260
LINE 1: SELECT abalance FROM pgbench_accounts WHERE aid = 525212;
 ^
Client 4 aborted in state 0: ERROR:  relation "pgbench_accounts" does not exist
... etc etc ...

On my four-core workstation, the failures are infrequent at up to 30
clients but come pretty fast and furious at 50.

What is happening here is this:

1. Some backend commits an ALTER TABLE and sends out an sinval message.

2. In response, some other backend starts to reload its relcache entry
for pgbench_accounts when it begins its next command.  It does an
indexscan with SnapshotNow on pg_class to find the updated pg_class row.

3. Meanwhile, some third backend commits another ALTER TABLE, updating
the pg_class row another time.  Since we have removed the
AccessExclusiveLock that all variants of ALTER TABLE used to take, this
commit can happen while backend #2 is in process of scanning pg_class.

4. Backend #2 visits the new, about-to-be-committed version of
pgbench_accounts' pg_class row just before backend #3 commits.
It sees the row as not good and keeps scanning.  By the time it
reaches the previous version of the row, however, backend #3
*has* committed.  So that version isn't good according to SnapshotNow
either.

5. Thus, backend #2 fails to find any version of the pg_class row
that satisfies SnapshotNow, and it reports an error.  Depending on just
when this happens during the cache load process, you can get any of
the errors displayed above, or probably some other ones.

The particular case I'm showing here only updates pg_class, but other
non-exclusive-lock variants of ALTER TABLE can probably provoke similar
failures with respect to other catalogs, leading to yet different
misbehaviors.

In typical cases where both versions of the row are on the same page,
the window for the concurrent commit to happen is very narrow --- that's
why you need so many clients to make it happen easily.  With enough
clients there's a good chance of losing the CPU between tuple visits.
But of course Murphy's Law says this will happen in production
situations even if the load isn't so high.

I believe that this is fundamentally unavoidable so long as we use
SnapshotNow to read catalogs --- which is something we've talked about
changing, but it will require a pretty major R&D effort to make it
happen.  In the meantime, we have to go back to using
AccessExclusiveLock for table alterations.  It doesn't help to have
a lower lock level if that means that concurrent transactions will
unpredictably fail instead of waiting.

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] Re: patch review : Add ability to constrain backend temporary file space

[Cédric Villemain]

2011/6/15 Mark Kirkwood :
> On 15/06/11 02:52, Cédric Villemain wrote:
>>
>> 2011/6/3 Mark Kirkwood:
>>>
>>> Corrected v4 patch with the test files, for completeness. Note that
>>> discussion has moved on and there will be a v5 :-)
>>>
>> Mark, can you submit your updated patch ?
>>
>
> Thanks for the reminder! Here is v5. The changes are:

I have issues applying it.
Please can you remove trailing space?
Also, you can generate a cool patch like this :

get git-external-diff from postgres/src/tools to /usr/lib/git-core/
chmod +x it
export GIT_EXTERNAL_DIFF=git-external-diff
git format-patch --ext-diff origin


my log:
$ git apply  temp-files-v5.patch
temp-files-v5.patch:22: trailing whitespace.
defaults to unlimited (-1). Values larger than zero
temp-files-v5.patch:23: trailing whitespace.
constraint the temporary file space usage to be that number of
temp-files-v5.patch:28: trailing whitespace.
the total space used by all the files produced by one backend is
temp-files-v5.patch:35: trailing whitespace.
constrain disk space used for temporary table storage. However if
temp-files-v5.patch:105: trailing whitespace.
/*
error: patch failed: src/backend/storage/file/fd.c:132
error: src/backend/storage/file/fd.c: patch does not apply
error: src/test/regress/expected/resource.out: No such file or directory
error: src/test/regress/sql/resource.sql: No such file or directory


>
> - guc is called temp_file_limit, which seems like the best choice to date
> :-)
> - removed code to do with truncating files, as after testing I agree with
> you that temp work files don't seem to get truncated.
>
> I have not done anything about the business on units - so we are in KB still
> - there is no MB unit avaiable in the code as yet - I'm not sure we need one
> at this point, as most folk who use this feature will find 4096GB a big
> enough *per backend* limit. Obviously it might be a good investment to plan
> to have MB, and GB as possible guc units too. Maybe this could be a separate
> piece of work since any *other* resource limiters we add might find it
> convenient to have these available?
>
> Cheers
>
> Mark
>



-- 
Cédric Villemain               2ndQuadrant
http://2ndQuadrant.fr/     PostgreSQL : Expertise, Formation et Support

-- 
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] Another swing at JSON

[Tom Lane]

Bernd Helmle  writes:
> After reading Joseph's comment upthread, I don't see any consensus
> wether the existing pre-9.1 support is required or even desired. Maybe
> i missed it, but do we really expect an extension (or contrib module)
> to be backwards compatible to earlier major releases, when shipped in
> contrib/ ?

No, we don't.  You won't find any attempt in any contrib module to build
against prior releases.  There's not much point, since they're shipped
with a specific release of the core.

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] Boolean operators without commutators vs. ALL/ANY

[Tom Lane]

Peter Eisentraut  writes:
> I don't really agree that visual correlation needs to trump everything.
> If say
> foo =~ bar
> and
> foo ~= bar
> were to produce completely different results, this would introduce bugs
> all over the place.

Huh?  That's about like arguing that standard mathematical notation is
broken because a < b and a > b don't produce the same result.

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] Another swing at JSON

[Bernd Helmle]

--On 29. März 2011 21:15:11 -0400 Joseph Adams  
wrote:



Thanks.  I applied a minor variation of this trick to the JSON module,
so now it builds/installs/tests cleanly on both REL8_4_0 and HEAD
(though it won't work if you copy contrib/json into a pre-9.1
PostgreSQL source directory and type `make` without USE_PGXS=1).

I also went ahead and renamed uninstall_json.sql to
json--uninstall--0.1.sql (again, it's for pre-9.1 users) and removed
unnecessary trailing spaces.


Anything going into the PostgreSQL code base will be for 9.2, so
anything else would be a separate (if somewhat related) project.  I
suspect the code will be a good deal cleaner if you do just the 9.2+
version and see who wants it back-patched, if anyone does :)


It's a trivial matter to remove backward compatibility from
contrib/json, if anybody wants me to do it.  I can just remove
compat.[ch], */init-pre9.1.* , remove the PREFIX_PGVER trick from the
Makefile, remove a few lines in the source code, and maintain the
backported json module elsewhere.  It's just a matter of whether or
not explicit backward-compatibility is desirable in modules shipped
with releases.


I started looking into this. A very minor adjusted patch to filelist.sgml was 
required to apply the patch cleanly to current -HEAD (attached).


After reading Joseph's comment upthread, I don't see any consensus wether the 
existing pre-9.1 support is required or even desired. Maybe i missed it, but do 
we really expect an extension (or contrib module) to be backwards compatible to 
earlier major releases, when shipped in contrib/ ?


--
Thanks

Bernd



json_contrib_cleaned.patch.gz
Description: GNU Zip compressed data

-- 
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] procpid?

[Bruce Momjian]

Greg Smith wrote:
> -It is still useful to set current_query to descriptive text in the 
> cases where the transaction is  etc.  That text is not ambiguous 
> with a real query, it is useful for a human-readable view, and it 
> improves the potential for pg_stat_sessions to fully replace a 
> deprecated pg_stat_activity (instead of just co-existing with it).  That 
> the query text is overloaded with this information seems agreed to be a 
> good thing; it's just that filtering on the state information there 
> should not require parsing it.  The additional booleans will handle 
> that.  If idle sessions can be filtered using "WHERE NOT idle", whether 
> the current_query for them reads "" or is null won't matter to 
> typical monitoring use.  Given no strong preference there, using 
> "" is both familiar and more human readable.

Uh, if we are going to do that, why not just add the boolean columns to
the existing view?  Clearly renaming procpid isn't worth creating
another view.

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

  + It's impossible for everything to be true. +

-- 
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] pg_upgrade using appname to lock out other users

[Bruce Momjian]

Robert Haas wrote:
> On Wed, Jun 15, 2011 at 1:35 PM, Bruce Momjian  wrote:
> > I now believe we are overthinking all this. ?pg_upgrade has always
> > supported specification of a port number. ?Why not just tell users to
> > specify an unused port number > 1023, and not to use the default value?
> 
> 1. Because it shouldn't be the user's problem to figure out a good
> choice of port number.
> 
> 2. Because we also really ought to be ignoring the contents of
> pg_hba.conf during an upgrade, and instead have some mechanism that
> allows pg_upgrade to be sure of getting in (without creating a
> security hole in the process).
> 
> I agree that back-patching these changes wouldn't be a wonderful
> thing, but we are going to do a lot more releases that have pg_upgrade
> in them in the future than we've already done in the past.  It's not a
> bad thing to try to start improving on the basic mechanism, even if
> takes a while for versions that support that mechanism to become
> commonplace.  Limiting what we're willing to do the server to improve
> the pg_upgrade experience in the future to what we're willing to
> back-patch is not going to be a winning strategy.

OK, well, we have three possible directions:

1.  Just document that people should use -p and -P on unused ports to
avoid user connections

2.  Do #1 and also require -p and -P to be used (no defaults)

3.  Have pg_upgrade default to use a typically unused port number, e.g.
25432 (on Unix, it might only be using unix domain sockets anyway)

4.  Require appname to match 'binary-upgrade' when in -b (binary-upgrade)
mode

5.  Disable TCP when in -b mode on Unix (not possible on Windows)

We can pick different options for 9.0, 9.1, and 9.2.  (For PG 9.0
probably only #1 is appropriate.)

FYI, pg_upgrade 9.1 will already lock out non-super users, but that
doesn't lock out users from old pre-9.1 servers.

So, these are all only a few lines of code --- we just need to figure
out what we want.

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

  + It's impossible for everything to be true. +

-- 
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 - Debug builds without optimization

[Radosław Smogura]

On Thu, 16 Jun 2011 16:00:21 -0400, Tom Lane wrote:

Alvaro Herrera  writes:
I disagree with this change.  Debug builds are very useful to have 
in
production, and you don't want to be running -O0 there.  I have 
found
that you can use a src/Makefile.custom like this for those times 
when you

want to debug stuff in a particular set of files:



CFLAGS := $(patsubst -O2,-O0,$(CFLAGS))



Then you remove the .o files that you want to debug, and rerun make.


FWIW, I only use Makefile.custom for more-or-less-permanent changes 
to

the build behavior of a particular machine.  For one-shot things like
recompiling some particular file(s) at -O0, it's easier to do this:

rm foo.o
make PROFILE=-O0
reinstall postgres executable

The makefiles automatically add PROFILE at the end of CFLAGS, so you 
can
inject any compile flag this way --- I think the original intent was 
to

use it to add -pg for gprof-enabled builds.  But it's handy for this.

BTW, if you're hacking Postgres code and don't already have a
"reinstall" script, you need one.  Mine is basically

pg_ctl stop
cd $PGBLDROOT/src/backend
make install-bin
pg_ctl start

regards, tom lane

Thanks,

Actually I do something like above, but good to know "install-bin" 
target, I fired before "gmake -j5 install".


Regards,
Radek

--
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: CreateComments: use explicit indexing for ``values''

[Tom Lane]

richhguard-monot...@yahoo.co.uk writes:
> This patch makes the intent of each initialization clear by using the 
> constants directly instead of in a comment, and has the effect of being able 
> to verify each line on it's own. The original requires verification of the 
> preceding i++'s.

Applied, along with changing update_attstats() using Alvaro's idea.

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] Nested CASE-WHEN scoping

[Tom Lane]

Heikki Linnakangas  writes:
> The complicated part is to ensure that levelsup is always set correctly. 
> At parse time, levelsup is always set to 0, as the syntax doesn't allow 
> referencing upper levels directly. When an SQL function is inlined, any 
> ExpressionParams in the expressions that are substituted for Params need 
> to have their levelsup adjusted, so that it still refers to the right 
> value if there's CASE expressions in the inlined function. Also, when an 
> ExpressionParam is replaced with a Const, the levelsup fields of any 
> other ExpressionParams within the CaseExpr referring to higher levels 
> need to have their levelsup decremented to account for the fact that the 
> CaseExpr doesn't push the expression parameter anymore.

I believe this is an unworkably complex, and almost certainly buggy
Rube Goldberg device.  Even if it manages to work today, it's going to
be impossible to maintain those levelsup values correctly during
any sort of expression rearrangement or optimization.

Please take another look at just assigning a PARAM_EXEC parameter per
Case expression.

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] gcc 4.6 -Wunused-but-set-variable

[Peter Eisentraut]

On ons, 2011-06-15 at 19:28 -0400, Tom Lane wrote:
> Peter Eisentraut  writes:
> > I couldn't see a way good way of programming around this (perhaps in the
> > second case, but it would get uselessly ugly), so perhaps just marking
> > the variables as potentially unused would be appropriate?  See patch.
> 
> Of course this would break not only on non-gcc compilers, but old
> versions of gcc.  I'd suggest a macro (cf PERL_UNUSED_DECL) and some
> version checks at the site of the macro declaration (perhaps the ones
> emitted by bison for its use of this construct will do).

Non-GCC compilers would be fine, because we define away __attribute__
there anyway, but on GCC itself, you're right, the "unused" attribute is
a bit more recent than ancient.

Actually, casting to void, which is the convention we already use
elsewhere, works for this, so done that way.



-- 
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] proposal: a validator for configuration files

[Alexey Klyukin]

Hi,

On Jun 16, 2011, at 9:18 PM, Florian Pflug wrote:

> On Jun16, 2011, at 20:14 , Alexey Klyukin wrote:
>> 
>> Well, while thinking about this I decided to leave the counter for the 
>> ParseConfigFp, but 
>> drop it in ProcessConfigFile. The case we are protecting against is a single 
>> file full of junk.
>> It's unlikely that this junk would contain include directives with valid 
>> file paths, neither it's
>> likely to find a file with a correct syntax, but full of invalid directives.
> 
> Sounds good.


Attached is the v2 of the patch to show all parse errors at postgresql.conf.
Changes (per review and suggestions from Florian):

- do not stop on the first error during postmaster's start.
- fix errors in processing files from include directives.
- show only a single syntax error per line, i.e. fast forward to the EOL after 
coming across the first one.
- additional comments/error messages, code cleanup

Questions:

- Should we add a comment for the changes in guc.c? I think the existing ones 
are still valid, but they might be harder go grasp, given that we've removed 
PGC_SIGHUP from the condition.
- The error message that we emit when the parsing is unsuccessful, will it 
cause incompatibility w/ 3rd party tools, which may, in theory, show only one 
error message (would it better to show the first error instead, as proposed by 
Florian?).

I'd appreciate your comments and suggestions.

Thank you,
Alexey.

--
Command Prompt, Inc.  http://www.CommandPrompt.com
PostgreSQL Replication, Consulting, Custom Development, 24x7 support



parser_continue_on_errors_v2.diff
Description: Binary data


-- 
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] Nested CASE-WHEN scoping

[Heikki Linnakangas]

On 16.06.2011 22:46, Pavel Stehule wrote:

I have a query - should be ExpressionParam used for removing a
multiple function call when composite result is expanded?

With some similar optimization a SELECT (fce(x)).* FROM tab will be optimal


Hmm, I don't think it will work as is for that purpose, as each 
targetlist item is a separate expression, and ExpressionParams only work 
within an expression.


--
  Heikki Linnakangas
  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] Boolean operators without commutators vs. ALL/ANY

[Peter Eisentraut]

On tor, 2011-06-16 at 00:50 -0400, Tom Lane wrote:
> Peter Eisentraut  writes:
> > On tis, 2011-06-14 at 15:38 +0200, Florian Pflug wrote:
> >> BTW, there's actually precedent for a commutator of "~", namely
> >> "@". Some of the geometric types (polygon, box, circle, point,
> >> path) use "~" as a commutator for "@" (which stands for
> "contains"). 
> 
> > I wouldn't have a problem with naming the reverse operator "@".
> 
> We deprecated those names for the geometric operators largely because
> there wasn't any visual correlation between the commutator pairs.
> I can't see introducing the same pairing for regex operators if we
> already decided the geometric case was a bad idea.

I actually reported the exact issue that Florian reported a while ago
and we had this same sort of discussion.  I think I'm running with a
custom operator named ~~~ somewhere in production.  So yay for adding a
commutator in any case.

I don't really agree that visual correlation needs to trump everything.
If say

foo =~ bar

and

foo ~= bar

were to produce completely different results, this would introduce bugs
all over the place.  Most programming languages would get away with this
kind of issue because the pattern has a different data type than the
string to be matched against, so mistakes will be caught.

Looking at the list of geometric operators, I can't help but feel that
the silliness of operator naming is reaching its limits.  We can
probably come up with a few more for this particular problem, but long
term we might want to think of other solutions, such as attaching the
optimization information to functions instead, and/or inventing an infix
function call syntax like in Haskell.



-- 
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 - Debug builds without optimization

[Tom Lane]

Alvaro Herrera  writes:
> I disagree with this change.  Debug builds are very useful to have in
> production, and you don't want to be running -O0 there.  I have found
> that you can use a src/Makefile.custom like this for those times when you
> want to debug stuff in a particular set of files:

> CFLAGS := $(patsubst -O2,-O0,$(CFLAGS))

> Then you remove the .o files that you want to debug, and rerun make.

FWIW, I only use Makefile.custom for more-or-less-permanent changes to
the build behavior of a particular machine.  For one-shot things like
recompiling some particular file(s) at -O0, it's easier to do this:

rm foo.o
make PROFILE=-O0
reinstall postgres executable

The makefiles automatically add PROFILE at the end of CFLAGS, so you can
inject any compile flag this way --- I think the original intent was to
use it to add -pg for gprof-enabled builds.  But it's handy for this.

BTW, if you're hacking Postgres code and don't already have a
"reinstall" script, you need one.  Mine is basically

pg_ctl stop
cd $PGBLDROOT/src/backend
make install-bin
pg_ctl start

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] flexible array members

[Peter Eisentraut]

On ons, 2011-06-15 at 18:19 -0400, Tom Lane wrote:
> Peter Eisentraut  writes:
> > Is this a route we want to go down?
> 
> > - GISTENTRY   vector[1];  /* variable-length
> array */
> > + GISTENTRY   vector[FLEXIBLE_ARRAY_MEMBER];
> 
> Yes, I was thinking about the same trick after noting these warnings
> on Fedora 15, although personally I'd name the macro
> VARIABLE_LENGTH_ARRAY.

This macro is provided by Autoconf and it appears to be using the
standard's terminology.

Actually, the term "variable-length array" appears to refer to another
C99 feature, namely this one:

void
foo(int n)
{
bar int[n];

do_something();
}



-- 
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] Boolean operators without commutators vs. ALL/ANY

[Tom Lane]

Florian Pflug  writes:
> Well, I think there are basically three choices here, kludge or no
> kludge.

> (1) We either decree once and for all that binary operations ought to
> have commutators, modify CREATE TYPE to issue a warning if you
> create one without, add the missing ones, and add a check for
> that to opr_sanity (possibly excluding some deprecated operators).

> or

> (2) We arrange for commutators of binary operators to be created
> automatically. 

> or

> (3) Or we bit the bullet and provide something similar to
> "ANY/ALL op scalar". We do have the liberty to pick whatever syntax we
> feel comfortable with, though, since we're out of SQL standard territory
> anyway.

All three of these are massive overkill.  What we need is a general
policy that providing commutators is a good idea.  We do not need to try
to make it 100.00% with an enforcement mechanism.  As for #2, what's
your plan for automatically selecting a commutator operator name?

(Having said that, I *was* thinking of adding an opr_sanity test ... but
not expecting that we'd get it to find zero rows.)

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] Nested CASE-WHEN scoping

[Pavel Stehule]

Hello

2011/6/3 Heikki Linnakangas :
> On 31.05.2011 19:10, Heikki Linnakangas wrote:
>>
>> For index expressions, we could use a function similar to
>> ChangeVarNodes(), that shifts all the paramids in the already-planned
>> expression, preparing it for inclusion within the enclosing plan. I'm a
>> bit worried that that might screw up the logic used to compare if an
>> expression matches the index expression, though; the param ids in the
>> two expressions won't match.
>
> Yeah, the expression comparison logic gets all confused by this :-(. I
> couldn't figure out a way to make it work without making the comparison a
> whole lot more complicated than it is today. I'm going back to my original
> thoughts of a new kind of node to replace CaseTestExpr, which allows
> referencing values from upper levels in the expression tree.
>
> So, here's a WIP patch using that approach. I've replaced CaseTestExpr with
> ExpressionParam. ExpressionParam has a levelsup field, which is similar to
> varlevelsup in Var. With levelsup == 0, ExpressionParam works just like
> CaseTestExpr did. With levelsup == 1, it refers to the value from above the
> enclosing CaseExpr (or any other node that uses these
> ExpressionParams/CaseTestExprs).
>

I have a query - should be ExpressionParam used for removing a
multiple function call when composite result is expanded?

With some similar optimization a SELECT (fce(x)).* FROM tab will be optimal

Regards

Pavel Stěhule

> The complicated part is to ensure that levelsup is always set correctly. At
> parse time, levelsup is always set to 0, as the syntax doesn't allow
> referencing upper levels directly. When an SQL function is inlined, any
> ExpressionParams in the expressions that are substituted for Params need to
> have their levelsup adjusted, so that it still refers to the right value if
> there's CASE expressions in the inlined function. Also, when an
> ExpressionParam is replaced with a Const, the levelsup fields of any other
> ExpressionParams within the CaseExpr referring to higher levels need to have
> their levelsup decremented to account for the fact that the CaseExpr doesn't
> push the expression parameter anymore.
>
> At execution time, the expression parameters form a stack. We've always done
> the save-restore logic, but the stack is now represented explicitly as a
> List in ExprContext. When an ExpressionParam is evaluated, the nth element
> is fetched from the stack, corresponding to levelsup.
>
> --
>  Heikki Linnakangas
>  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
>
>

-- 
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] procpid?

[Alvaro Herrera]

Excerpts from Greg Sabino Mullane's message of jue jun 16 15:33:35 UTC 2011:
> 
> Hash: RIPEMD160
> 
> >> Or perhaps pg_connections. Yes, +1 to making things fully backwards
> >> compatible by keeping pg_stat_activity around but making a better
> >> designed and better named table (view/SRF/whatever).
> 
> > I thought about that too when reading the thread the first time, but 
> > "pg_stat_sessions" sounds better. Our documentation also primarily refers 
> > to a 
> > database connection as a "session", i think.
> 
> No, this is clearly connections, not sessions. At least based on the items 
> in the postgresql.conf file, especially max_connections (probably one of the 
> items most closely associated with pg_stat_activity)

That doesn't include autovacuum, though, whereas the new view would.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] Re: starting to review the Extend NOT NULL representation to pg_constraint patch

[Alvaro Herrera]

Excerpts from Bernd Helmle's message of jue jun 16 14:30:48 -0400 2011:
> 
> --On 16. Juni 2011 13:25:05 -0400 Tom Lane  wrote:
> 
> > Possible solution is to leave bootstrap's behavior alone, and have a
> > step during initdb's post-bootstrap stuff that creates a matching
> > pg_constraint row for every pg_attribute entry that's marked attnotnull.
> 
> +1 for this idea. I never came to an end about this because i didn't have any 
> clue how to do it efficiently.

Okay, I have done it this way -- adding one more fixup function to
initdb is very easy.  I only wish that the ending \n in the query to
initdb would be optional -- it took me a while to realize that if you
omit it, the query doesn't get run at all.  Oh well.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] POSIX shared memory patch status

[Heikki Linnakangas]

On 16.06.2011 20:22, A.M. wrote:

I don't believe any conclusions were reached because the debate concerned 
whether or not fcntl locking was sufficient. I thought so while others pointed 
out that the proposed interlock would not work with mutli-client NFSv3 despite 
the fact that the current interlock doesn't either.

I originally made the patch because SysV memory sometimes requires reboots 
which is especially annoying when expanding an existing production db server. 
Even if the next version of postgresql incorporates a hybrid SysV/POSIX shmem 
setup, reboots may still be required if one runs any other processes requiring 
SysV shmem (such as older versions of postgresql).

In any case, I lost interest in maintaining the patch and would not object to 
having the patch removed from the CommitFest.


Ok, I'll mark this as "returned with feedback" then. Thanks for your 
efforts anyway!


--
  Heikki Linnakangas
  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] Boolean operators without commutators vs. ALL/ANY

[Robert Haas]

On Thu, Jun 16, 2011 at 2:22 PM, Florian Pflug  wrote:
> On Jun16, 2011, at 19:54 , Robert Haas wrote:
>> On Thu, Jun 16, 2011 at 12:50 AM, Tom Lane  wrote:
>>> We deprecated those names for the geometric operators largely because
>>> there wasn't any visual correlation between the commutator pairs.
>>> I can't see introducing the same pairing for regex operators if we
>>> already decided the geometric case was a bad idea.
>>
>> I'm having trouble avoiding the conclusion that we're trying to shove
>> a round peg into a square hole.  The idea that we have to have a
>> commutator for every operator just because we don't handle left and
>> right symmetrically sits poorly with me.  I can't really argue with
>> your statement that it's the easiest way to address Florian's gripe,
>> but because it almost surely is.  But it still feels like a kludge.
>
> Well, I think there are basically three choices here, kludge or no
> kludge.
>
> (1) We either decree once and for all that binary operations ought to
> have commutators, modify CREATE TYPE to issue a warning if you
> create one without, add the missing ones, and add a check for
> that to opr_sanity (possibly excluding some deprecated operators).
>
> or
>
> (2) We arrange for commutators of binary operators to be created
> automatically.
>
> or
>
> (3) Or we bit the bullet and provide something similar to
> "ANY/ALL op scalar". We do have the liberty to pick whatever syntax we
> feel comfortable with, though, since we're out of SQL standard territory
> anyway.
>
> What I *wouldn't* like us to is just a few missing commutators and be
> done with it. That pretty much guarantees that this issue will pop up
> again some time in the future.
>
> I personally prefer (3), but would also be content with (1), and be
> ready to provide a patch for that. To be fair, (1) really doesn't seem
> that kludgy if one takes into account that all indexable operators must
> have commutators anyway.
>
> I haven't checked how viable (2) actually is, but I dare say that it's
> probably quite a bit of work. Essentially, we'd need a way to automatically
> swap a function's argument before invoking the function, which I'm not
> sure that fmgr can cleanly be persuaded to do.
>
> Now all that's required is to agree on a way forward ;-)

Well, Tom seems pretty strongly in favor of #1, or some variant of it,
and while I don't find that to be enormously elegant it does have the
virtue of being quite a bit less work than any of the other options.
I think the chances of that being a complete and permanent solution
are less than 50%, but perhaps it's close enough for government work.

-- 
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] WIP: Fast GiST index build

[Alexander Korotkov]

Oh, actually it's so easy. Thanks.

--
With best regards,
Alexander Korotkov.

On Thu, Jun 16, 2011 at 10:26 PM, Heikki Linnakangas <
heikki.linnakan...@enterprisedb.com> wrote:

> On 16.06.2011 21:13, Alexander Korotkov wrote:
>
>> My current idea is to measure number of IO accesses by pg_stat_statements
>> and measure CPU usage by /proc/PID/stat. Any thoughts?
>>
>
> Actually, you get both of those very easily with:
>
> set log_statement_stats=on
>
> LOG:  QUERY STATISTICS
> DETAIL:  ! system usage stats:
>!   0.000990 elapsed 0.00 user 0.00 system sec
>!   [0.00 user 0.008000 sys total]
>!   0/0 [32/0] filesystem blocks in/out
>!   0/0 [0/959] page faults/reclaims, 0 [0] swaps
>!   0 [0] signals rcvd, 0/0 [0/0] messages rcvd/sent
>!   0/0 [10/1] voluntary/involuntary context switches
> STATEMENT:  SELECT generate_series(1,100);
>
>
>
> --
>  Heikki Linnakangas
>  EnterpriseDB   http://www.enterprisedb.com
>

Re: [HACKERS] Re: starting to review the Extend NOT NULL representation to pg_constraint patch

[Bernd Helmle]

--On 16. Juni 2011 13:25:05 -0400 Tom Lane  wrote:


Possible solution is to leave bootstrap's behavior alone, and have a
step during initdb's post-bootstrap stuff that creates a matching
pg_constraint row for every pg_attribute entry that's marked attnotnull.


+1 for this idea. I never came to an end about this because i didn't have any 
clue how to do it efficiently.


--
Thanks

Bernd

--
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] time-delayed standbys

[Robert Haas]

On Wed, Jun 15, 2011 at 1:58 AM, Fujii Masao  wrote:
> When the replication connection is terminated, the standby tries to read
> WAL files from the archive. In this case, there is no walreceiver process,
> so how does the standby calculate the clock difference?

Good question.  Also, just because we have streaming replication
available doesn't mean that we should force people to use it.  It's
still perfectly legit to set up a standby that only use
archive_command and restore_command, and it would be nice if this
feature could still work in such an environment.  I anticipate that
most people want to use streaming replication, but a time-delayed
standby is a good example of a case where you might decide you don't
need it.  It could be useful to have all the WAL present (but not yet
applied) if you're thinking you might want to promote that standby -
but my guess is that in many cases, the time-delayed standby will be
*in addition* to one or more regular standbys that would be the
primary promotion candidates.  So I can see someone deciding that
they'd rather not have the load of another walsender on the master,
and just let the time-delayed standby read from the archive.

Even if that were not an issue, I'm still more or less of the opinion
that trying to solve the time synchronization problem is a rathole
anyway.  To really solve this problem well, you're going to need the
standby to send a message containing a timestamp, get a reply back
from the master that contains that timestamp and a master timestamp,
and then compute based on those two timestamps plus the reply
timestamp the maximum and minimum possible lag between the two
machines.  Then you're going to need to guess, based on several cycles
of this activity, what the actual lag is, and adjust it over time (but
not too quckly, unless of course a large manual step has occurred) as
the clocks potentially drift apart from each other.  This is basically
what ntpd does, except that it can be virtually guaranteed that our
implementation will suck by comparison.  Time synchronization is
neither easy nor our core competency, and I think trying to include it
in this feature is going to result in a net loss of reliability.

>> errmsg("parameter \"%s\" requires a temporal value", "recovery_time_delay"),
>
> We should s/"a temporal"/"an Integer"?

It seems strange to ask for an integer when what we want is an amount
of time in seconds or minutes...

> After we run "pg_ctl promote", time-delayed replication should be disabled?
> Otherwise, failover might take very long time when we set recovery_time_delay
> to high value.

Yeah, I think so.

> http://forge.mysql.com/worklog/task.php?id=344
> According to the above page, one purpose of time-delayed replication is to
> protect against user mistakes on master. But, when an user notices his wrong
> operation on master, what should he do next? The WAL records of his wrong
> operation might have already arrived at the standby, so neither "promote" nor
> "restart" doesn't cancel that wrong operation. Instead, probably he should
> shutdown the standby, investigate the timestamp of XID of the operation
> he'd like to cancel, set recovery_target_time and restart the standby.
> Something like this procedures should be documented? Or, we should
> implement new "promote" mode which finishes a recovery as soon as
> "promote" is requested (i.e., not replay all the available WAL records)?

I like the idea of a new promote mode; and documenting the other
approach you mention doesn't sound bad either.  Either one sounds like
a job for a separate patch, though.

The other option is to pause recovery and run pg_dump...

-- 
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] WIP: Fast GiST index build

[Heikki Linnakangas]

On 16.06.2011 21:13, Alexander Korotkov wrote:

My current idea is to measure number of IO accesses by pg_stat_statements
and measure CPU usage by /proc/PID/stat. Any thoughts?


Actually, you get both of those very easily with:

set log_statement_stats=on

LOG:  QUERY STATISTICS
DETAIL:  ! system usage stats:
!   0.000990 elapsed 0.00 user 0.00 system sec
!   [0.00 user 0.008000 sys total]
!   0/0 [32/0] filesystem blocks in/out
!   0/0 [0/959] page faults/reclaims, 0 [0] swaps
!   0 [0] signals rcvd, 0/0 [0/0] messages rcvd/sent
!   0/0 [10/1] voluntary/involuntary context switches
STATEMENT:  SELECT generate_series(1,100);


--
  Heikki Linnakangas
  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] Boolean operators without commutators vs. ALL/ANY

[Florian Pflug]

On Jun16, 2011, at 19:54 , Robert Haas wrote:
> On Thu, Jun 16, 2011 at 12:50 AM, Tom Lane  wrote:
>> We deprecated those names for the geometric operators largely because
>> there wasn't any visual correlation between the commutator pairs.
>> I can't see introducing the same pairing for regex operators if we
>> already decided the geometric case was a bad idea.
> 
> I'm having trouble avoiding the conclusion that we're trying to shove
> a round peg into a square hole.  The idea that we have to have a
> commutator for every operator just because we don't handle left and
> right symmetrically sits poorly with me.  I can't really argue with
> your statement that it's the easiest way to address Florian's gripe,
> but because it almost surely is.  But it still feels like a kludge.

Well, I think there are basically three choices here, kludge or no
kludge.

(1) We either decree once and for all that binary operations ought to
have commutators, modify CREATE TYPE to issue a warning if you
create one without, add the missing ones, and add a check for
that to opr_sanity (possibly excluding some deprecated operators).

or

(2) We arrange for commutators of binary operators to be created
automatically. 

or

(3) Or we bit the bullet and provide something similar to
"ANY/ALL op scalar". We do have the liberty to pick whatever syntax we
feel comfortable with, though, since we're out of SQL standard territory
anyway.

What I *wouldn't* like us to is just a few missing commutators and be
done with it. That pretty much guarantees that this issue will pop up
again some time in the future.

I personally prefer (3), but would also be content with (1), and be
ready to provide a patch for that. To be fair, (1) really doesn't seem
that kludgy if one takes into account that all indexable operators must
have commutators anyway.

I haven't checked how viable (2) actually is, but I dare say that it's
probably quite a bit of work. Essentially, we'd need a way to automatically
swap a function's argument before invoking the function, which I'm not
sure that fmgr can cleanly be persuaded to do.

Now all that's required is to agree on a way forward ;-)

best regards,
Florian Pflug


-- 
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] proposal: a validator for configuration files

[Florian Pflug]

On Jun16, 2011, at 20:14 , Alexey Klyukin wrote:
> On Jun 16, 2011, at 8:01 PM, Florian Pflug wrote:
>> On Jun16, 2011, at 18:46 , Alexey Klyukin wrote:
>>> I just recalled a reason for counting the total number of errors. There is 
>>> a condition that
>>> checks that the total number of errors is less than 100 and bails out if 
>>> it's more than that
>>> (100 is arbitrary). The reason is to avoid bloating the logs w/ something 
>>> totally unrelated
>>> to postgresql.conf. That was suggested by Tom Lane here:
>>> http://archives.postgresql.org/pgsql-hackers/2009-03/msg01142.php
>> 
>> Ah, right, I missed that. Guess it'll have to stay a counter, then. Still, I 
>> don't think it's
>> worth the effort to make the count correct in case of included files, so I'd 
>> say just add
>> a comment explaining that the count isn't totally accurate.
> 
> Well, while thinking about this I decided to leave the counter for the 
> ParseConfigFp, but 
> drop it in ProcessConfigFile. The case we are protecting against is a single 
> file full of junk.
> It's unlikely that this junk would contain include directives with valid file 
> paths, neither it's
> likely to find a file with a correct syntax, but full of invalid directives.

Sounds good.

best regards,
Florian Pflug


-- 
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] proposal: a validator for configuration files

[Alexey Klyukin]

On Jun 16, 2011, at 8:01 PM, Florian Pflug wrote:

> On Jun16, 2011, at 18:46 , Alexey Klyukin wrote:
>> On Jun 16, 2011, at 6:49 PM, Florian Pflug wrote:
>>> Hm, wouldn't a test for "context == PGC_POSTMASTER" be more appropriate?
>> 
>> In such a case the errors caused by command-line arguments won't stop the 
>> postmaster.
>> PGC_S_FILE seems to handle this correctly. I'm not sure whether it is 
>> appropriate to use
>> there though.
> 
> Ah, yeah, you're right. PGC_S_FILE sounds fine, then. I guess this means you 
> can
> drop the check for "context == PGC_SIGHUP" though, because surely the source 
> must
> be PGC_S_DEFAULT or PGC_S_FILE if context == PGC_SIGHUP, right? So the check 
> would
> become
>  if (source == PGC_S_FILE || source == PGC_S_DEFAULT)
> where it now says
>  if (context == PGC_SIGHUP || source == PGC_S_DEFAULT)

Yes, AFAIK PGC_SIGHUP is redundant, thank you for the suggestion.

>> 
>> I just recalled a reason for counting the total number of errors. There is a 
>> condition that
>> checks that the total number of errors is less than 100 and bails out if 
>> it's more than that
>> (100 is arbitrary). The reason is to avoid bloating the logs w/ something 
>> totally unrelated
>> to postgresql.conf. That was suggested by Tom Lane here:
>> http://archives.postgresql.org/pgsql-hackers/2009-03/msg01142.php
> 
> Ah, right, I missed that. Guess it'll have to stay a counter, then. Still, I 
> don't think it's
> worth the effort to make the count correct in case of included files, so I'd 
> say just add
> a comment explaining that the count isn't totally accurate.

Well, while thinking about this I decided to leave the counter for the 
ParseConfigFp, but 
drop it in ProcessConfigFile. The case we are protecting against is a single 
file full of junk.
It's unlikely that this junk would contain include directives with valid file 
paths, neither it's
likely to find a file with a correct syntax, but full of invalid directives.

Thank you,
Alexey.

--
Command Prompt, Inc.  http://www.CommandPrompt.com
PostgreSQL Replication, Consulting, Custom Development, 24x7 support




-- 
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] Boolean operators without commutators vs. ALL/ANY

[Tom Lane]

Robert Haas  writes:
> I'm having trouble avoiding the conclusion that we're trying to shove
> a round peg into a square hole.  The idea that we have to have a
> commutator for every operator just because we don't handle left and
> right symmetrically sits poorly with me.  I can't really argue with
> your statement that it's the easiest way to address Florian's gripe,
> but because it almost surely is.  But it still feels like a kludge.
> The syntax foo = ANY(bar) is really quite a poorly-designed syntax,
> because the top-level operation is really "ANY", and it has three
> arguments: foo, =, bar.  If the SQL committee had standardized on
> ANY(foo = $0, bar) or some such thing we wouldn't be having this
> conversation.

[ shrug... ]  Take it up with the committee.  The syntax is what it is,
and we should select our operators to fit it, not vice versa.

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] WIP: Fast GiST index build

[Alexander Korotkov]

My current idea is to measure number of IO accesses by pg_stat_statements
and measure CPU usage by /proc/PID/stat. Any thoughts?

--
With best regards,
Alexander Korotkov.


On Thu, Jun 16, 2011 at 1:33 PM, Alexander Korotkov wrote:

> Actually, I would like to measure CPU and IO load independently for more
> comprehensive benchmarks. Can you advice me some appropriate tools for it?
>
> --
> With best regards,
> Alexander Korotkov.
>

Re: [HACKERS] planinstr, showing planner time on EXPLAIN

[Robert Haas]

On Tue, Jun 14, 2011 at 11:18 PM, Hitoshi Harada  wrote:
> Yesterday on PGXN I just released the first version of planinstr, a
> plugin module to append planner time to EXPLAIN. I post this here
> since it is mostly for developers.
>
> http://www.pgxn.org/dist/planinstr/
>
> db1=# load '$libdir/planinstr';
> LOAD
> db1=# explain select * from pg_class;
>                          QUERY PLAN
> ---
>  Seq Scan on pg_class  (cost=0.00..141.87 rows=3287 width=194)
>  Plan time: 0.119 ms
>
> db1=# explain analyze select count(*) from size_m;
>                                                       QUERY PLAN
>
> 
>  Aggregate  (cost=26272.00..26272.01 rows=1 width=0) (actual
> time=51.938..51.938 rows=1 loops=1)
>   ->  Append  (cost=0.00..23147.00 rows=125 width=0) (actual
> time=0.037..45.809 rows=2 loops=1)
>         ->  Seq Scan on size_m  (cost=0.00..847.00 rows=2
> width=0) (actual time=0.037..41.863 rows=2 loops=1)
> <.. snip ..>
>         ->  Seq Scan on myt1000 size_m  (cost=0.00..22.30 rows=1230
> width=0) (actual time=0.001..0.001 rows=0 loops=1)
>  Total runtime: 75.217 ms
>  Plan time: 61.353 ms
> (1005 rows)

Wow, that is awesome.  I sorta wish we had something like that in core
-- and I don't even think it should be optional, I think it should
just always give you that additional piece of information.

I can't immediately count the number of times this would have helped
me, but it's significant.

-- 
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] patch: update README-SSI

[Heikki Linnakangas]

On 16.06.2011 20:33, Dan Ports wrote:

On Thu, Jun 16, 2011 at 04:39:09PM +0300, Heikki Linnakangas wrote:

There's no mention on what T1 is. I believe it's supposed to be Tin, in
the terminology used in the graph.


Yes, I changed the naming after I originally wrote it, and missed a
couple spots. T1 should be Tin.


I don't see how there can be a ww-dependency between T0 and Tin. There
can't be a rw-conflict because Tin is read-only, so surely there can't
be a ww-conflict either?


Yes, it can only be a wr-conflict. Good catch.


Ok, I'll commit with those changes.

--
  Heikki Linnakangas
  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] Boolean operators without commutators vs. ALL/ANY

[Robert Haas]

On Thu, Jun 16, 2011 at 12:50 AM, Tom Lane  wrote:
> We deprecated those names for the geometric operators largely because
> there wasn't any visual correlation between the commutator pairs.
> I can't see introducing the same pairing for regex operators if we
> already decided the geometric case was a bad idea.

I'm having trouble avoiding the conclusion that we're trying to shove
a round peg into a square hole.  The idea that we have to have a
commutator for every operator just because we don't handle left and
right symmetrically sits poorly with me.  I can't really argue with
your statement that it's the easiest way to address Florian's gripe,
but because it almost surely is.  But it still feels like a kludge.
The syntax foo = ANY(bar) is really quite a poorly-designed syntax,
because the top-level operation is really "ANY", and it has three
arguments: foo, =, bar.  If the SQL committee had standardized on
ANY(foo = $0, bar) or some such thing we wouldn't be having this
conversation.

-- 
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] use less space in xl_xact_commit patch

[Simon Riggs]

On Thu, Jun 16, 2011 at 5:34 PM, Robert Haas  wrote:
> On Thu, Jun 16, 2011 at 12:12 PM, Simon Riggs  wrote:
>> On Thu, Jun 16, 2011 at 3:14 PM, Tom Lane  wrote:
>>> Robert Haas  writes:
 On Thu, Jun 16, 2011 at 7:25 AM, Simon Riggs 
 wrote:
> With regards to the naming, I think it would be better if we kept
> XLOG_XACT_COMMIT record exactly as it is now, and make the second
> record an entirely new record called XLOG_XACT_COMMIT_FASTPATH. That
> way we retain backwards compatibility.
>>>
 I liked your previous suggestion of commit and commit-with-info
 better.  There's nothing particularly fast about this; it's just less
 info.  So to speak.
>>>
>>> Yes.  There is no need to preserve backwards compatibility here, so
>>> let's just design the records in a way that makes sense on its own.
>>
>> The only difference I'm proposing is the naming. It was foolish of me
>> to propose that the data structure that is exactly the same should
>> have a different name, yet the new structure should have the same name
>> as the previous version. That will lead to confusion to no benefit. My
>> second suggestion makes sense on its own, for no other reason.
>
> That's a reasonable point, but I still don't really like the name
> "fastpath", because it's not faster, and it's not a path.  It's just
> smaller.  How about xl_xact_commit_simple or xl_xact_commit_compact or
> something like that?

Sure, anything like that is fine for me.

Rather annoyed at my email client because I lost my first email on
this topic, then had to retype it all from memory. The second copy
omitted things like "or similar name, that's not important" which
would have avoided the last couple of mails. :-(

-- 
 Simon Riggs   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] Re: starting to review the Extend NOT NULL representation to pg_constraint patch

[Robert Haas]

On Thu, Jun 16, 2011 at 1:25 PM, Tom Lane  wrote:
> Possible solution is to leave bootstrap's behavior alone, and have a
> step during initdb's post-bootstrap stuff that creates a matching
> pg_constraint row for every pg_attribute entry that's marked attnotnull.

That seems like a pretty good solution.

> I have a feeling that omitting these entries for system catalogs would
> bite us in other ways down the road, even if inheritance were the only
> soft spot right now.

I share that feeling.

-- 
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] pg_upgrade using appname to lock out other users

[Robert Haas]

On Wed, Jun 15, 2011 at 1:35 PM, Bruce Momjian  wrote:
> I now believe we are overthinking all this.  pg_upgrade has always
> supported specification of a port number.  Why not just tell users to
> specify an unused port number > 1023, and not to use the default value?

1. Because it shouldn't be the user's problem to figure out a good
choice of port number.

2. Because we also really ought to be ignoring the contents of
pg_hba.conf during an upgrade, and instead have some mechanism that
allows pg_upgrade to be sure of getting in (without creating a
security hole in the process).

I agree that back-patching these changes wouldn't be a wonderful
thing, but we are going to do a lot more releases that have pg_upgrade
in them in the future than we've already done in the past.  It's not a
bad thing to try to start improving on the basic mechanism, even if
takes a while for versions that support that mechanism to become
commonplace.  Limiting what we're willing to do the server to improve
the pg_upgrade experience in the future to what we're willing to
back-patch is not going to be a winning strategy.

-- 
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] patch: update README-SSI

[Dan Ports]

On Thu, Jun 16, 2011 at 04:39:09PM +0300, Heikki Linnakangas wrote:
> There's no mention on what T1 is. I believe it's supposed to be Tin, in 
> the terminology used in the graph.

Yes, I changed the naming after I originally wrote it, and missed a
couple spots. T1 should be Tin.

> I don't see how there can be a ww-dependency between T0 and Tin. There 
> can't be a rw-conflict because Tin is read-only, so surely there can't 
> be a ww-conflict either?

Yes, it can only be a wr-conflict. Good catch.

Dan

-- 
Dan R. K. Ports  MIT CSAILhttp://drkp.net/

-- 
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] Re: starting to review the Extend NOT NULL representation to pg_constraint patch

[Tom Lane]

Alvaro Herrera  writes:
> So, question: do we need pg_constraint rows to exist for all NOT NULL
> constraints, including those in system catalogs, and including those in
> bootstrap catalogs?  If we're going to require that, we're going to need
> to add a few initial data lines to the pg_constraint catalog definition,
> plus some code to handle the other bootstrap cases (non bootstrap
> relations).

Installing such rows during bootstrap would be problematic, because what
do you do for catalogs that are created before pg_constraint?

Possible solution is to leave bootstrap's behavior alone, and have a
step during initdb's post-bootstrap stuff that creates a matching
pg_constraint row for every pg_attribute entry that's marked attnotnull.

> We could also declare that we don't need pg_constraint rows for NOT NULL
> constraints in system catalogs; but if we're going to do that, I guess
> we'd better disallow tables from inheriting system catalogs.

I have a feeling that omitting these entries for system catalogs would
bite us in other ways down the road, even if inheritance were the only
soft spot right now.

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] POSIX shared memory patch status

[A.M.]

On Jun 16, 2011, at 11:51 AM, Heikki Linnakangas wrote:

> What's the current state of the POSIX shared memory patch? I grabbed the 
> patch from 
> http://archives.postgresql.org/message-id/d9edacf7-53f1-4355-84f8-2e74cd19d...@themactionfaction.com
>  and it doesn't seem to apply cleanly any more. Are you planning to continue 
> working on it?
> 
> If I understood the conclusion of the discussions correctly, the current plan 
> is to continue using a small SysV shared memory segment for the interlock, 
> and POSIX shared memory for the rest. That lets us stay below SHMMAX even if 
> it's small, which is convenient for admins. Was there a conclusion on whether 
> we should use fnctl() to provide some extra safety in the current interlock 
> mechanism? I'm feeling that that should probably be split off to a separate 
> patch, it would be easier to review separately.

Hello,

Please try a merge from my github branch:

https://github.com/agentm/postgres/tree/posix_shmem

I don't believe any conclusions were reached because the debate concerned 
whether or not fcntl locking was sufficient. I thought so while others pointed 
out that the proposed interlock would not work with mutli-client NFSv3 despite 
the fact that the current interlock doesn't either.

I originally made the patch because SysV memory sometimes requires reboots 
which is especially annoying when expanding an existing production db server. 
Even if the next version of postgresql incorporates a hybrid SysV/POSIX shmem 
setup, reboots may still be required if one runs any other processes requiring 
SysV shmem (such as older versions of postgresql).

In any case, I lost interest in maintaining the patch and would not object to 
having the patch removed from the CommitFest.

Cheers,
M
-- 
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 - Debug builds without optimization

[Alvaro Herrera]

Excerpts from Bernd Helmle's message of jue jun 16 09:37:24 -0400 2011:
> 
> 
> --On 16. Juni 2011 14:30:27 +0200 Radosław Smogura  
> wrote:
> 
> >  Hello,
> >
> >  I'm sending following patch which disables optimization when  
> > --enable-debug
> > is passed. It was nasty (for me, at least) that debug  build required 
> > passing
> > of CFLAGS with -O0 to get nice traceable code.
> 
> -O0 hides bugs in your code (e.g. look at 
> 
>  
> and replies for an example to do it better). Doing this automatically on 
> debug 
> builds would be a step backwards.

Hah, seems I don't always do it the same way ;-)

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] Re: starting to review the Extend NOT NULL representation to pg_constraint patch

[Alvaro Herrera]

Thanks, I am looking at the new version from Bernd's git repo.  One
problem I noticed is that it doesn't really work correctly for all
callers of heap_create_with_catalog -- you're only passing the cooked
not null constraints in DefineRelation, but there are some other places
that call heap_create_with_catalog too.  In particular, bootstrap mode
is not handled; I haven't checked the other callers yet.  I'm looking
for the best way to handle that.

So, question: do we need pg_constraint rows to exist for all NOT NULL
constraints, including those in system catalogs, and including those in
bootstrap catalogs?  If we're going to require that, we're going to need
to add a few initial data lines to the pg_constraint catalog definition,
plus some code to handle the other bootstrap cases (non bootstrap
relations).

We could also declare that we don't need pg_constraint rows for NOT NULL
constraints in system catalogs; but if we're going to do that, I guess
we'd better disallow tables from inheriting system catalogs.  Right now
it's not disallowed but I guess it's pretty useless

alvherre=# create table bar() inherits (pg_class);
CREATE TABLE

... on the other hand, being able to use a catalog in a LIKE column
definition sounds like it could be useful:

alvherre=# create table qux (now timestamp, like pg_class);
CREATE TABLE
alvherre=# \d qux
  Tabla «public.qux»
Columna |Tipo | Modificadores 
+-+---
 now| timestamp without time zone | 
 relname| name| not null
 relnamespace   | oid | not null
 reltype| oid | not null


-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] proposal: a validator for configuration files

[Florian Pflug]

On Jun16, 2011, at 18:46 , Alexey Klyukin wrote:
> On Jun 16, 2011, at 6:49 PM, Florian Pflug wrote:
>> Hm, wouldn't a test for "context == PGC_POSTMASTER" be more appropriate?
> 
> In such a case the errors caused by command-line arguments won't stop the 
> postmaster.
> PGC_S_FILE seems to handle this correctly. I'm not sure whether it is 
> appropriate to use
> there though.

Ah, yeah, you're right. PGC_S_FILE sounds fine, then. I guess this means you can
drop the check for "context == PGC_SIGHUP" though, because surely the source 
must
be PGC_S_DEFAULT or PGC_S_FILE if context == PGC_SIGHUP, right? So the check 
would
become
  if (source == PGC_S_FILE || source == PGC_S_DEFAULT)
where it now says
  if (context == PGC_SIGHUP || source == PGC_S_DEFAULT)

 I see that you basically replaced "goto cleanup..." in both ParseConfigFp()
 and ProcessConfigFile() with "++errorcount", and arranged for 
 ParseConfigFp()
 to return false, and for ProcessConfigFile() to skip the GUC updates if
 "errorcount > 0". The actual value of errorcount is never inspected. The 
 value
 is also wrong in the case of include files containing more than error, 
 since
 ParseConfigFp() simply increments errorcount by one for each failed
 ParseConfigFile() of an included file.
 
 I thus suggest that you replace "errorcount" with a boolean 
 "erroroccurred".
>>> 
>>> I can actually pass errorcount down from the ParseConfigFp() to report the 
>>> total
>>> number of errors (w/ the include files) at the end of ProcessConfigFile if 
>>> there is
>>> any interest in having the number of errors reported to a user. If not - 
>>> I'll change
>>> it to boolean.
>> 
>> Nah, just use a boolean, unless you have concrete plans to actually use the 
>> errorcount
>> for something other than test a la "errorcount  > 0".
> 
> I just recalled a reason for counting the total number of errors. There is a 
> condition that
> checks that the total number of errors is less than 100 and bails out if it's 
> more than that
> (100 is arbitrary). The reason is to avoid bloating the logs w/ something 
> totally unrelated
> to postgresql.conf. That was suggested by Tom Lane here:
> http://archives.postgresql.org/pgsql-hackers/2009-03/msg01142.php

Ah, right, I missed that. Guess it'll have to stay a counter, then. Still, I 
don't think it's
worth the effort to make the count correct in case of included files, so I'd 
say just add
a comment explaining that the count isn't totally accurate.

best regards,
Florian Pflug


-- 
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] Why polecat and colugos are failing to build back branches

[Robert Haas]

On Thu, Jun 16, 2011 at 9:48 AM, Robert Creager  wrote:
> On Jun 15, 2011, at 7:51 PM, Tom Lane  wrote:
>
>> ...
>> installation paths.  About the only good thing to be said about it is
>> that these characters are so troublesome that Unix users are unlikely
>> to use them in directory names anyway.
>
> So I'm guessing you don't want this path name?  I was going to throw some \n 
> and & in it also, maybe some *,' and " for good measure >-)  Shall I just 
> stick with spaces?
>
> drwxr-xr-x   2 Robert  wheel    68B Jun 15 17:26 pg bu!ldfa$m\a\$y/
> Robert-Creagers-iMac:src Robert$

LOL.  +1 for including that in the buildfarm, but only if you write
the patch that makes it work yourself...!

-- 
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] procpid?

[Kevin Grittner]

Tom Lane  wrote:
 
> The point is that another backend's entry could be in a different
> *server* encoding, and what do you do if there's no equivalent
> character in your encoding?
 
My first thought was that it was just a matter of picking a
character to represent the "unprintable" characters.  My second
thought was that if you don't understand the encoding scheme, you're
not even going to know where the character boundaries are.  :-(
 
-Kevin

-- 
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] procpid?

[Bernd Helmle]

--On 16. Juni 2011 15:33:35 + Greg Sabino Mullane  wrote:


No, this is clearly connections, not sessions. At least based on the items
in the postgresql.conf file, especially max_connections (probably one of the
items most closely associated with pg_stat_activity)


Well, but it doesn't show database connection(s) only, it also shows what 
actions are currently performed through the various connections on the 
databases and state information about them. I'm not a native english speaker, 
but i have the feeling that "sessions" is better suited for this kind of 
interactive monitoring. I believe Oracle also has a v$session view to query 
various information about what's going on.


--
Thanks

Bernd

--
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] POSIX shared memory patch status

[Robert Haas]

On Thu, Jun 16, 2011 at 11:51 AM, Heikki Linnakangas
 wrote:
> What's the current state of the POSIX shared memory patch? I grabbed the
> patch from
> http://archives.postgresql.org/message-id/d9edacf7-53f1-4355-84f8-2e74cd19d...@themactionfaction.com
> and it doesn't seem to apply cleanly any more. Are you planning to continue
> working on it?
>
> If I understood the conclusion of the discussions correctly, the current
> plan is to continue using a small SysV shared memory segment for the
> interlock, and POSIX shared memory for the rest. That lets us stay below
> SHMMAX even if it's small, which is convenient for admins.

+1, emphatically.

> Was there a
> conclusion on whether we should use fnctl() to provide some extra safety in
> the current interlock mechanism? I'm feeling that that should probably be
> split off to a separate patch, it would be easier to review separately.

+1 for a separate patch.  I see no reason why fixing that's got to be
intertwined with this.  It'd be relevant if we were planning to remove
the current POSIX shm interlock, but so far nobody has offered a
compelling explanation of why we should.  The facility is supported
pretty much everywhere; it's just the size limit that's a problem.

-- 
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] use less space in xl_xact_commit patch

[Tom Lane]

Robert Haas  writes:
> That's a reasonable point, but I still don't really like the name
> "fastpath", because it's not faster, and it's not a path.  It's just
> smaller.  How about xl_xact_commit_simple or xl_xact_commit_compact or
> something like that?


xl_xact_commit_short ?


"_simple" would probably be my next choice.

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] proposal: a validator for configuration files

[Alexey Klyukin]

On Jun 16, 2011, at 6:49 PM, Florian Pflug wrote:

> On Jun16, 2011, at 17:23 , Alexey Klyukin wrote:
>> On Jun 16, 2011, at 2:34 PM, Florian Pflug wrote:
>>> The first problem I ran into when I tried to test this is that it *only*
>>> reports multiple errors during config file reload on SIHUP, not during
>>> postmaster startup. I guess it's been done that way because we
>>> ereport(ERROR,..) not ereport(LOG,...) during postmaster startup, so it's
>>> not immediatly clear how to report multiple errors. But that proplem
>>> seems solvable. What if you ereport(LOG,..)ed the individual errors during
>>> postmaster startup, and then emitted an ereport(ERROR) at the end if
>>> errors occurred? The ERROR could either repeat the first error that was
>>> encountered, or simply say "config file contains errors".
>> 
>> Makes sense. One problem I came across is that set_config_option from guc.c
>> sets the elevel by itself. I've changed it to emit LOG errors on PGC_S_FILE
>> source, apparently all the callers of this function with this source are from
>> guc-file.l, so hopefully I won't break anything with this change.
> 
> Hm, wouldn't a test for "context == PGC_POSTMASTER" be more appropriate?

In such a case the errors caused by command-line arguments won't stop the 
postmaster.
PGC_S_FILE seems to handle this correctly. I'm not sure whether it is 
appropriate to use
there though.

> 
>>> I see that you basically replaced "goto cleanup..." in both ParseConfigFp()
>>> and ProcessConfigFile() with "++errorcount", and arranged for 
>>> ParseConfigFp()
>>> to return false, and for ProcessConfigFile() to skip the GUC updates if
>>> "errorcount > 0". The actual value of errorcount is never inspected. The 
>>> value
>>> is also wrong in the case of include files containing more than error, since
>>> ParseConfigFp() simply increments errorcount by one for each failed
>>> ParseConfigFile() of an included file.
>>> 
>>> I thus suggest that you replace "errorcount" with a boolean "erroroccurred".
>> 
>> I can actually pass errorcount down from the ParseConfigFp() to report the 
>> total
>> number of errors (w/ the include files) at the end of ProcessConfigFile if 
>> there is
>> any interest in having the number of errors reported to a user. If not - 
>> I'll change
>> it to boolean.
> 
> Nah, just use a boolean, unless you have concrete plans to actually use the 
> errorcount
> for something other than test a la "errorcount  > 0".

I just recalled a reason for counting the total number of errors. There is a 
condition that
checks that the total number of errors is less than 100 and bails out if it's 
more than that
(100 is arbitrary). The reason is to avoid bloating the logs w/ something 
totally unrelated
to postgresql.conf. That was suggested by Tom Lane here:
http://archives.postgresql.org/pgsql-hackers/2009-03/msg01142.php

Thank you,
Alexey.

--
Command Prompt, Inc.  http://www.CommandPrompt.com
PostgreSQL Replication, Consulting, Custom Development, 24x7 support




-- 
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] procpid?

[Tom Lane]

Greg Smith  writes:
> The only other item related to this view on the TODO was "Have 
> pg_stat_activity display query strings in the correct client encoding".  
> That might be worthwhile to bundle into this rework, but it doesn't seem 
> something that impacts the UI such that it must be considered early.

That entry is garbled to the point of uselessness anyway, as client
encoding has got exactly zip to do with it.  The point is that another
backend's entry could be in a different *server* encoding, and what do
you do if there's no equivalent character in your encoding?

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] use less space in xl_xact_commit patch

[Robert Haas]

On Thu, Jun 16, 2011 at 12:12 PM, Simon Riggs  wrote:
> On Thu, Jun 16, 2011 at 3:14 PM, Tom Lane  wrote:
>> Robert Haas  writes:
>>> On Thu, Jun 16, 2011 at 7:25 AM, Simon Riggs 
>>> wrote:
 With regards to the naming, I think it would be better if we kept
 XLOG_XACT_COMMIT record exactly as it is now, and make the second
 record an entirely new record called XLOG_XACT_COMMIT_FASTPATH. That
 way we retain backwards compatibility.
>>
>>> I liked your previous suggestion of commit and commit-with-info
>>> better.  There's nothing particularly fast about this; it's just less
>>> info.  So to speak.
>>
>> Yes.  There is no need to preserve backwards compatibility here, so
>> let's just design the records in a way that makes sense on its own.
>
> The only difference I'm proposing is the naming. It was foolish of me
> to propose that the data structure that is exactly the same should
> have a different name, yet the new structure should have the same name
> as the previous version. That will lead to confusion to no benefit. My
> second suggestion makes sense on its own, for no other reason.

That's a reasonable point, but I still don't really like the name
"fastpath", because it's not faster, and it's not a path.  It's just
smaller.  How about xl_xact_commit_simple or xl_xact_commit_compact or
something like that?

-- 
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] PATCH: CreateComments: use explicit indexing for ``values''

[Florian Pflug]

On Jun14, 2011, at 17:47 , richhguard-monot...@yahoo.co.uk wrote:
> This patch makes the intent of each initialization clear by using
> the constants directly instead of in a comment, and has the effect
> of being able to verify each line on it's own. The original requires
> verification of the preceding i++'s.

Here's a review of that patch.

The patch applies cleanly to HEAD, looks correct, and passes "make check".

The patch makes the code far more readable and makes adding new columns
to one of the affected system catalogs less error-prone.

Since the chance of us ever back-patching changes to the system catalog's
structure, the patch doesn't introduce additional back-patching hurdles.

I'm thus marking this Read for Committer.

best regards,
Florian Pflug


-- 
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] use less space in xl_xact_commit patch

[Simon Riggs]

On Thu, Jun 16, 2011 at 3:14 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Thu, Jun 16, 2011 at 7:25 AM, Simon Riggs 
>> wrote:
>>> With regards to the naming, I think it would be better if we kept
>>> XLOG_XACT_COMMIT record exactly as it is now, and make the second
>>> record an entirely new record called XLOG_XACT_COMMIT_FASTPATH. That
>>> way we retain backwards compatibility.
>
>> I liked your previous suggestion of commit and commit-with-info
>> better.  There's nothing particularly fast about this; it's just less
>> info.  So to speak.
>
> Yes.  There is no need to preserve backwards compatibility here, so
> let's just design the records in a way that makes sense on its own.

The only difference I'm proposing is the naming. It was foolish of me
to propose that the data structure that is exactly the same should
have a different name, yet the new structure should have the same name
as the previous version. That will lead to confusion to no benefit. My
second suggestion makes sense on its own, for no other reason.

-- 
 Simon Riggs   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] use less space in xl_xact_commit patch

[Alvaro Herrera]

Excerpts from Leonardo Francalanci's message of jue jun 16 09:00:15 -0400 2011:

> Should I also change the struct name from xl_xact_commit to
> xl_xact_commit_fast_path?

Yes, please.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] On-the-fly index tuple deletion vs. hot_standby

[Noah Misch]

On Thu, Jun 16, 2011 at 04:53:41PM +0100, Simon Riggs wrote:
> On Thu, Jun 16, 2011 at 3:47 PM, Noah Misch  wrote:
> > Thanks. ?We still hit a conflict when btpo.xact == RecentGlobalXmin and the
> > standby has a transaction older than any master transaction. ?This happens
> > because the tests at nbtpage.c:704 and procarray.c:1843 both pass when the 
> > xid
> > exactly is that of the oldest standby transaction (line numbers as of git
> > cb94db91b). ?I only know this because the test script from my last message 
> > hits
> > this case; it might never get hit in real usage. ?Still, seems like a hole 
> > not
> > worth leaving. ?I think the most-correct fix is to TransactionIdRetreat the
> > btpo.xact before using it as xl_btree_reuse_page.lastestRemovedXid. 
> > ?btpo.xact
> > is the first known-safe xid, but latestRemovedXid is the last known-unsafe 
> > xmin.
> 
> I think you are pointing out another bug, rather than a problem in my
> last commit.
> 
> The bug was caused by assuming that the xid is a "latestRemovedXid",
> as is the case in the rest of Hot Standby, which masks the off-by-one
> error through poor use of terms.

Exactly.

> I agree with your suggested fix.

(Note that TransactionIdRetreat mutates its argument and returns nothing.)

Thanks,
nm

-- 
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] FOREIGN TABLE doc fix

[Ross J. Reedstrom]

Right, but I think he needs the "it's not easy, here's the whole
workflow" overview first.

Ross
-- 
Ross Reedstrom, Ph.D. reeds...@rice.edu
Systems Engineer & Admin, Research Scientistphone: 713-348-6166
Connexions  http://cnx.orgfax: 713-348-3665
Rice University MS-375, Houston, TX 77005
GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E  F888 D3AE 810E 88F0 BEDE


On Mon, Jun 13, 2011 at 04:54:13PM +0100, Dave Page wrote:
> On Mon, Jun 13, 2011 at 4:38 PM, Andrew Dunstan  wrote:
> >
> >
> > On 06/13/2011 10:25 AM, Dave Page wrote:
> >>
> >>> Don't hold your breath.  We'll probably be making enough changes in the
> >>> FDW infrastructure (particularly planner support) that making an FDW
> >>> work on both 9.1 and 9.2 would be an exercise in frustration, if it's
> >>> even possible.
> >>
> >> Oh joy. There's a GSoC student working on 2 non-trivial FDW's right
> >> now, and I have a couple I've been working on. If we're going to make
> >> the API incompatible to that extent, we might as well not bother :-(
> >>
> >
> > If nobody bothers then there won't be any experience on which to base a
> > stable API. In particular, I think it's crucial that we get working FDWs for
> > MySQL, SQLServer and Oracle ASAP.
> 
> Yeah - MySQL is one of the ones I've been hacking on. It's hard to be
> motivated if its going to need a complete rewrite within a year
> though. I'll still have to work on it, as I've committed to giving
> talks on it, but others might not bother to even start.
 
I think PostgreSQL has a better track record (especially recently) than
most open source projects at supporting the shared incremental creation and
improvement of first-class features. Yes, getting the first cut of FDW
in place was hard: now it's time for users of that feature to take the
leap of faith and write some code. The faith bit is that others _will_
come forward to help with the rewrites necessary to make it work (or
work better) for their use cases. 

-- 
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] On-the-fly index tuple deletion vs. hot_standby

[Simon Riggs]

On Thu, Jun 16, 2011 at 4:53 PM, Simon Riggs  wrote:

> I agree with your suggested fix.

Please ignore the previous patch, which was sent in error. Here's the
fix. I'll apply this tomorrow morning if we all still agree.

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


bt_recycle_offby1.v1.patch
Description: Binary data

-- 
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] [WIP] [Stream] Preview of pg_type changes

[Radoslaw Smogura]

This is some attempt to make "streaming"  protocol. Difference is that instead 
of returning bytes it is intended to take stream, and self-stream.

I posted, one day, some requirements for streaming, I can't reference it now, 
as I am away from  computer.

Regards,
Radek

-Original Message-
From: Pavel Stehule
Sent: 16 czerwca 2011 17:37
To: Radosław Smogura
Cc: PG Hackers
Subject: Re: [HACKERS] [WIP] [Stream] Preview of pg_type changes

Hello

2011/6/16 Radosław Smogura :
> Hello,
>
> Here I would like to expose changes to pg_type and type infrastructure about
> streaming. Changes are as follows:
> - added new column typstreamin typestremout
> - general contract for those is for streamin same as receive (receive use
> internal), for streamout it is (internal, )
> - changes to psql help
> - and all functionality for type manipulation.
>

what is difference between typestremout function and typesend function?

Regards

Pavel Stehule

> There is no streamin/streamout methods implemented.
>
> If someone wants and have time, as this is WIP patch, then suggestions are
> welcome.
>
> Regards,
> Radek
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>



-- 
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] Latch implementation that wakes on postmaster death on both win32 and Unix

[Alvaro Herrera]

Excerpts from Peter Geoghegan's message of jue jun 16 08:42:39 -0400 2011:
> On 16 June 2011 13:15, Heikki Linnakangas
>  wrote:
> 
> > Hmm, I'm not sure having the pid in that error message is too useful in the
> > first place. The process was just spawned, and it will die at that error.
> > When you try to debug that sort of error, what you would compare the pid
> > with? And you can include the pid in log_line_prefix if it turns out to be
> > useful after all.
> 
> All fair points. FWIW, I think it's pretty unlikely that anyone will
> ever see this error message.

... in which case the getpid() call is not that expensive anyway.

I agree that the PID should be part of the log_line_prefix though, which
is why I was trying to propose we include it (or the session ID) in the
default log_line_prefix along with a suitable timestamp.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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 - Debug builds without optimization

[Alvaro Herrera]

Excerpts from Radosław Smogura's message of jue jun 16 08:30:27 -0400 2011:
>  Hello,
> 
>  I'm sending following patch which disables optimization when 
>  --enable-debug is passed. It was nasty (for me, at least) that debug 
>  build required passing of CFLAGS with -O0 to get nice traceable code.

I disagree with this change.  Debug builds are very useful to have in
production, and you don't want to be running -O0 there.  I have found
that you can use a src/Makefile.custom like this for those times when you
want to debug stuff in a particular set of files:

CFLAGS := $(patsubst -O2,-O0,$(CFLAGS))

Then you remove the .o files that you want to debug, and rerun make.
This places the burden on the developer wanting to mess with random code
changes.  Of course, this means that production builds are not as
debuggable, but IME it's much less of a problem there.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] On-the-fly index tuple deletion vs. hot_standby

[Simon Riggs]

On Thu, Jun 16, 2011 at 3:47 PM, Noah Misch  wrote:
> On Thu, Jun 16, 2011 at 12:02:47AM +0100, Simon Riggs wrote:
>> On Tue, Jun 14, 2011 at 5:28 AM, Noah Misch  wrote:
>> > On Mon, Jun 13, 2011 at 04:16:06PM +0100, Simon Riggs wrote:
>> >> On Mon, Jun 13, 2011 at 3:11 AM, Robert Haas  
>> >> wrote:
>> >> > On Sun, Jun 12, 2011 at 3:01 PM, Noah Misch  wrote:
>> >> >> Assuming that conclusion, I do think it's worth starting
>> >> >> with something simple, even if it means additional bloat on the master 
>> >> >> in the
>> >> >> wal_level=hot_standby + vacuum_defer_cleanup_age / 
>> >> >> hot_standby_feedback case.
>> >> >> In choosing those settings, the administrator has taken constructive 
>> >> >> steps to
>> >> >> accept master-side bloat in exchange for delaying recovery conflict. 
>> >> >> ?What's
>> >> >> your opinion?
>> >> >
>> >> > I'm pretty disinclined to go tinkering with 9.1 at this point, too.
>> >>
>> >> Not least because a feature already exists in 9.1 to cope with this
>> >> problem: hot standby feedback.
>> >
>> > A standby's receipt of an XLOG_BTREE_REUSE_PAGE record implies that the
>> > accompanying latestRemovedXid preceded or equaled the master's RecentXmin 
>> > at the
>> > time of issue (see _bt_page_recyclable()). ?Neither hot_standby_feedback 
>> > nor
>> > vacuum_defer_cleanup_age affect RecentXmin. ?Therefore, neither facility 
>> > delays
>> > conflicts arising directly from B-tree page reuse. ?See attached test 
>> > script,
>> > which yields a snapshot conflict despite active hot_standby_feedback.
>>
>> OK, agreed. Bug. Good catch, Noah.
>>
>> Fix is to use RecentGlobalXmin for the cutoff when in Hot Standby
>> mode, so that it is under user control.
>>
>> Attached patch will be applied to head and backpatched to 9.1 and 9.0
>> to fix this.
>
> Thanks.  We still hit a conflict when btpo.xact == RecentGlobalXmin and the
> standby has a transaction older than any master transaction.  This happens
> because the tests at nbtpage.c:704 and procarray.c:1843 both pass when the xid
> exactly is that of the oldest standby transaction (line numbers as of git
> cb94db91b).  I only know this because the test script from my last message 
> hits
> this case; it might never get hit in real usage.  Still, seems like a hole not
> worth leaving.  I think the most-correct fix is to TransactionIdRetreat the
> btpo.xact before using it as xl_btree_reuse_page.lastestRemovedXid.  btpo.xact
> is the first known-safe xid, but latestRemovedXid is the last known-unsafe 
> xmin.

I think you are pointing out another bug, rather than a problem in my
last commit.

The bug was caused by assuming that the xid is a "latestRemovedXid",
as is the case in the rest of Hot Standby, which masks the off-by-one
error through poor use of terms.

I agree with your suggested fix.

Thanks again.

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


bt_recycle_offby1.v1.patch
Description: Binary data

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

[HACKERS] POSIX shared memory patch status

[Heikki Linnakangas]

What's the current state of the POSIX shared memory patch? I grabbed the 
patch from 
http://archives.postgresql.org/message-id/d9edacf7-53f1-4355-84f8-2e74cd19d...@themactionfaction.com 
and it doesn't seem to apply cleanly any more. Are you planning to 
continue working on it?


If I understood the conclusion of the discussions correctly, the current 
plan is to continue using a small SysV shared memory segment for the 
interlock, and POSIX shared memory for the rest. That lets us stay below 
SHMMAX even if it's small, which is convenient for admins. Was there a 
conclusion on whether we should use fnctl() to provide some extra safety 
in the current interlock mechanism? I'm feeling that that should 
probably be split off to a separate patch, it would be easier to review 
separately.


--
  Heikki Linnakangas
  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] proposal: a validator for configuration files

[Florian Pflug]

On Jun16, 2011, at 17:23 , Alexey Klyukin wrote:
> On Jun 16, 2011, at 2:34 PM, Florian Pflug wrote:
>> The first problem I ran into when I tried to test this is that it *only*
>> reports multiple errors during config file reload on SIHUP, not during
>> postmaster startup. I guess it's been done that way because we
>> ereport(ERROR,..) not ereport(LOG,...) during postmaster startup, so it's
>> not immediatly clear how to report multiple errors. But that proplem
>> seems solvable. What if you ereport(LOG,..)ed the individual errors during
>> postmaster startup, and then emitted an ereport(ERROR) at the end if
>> errors occurred? The ERROR could either repeat the first error that was
>> encountered, or simply say "config file contains errors".
> 
> Makes sense. One problem I came across is that set_config_option from guc.c
> sets the elevel by itself. I've changed it to emit LOG errors on PGC_S_FILE
> source, apparently all the callers of this function with this source are from
> guc-file.l, so hopefully I won't break anything with this change.

Hm, wouldn't a test for "context == PGC_POSTMASTER" be more appropriate?

>> I see that you basically replaced "goto cleanup..." in both ParseConfigFp()
>> and ProcessConfigFile() with "++errorcount", and arranged for ParseConfigFp()
>> to return false, and for ProcessConfigFile() to skip the GUC updates if
>> "errorcount > 0". The actual value of errorcount is never inspected. The 
>> value
>> is also wrong in the case of include files containing more than error, since
>> ParseConfigFp() simply increments errorcount by one for each failed
>> ParseConfigFile() of an included file.
>> 
>> I thus suggest that you replace "errorcount" with a boolean "erroroccurred".
> 
> I can actually pass errorcount down from the ParseConfigFp() to report the 
> total
> number of errors (w/ the include files) at the end of ProcessConfigFile if 
> there is
> any interest in having the number of errors reported to a user. If not - I'll 
> change
> it to boolean.

Nah, just use a boolean, unless you have concrete plans to actually use the 
errorcount
for something other than test a la "errorcount  > 0".

best regards,
Florian Pflug


-- 
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] [WIP] [Stream] Preview of pg_type changes

[Pavel Stehule]

Hello

2011/6/16 Radosław Smogura :
> Hello,
>
> Here I would like to expose changes to pg_type and type infrastructure about
> streaming. Changes are as follows:
> - added new column typstreamin typestremout
> - general contract for those is for streamin same as receive (receive use
> internal), for streamout it is (internal, )
> - changes to psql help
> - and all functionality for type manipulation.
>

what is difference between typestremout function and typesend function?

Regards

Pavel Stehule

> There is no streamin/streamout methods implemented.
>
> If someone wants and have time, as this is WIP patch, then suggestions are
> welcome.
>
> Regards,
> Radek
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

-- 
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] procpid?

[Greg Sabino Mullane]

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160


>> Or perhaps pg_connections. Yes, +1 to making things fully backwards
>> compatible by keeping pg_stat_activity around but making a better
>> designed and better named table (view/SRF/whatever).

> I thought about that too when reading the thread the first time, but 
> "pg_stat_sessions" sounds better. Our documentation also primarily refers to 
> a 
> database connection as a "session", i think.

No, this is clearly connections, not sessions. At least based on the items 
in the postgresql.conf file, especially max_connections (probably one of the 
items most closely associated with pg_stat_activity)

- -- 
Greg Sabino Mullane g...@turnstep.com
End Point Corporation http://www.endpoint.com/
PGP Key: 0x14964AC8 201106161132
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-BEGIN PGP SIGNATURE-

iEYEAREDAAYFAk36IjYACgkQvJuQZxSWSsg8MgCgkMNw1o37cgmtJdYBAsGl7kz6
Q8sAoISFra0LyQjyKw3zcapWBdCLh2RV
=EYAc
-END PGP SIGNATURE-



-- 
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] Latch implementation that wakes on postmaster death on both win32 and Unix

[Heikki Linnakangas]

This patch breaks silent_mode=on. In silent_mode, postmaster forks early 
on, to detach from the controlling tty. It uses fork_process() for that, 
which with patch closes the write end of the postmaster-alive pipe, but 
that's wrong because the child becomes the postmaster process.


On a stylistic note, the "extern" declaration in unix_latch.c is ugly, 
extern declarations should be in header files. Come to think of it, I 
feel the Init- and ReleasePostmasterDeathWatchHandle() functions should 
go to postmaster.c. postmaster_alive_fds[] and PostmasterHandle serve 
the same purpose, declaration and initialization of both should be kept 
together, perhaps by moving the initialization of PostmasterHandle into 
Init- and ReleasePostmasterDeathWatchHandle().


--
  Heikki Linnakangas
  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

[HACKERS] [WIP] [Stream] Preview of pg_type changes

[Radosław Smogura]

Hello,

Here I would like to expose changes to pg_type and type infrastructure 
about streaming. Changes are as follows:

- added new column typstreamin typestremout
- general contract for those is for streamin same as receive (receive 
use internal), for streamout it is (internal, )

- changes to psql help
- and all functionality for type manipulation.

There is no streamin/streamout methods implemented.

If someone wants and have time, as this is WIP patch, then suggestions 
are welcome.


Regards,
Radek

stream_pg_type.patch.gz
Description: GNU Zip compressed data

-- 
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] procpid?

[Greg Smith]

On 06/15/2011 12:41 PM, Robert Haas wrote:

But I will note that we had better be darn sure to make all the changes we
want to make in one go, because I dowanna have to create pg_sessions2
(or pg_tessions?) in a year or three.
   


I just added a new section to the TODO to start collecting up some of 
these related ideas into one place:  
http://wiki.postgresql.org/wiki/Todo#Monitoring so we might try to get 
as many as possible all in one go.


The other item on there related to pg_stat_activity that might impact 
this design was adding a column for tracking progress of commands like 
CREATE INDEX and VACUUM (I updated to note CLUSTER falls into that 
category too).  While query progress will always be a hard problem, 
adding a field to store some sort of progress indicator might be useful 
even if it only worked on these two initially.  Anyway, topic for 
another time.


The only other item related to this view on the TODO was "Have 
pg_stat_activity display query strings in the correct client encoding".  
That might be worthwhile to bundle into this rework, but it doesn't seem 
something that impacts the UI such that it must be considered early.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us



--
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] proposal: a validator for configuration files

[Alexey Klyukin]

Florian,

On Jun 16, 2011, at 2:34 PM, Florian Pflug wrote:

> Hi
> 
> On May14, 2011, at 00:49 , Alexey Klyukin wrote:
>> The patch forces the parser to report all errors (max 100) from the
>> ProcessConfigFile/ParseConfigFp. Currently, only the first parse error or an
>> invalid directive is reported. Reporting all of them is crucial to automatic
>> validation of postgres config files.
>> 
>> This patch is based on the one submitted earlier by Selena Deckelmann:
>> http://archives.postgresql.org/pgsql-hackers/2009-03/msg00345.php
>> 
>> It incorporates suggestions by Tom Lane for avoiding excessive bloat in logs
>> in case there is a junk instead of postgresql.conf.
>> http://archives.postgresql.org/pgsql-hackers/2009-03/msg01142.php
> 
> Here's my review of your patch.
> 
> The patch is in context diff format and applies cleanly to HEAD. It doesn't
> contain superfluous whitespace changes any more is and quite readable.
> 
> First, the behaviour.
> 
> The first problem I ran into when I tried to test this is that it *only*
> reports multiple errors during config file reload on SIHUP, not during
> postmaster startup. I guess it's been done that way because we
> ereport(ERROR,..) not ereport(LOG,...) during postmaster startup, so it's
> not immediatly clear how to report multiple errors. But that proplem
> seems solvable. What if you ereport(LOG,..)ed the individual errors during
> postmaster startup, and then emitted an ereport(ERROR) at the end if
> errors occurred? The ERROR could either repeat the first error that was
> encountered, or simply say "config file contains errors".

Makes sense. One problem I came across is that set_config_option from guc.c
sets the elevel by itself. I've changed it to emit LOG errors on PGC_S_FILE
source, apparently all the callers of this function with this source are from
guc-file.l, so hopefully I won't break anything with this change.


> 
> Now to the code.
> 
> I see that you basically replaced "goto cleanup..." in both ParseConfigFp()
> and ProcessConfigFile() with "++errorcount", and arranged for ParseConfigFp()
> to return false, and for ProcessConfigFile() to skip the GUC updates if
> "errorcount > 0". The actual value of errorcount is never inspected. The value
> is also wrong in the case of include files containing more than error, since
> ParseConfigFp() simply increments errorcount by one for each failed
> ParseConfigFile() of an included file.
> 
> I thus suggest that you replace "errorcount" with a boolean "erroroccurred".

I can actually pass errorcount down from the ParseConfigFp() to report the total
number of errors (w/ the include files) at the end of ProcessConfigFile if 
there is
any interest in having the number of errors reported to a user. If not - I'll 
change
it to boolean.

> 
> You've also introduced a bug in ParseConfigFp(). Previously, if an included
> file contained an error, it did "goto cleanup_exit()" and thus didn't
> ereport() on it's own. With your patch applied it ereport()s a parse error
> at the location of the "include" directive, which seems wrong.

Right, I noticed that I skipped switching the buffers and restoring the Lineno
as well. I'll fix it in the next revision.

> 
> Finally, I believe that ParseConfigFp() should make at least some effort to
> resync after hitting a parser error. I suggest that you simply fast-forward
> to the next GUC_EOL token after reporting a parser error.

Makes sense. 

Thank you for the review.

I'll post an updated patch, addressing you concerns, shortly.

Alexey.
--
Command Prompt, Inc.  http://www.CommandPrompt.com
PostgreSQL Replication, Consulting, Custom Development, 24x7 support




-- 
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] pg_upgrade using appname to lock out other users

[Christopher Browne]

On Thu, Jun 16, 2011 at 1:48 PM, Bruce Momjian  wrote:
> Tom Lane wrote:
>> "Ross J. Reedstrom"  writes:
>> > As an operations guy, the idea of an upgrade using a random,
>> > non-repeatable port selection gives me the hebejeebees.
>>
>> Yeah, I agree.  The latest version of the patch doesn't appear to have
>> any random component to it, though --- it just expects the user to
>> provide port numbers as switches.
>
> Oh, you wanted pg_upgrade to pick a random port number?  I can do that,
> but how would it check to see it is unused?

If no port is specified, that *might* be a reasonable behavior, but it
certainly throws in a dose of the wrong sort of nondeterminism, hence
heebie-jeebies...
-- 
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"

-- 
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 - Debug builds without optimization

[Greg Smith]

On 06/16/2011 10:10 AM, Tom Lane wrote:

I could see providing some other nonstandard configure switch that
changed the default -O level ... but realistically, would that do
anything that you couldn't already do by setting CFLAGS, ie

./configure CFLAGS="-O0 -g"
   


I think a small discussion of the issue Radek ran into is appropriate to 
put somewhere, with this example.  The install procedure section of the 
docs already includes a CFLAGS example:


./configure CC=/opt/bin/gcc CFLAGS='-O2 -pipe'

There is also a section talking about setting options like 
--enable-cassert in the Developer's FAQ.  Looking at all the info out 
there about developer/debug builds, it's really kind of sketchy and 
distributed though.  No one place that pulls all the most common things 
people need together into one resource.


What seems like the idea solution here is to add a new section to the 
install procedure with brief coverage of this entire area.  Here's a 
prototype of text that might go there:


= Installation for development and debugging =

When modifying the PostgreSQL source code, or when trying to find the 
source of a bug in the program, it may be helpful to build the program 
in a way that makes this process easier.  There are build-time only 
changes that enable better error checking and debugging, including:


Pass --enable-cassert to configure. This can make bugs more visible, 
because they cause operations to abort with a clear error.  That makes 
some types of debugging much easier.  This is risky on a production 
server, as described in the documentation for this parameter.


Pass --enable-debug to configure. This provides better information about 
what the server is doing when looking at it using a debugger.  It's less 
risky to a production server than enabling assertions, and it normally 
has less of a performance impact hgtoo.  See its documentation for more 
details.


Disable compiler optimization.  When using a debugger to trace into the 
source code of the server, steps may optimized away by the normal build 
process.  In some situations --enable-debug will disable such 
optimization, but this is not always the case.  Specifically disabling 
optimization is possible with many compilers by setting the compiler 
flags when configuration the source code build, such as:


./configure CFLAGS="-O0 -g"

This example for the gcc compiler disables optimizations, and tells the 
compiler to provide extra debugging information most useful with the gdb 
debugger.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us



--
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] Re: Latch implementation that wakes on postmaster death on both win32 and Unix

[Florian Pflug]

On May26, 2011, at 11:25 , Peter Geoghegan wrote:
> I'm a bit disappointed that no one has commented on this yet. I would
> have appreciated some preliminary feedback.

I noticed to your patch doesn't seem to register a SIGIO handler, i.e.
it doesn't use async IO machinery (or rather a tiny part thereof) to
get asynchronously notified if the postmaster dies. 

If that is on purpose, you can remove the fsetown() call, as it serves
no purpose without such a handler I think. Or, you might want to add
such a signal handler, and make it simply do "kill(getpid(), SIGTERM)".

best regards,
Florian Pflug


-- 
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] Latch implementation that wakes on postmaster death on both win32 and Unix

[Peter Geoghegan]

On 16 June 2011 15:27, Heikki Linnakangas
 wrote:

> I don't understand that comment. Why can't e.g postmaster death happen at
> the same time as a latch is set? I think the code is fine as it is, we just
> need to document that if there are several events that would wake up
> WaitLatch(), we make no promise that we return all of them in the return
> value. I believe all the callers would be fine with that.

I see your perspective...there is a window for the Postmaster to die
after the latch is set, but before it returns control to clients, and
this won't be reported. I would argue that Postmaster death didn't
actually coincide with the latch being set, because it didn't actually
cause the select() to unblock, and therefore we don't have a
responsibility to report it. Even if that view doesn't stand up to
scrutiny, and it may not, it is a fairly academic point, because, as
you say, it's unlikely that clients will ever much care. I'd be happy
to document that we make no promises, on the off chance that some
future caller might care.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and 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] On-the-fly index tuple deletion vs. hot_standby

[Noah Misch]

On Thu, Jun 16, 2011 at 12:02:47AM +0100, Simon Riggs wrote:
> On Tue, Jun 14, 2011 at 5:28 AM, Noah Misch  wrote:
> > On Mon, Jun 13, 2011 at 04:16:06PM +0100, Simon Riggs wrote:
> >> On Mon, Jun 13, 2011 at 3:11 AM, Robert Haas  wrote:
> >> > On Sun, Jun 12, 2011 at 3:01 PM, Noah Misch  wrote:
> >> >> Assuming that conclusion, I do think it's worth starting
> >> >> with something simple, even if it means additional bloat on the master 
> >> >> in the
> >> >> wal_level=hot_standby + vacuum_defer_cleanup_age / hot_standby_feedback 
> >> >> case.
> >> >> In choosing those settings, the administrator has taken constructive 
> >> >> steps to
> >> >> accept master-side bloat in exchange for delaying recovery conflict. 
> >> >> ?What's
> >> >> your opinion?
> >> >
> >> > I'm pretty disinclined to go tinkering with 9.1 at this point, too.
> >>
> >> Not least because a feature already exists in 9.1 to cope with this
> >> problem: hot standby feedback.
> >
> > A standby's receipt of an XLOG_BTREE_REUSE_PAGE record implies that the
> > accompanying latestRemovedXid preceded or equaled the master's RecentXmin 
> > at the
> > time of issue (see _bt_page_recyclable()). ?Neither hot_standby_feedback nor
> > vacuum_defer_cleanup_age affect RecentXmin. ?Therefore, neither facility 
> > delays
> > conflicts arising directly from B-tree page reuse. ?See attached test 
> > script,
> > which yields a snapshot conflict despite active hot_standby_feedback.
> 
> OK, agreed. Bug. Good catch, Noah.
> 
> Fix is to use RecentGlobalXmin for the cutoff when in Hot Standby
> mode, so that it is under user control.
> 
> Attached patch will be applied to head and backpatched to 9.1 and 9.0
> to fix this.

Thanks.  We still hit a conflict when btpo.xact == RecentGlobalXmin and the
standby has a transaction older than any master transaction.  This happens
because the tests at nbtpage.c:704 and procarray.c:1843 both pass when the xid
exactly is that of the oldest standby transaction (line numbers as of git
cb94db91b).  I only know this because the test script from my last message hits
this case; it might never get hit in real usage.  Still, seems like a hole not
worth leaving.  I think the most-correct fix is to TransactionIdRetreat the
btpo.xact before using it as xl_btree_reuse_page.lastestRemovedXid.  btpo.xact
is the first known-safe xid, but latestRemovedXid is the last known-unsafe xmin.

nm

-- 
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 - Debug builds without optimization

[Florian Pflug]

On Jun16, 2011, at 16:10 , Tom Lane wrote:
> Florian Pflug  writes:
>> I usually use -O1 for debug builds, these are usually still at least
>> somewhat debuggable with gdb.
> 
> I tend to do that too, but I still think that folding it into
> --enable-debug would be a mistake.

+1.

I didn't mean to suggest we fold -O1 into --enable-debug, I
was just handling out advice to the OP ;-)

best regards,
Florian Pflug


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