Re: [HACKERS] (re)start in our init scripts seems broken

2016-07-19 Thread Michael Paquier
On Wed, Jul 20, 2016 at 11:41 AM, Tomas Vondra
 wrote:
> Is there a reason why it's coded like this? I think we should use the pg_ctl
> instead or (at the very least) check the postmaster return code. Also,
> perhaps we should add an explicit timeout, higher than 60 seconds.

c8196c87 is one reason. Honestly, I have always found that using
pg_ctl start -w is more robust in such scripts, and it avoids
maintaining sanity checks that are duplicates of the ones in pg_ctl
after the postmaster has started. So +1 for using that. Passing the
PG_OOM_* flags is not an issue either.
-- 
Michael


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


Re: [HACKERS] plperl loading files

2016-07-19 Thread Peter Eisentraut
On 7/16/16 5:03 PM, Jeff Janes wrote:
> If you really want to suppress this per-backend activity, you can
> pre-emptively load the modules yourself with something like:
> 
> plperl.on_init ='require Carp; require Carp::Heavy; require feature;'
> 
> But, I don't see why that should be necessary.  I'd like to just put
> plperl into shared_preload_libraries and be done with it.
> 
> Am I missing something here?

I don't think there is any particular reason for this.  Changes can
probably be made, but backward compatibility needs to be taken into account.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-19 Thread Amit Kapila
On Wed, Jul 20, 2016 at 7:57 AM, Andres Freund  wrote:
>
>
> On July 19, 2016 7:14:42 PM PDT, Amit Kapila  wrote:
>>On Wed, Jul 20, 2016 at 5:02 AM, Andres Freund 
>>wrote:
>>> On 2016-07-19 18:09:59 -0500, Kevin Grittner wrote:
 As far as I can see, to do this the way that Andres and Amit
 suggest involves tying in to indexam.c and other code in incredibly
 ugly ways.
>>>
>>> Could you explain the problem you're seing?
>>>
>>> Isn't pretty much all all that we need to do:
>>> 1) add a InitSnapshotToast(Snapshot originMVCCSnap), which sets
>>SnapshotData->lsn
>>>to the the origin snapshot's lsn
>>> 2) adapt TestForOldSnapshot() to accept both HeapTupleSatisfiesMVCC
>>and
>>>HeapTupleSatisfiesToast?
>>>
>>
>>I also think so.  However, it is not clear what is the best place to
>>initialize toast snapshot.  One idea could be to do it in
>>GetSnapshotData() after capturing the required information for the
>>valid value of old_snapshot_threshold.  Do you have something else in
>>mind?
>
> There's very few callsites using toast snapshots. I'd just do it there. Don't 
> think we ever use GetSnapshotData for them.
>

I think Snapshot's members whenTaken and lsn are updated/initialized
only in GetSnapshotData().  So if GetSnapshotData() is not used, how
will you expect those fields to be updated.  We need those fields to
be updated for TestForOldSnapshot().

-- 
With Regards,
Amit Kapila.
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] (re)start in our init scripts seems broken

2016-07-19 Thread Tomas Vondra

Hi,

A few days ago I ran into a problem with the init script packaged in our 
community RPM packages. What happened was that they initiated a restart, 
but this happened:


# /etc/init.d/postgresql-9.3 restart
Stopping postgresql-9.3 service:   [FAILED]
Starting postgresql-9.3 service:   [  OK  ]

The database was however still in the shutdown mode, performing a 
checkpoint. Sadly the init script uses default timeout, so the stop 
terminates after just 60 seconds. But that seems fine, as the init 
script reports the failure correctly.


However the start action then seemingly succeeds, because it does this:

echo -n "$PSQL_START"
$SU -l postgres -c "$PGENGINE/postmaster -D '$PGDATA' ${PGOPTS} &" 
>> "$PGLOG" 2>&1 < /dev/null

sleep 2
pid=`head -n 1 "$PGDATA/postmaster.pid" 2>/dev/null`
if [ "x$pid" != x ]
then
success "$PSQL_START"
touch "$lockfile"
echo $pid > "$pidfile"
echo
else
failure "$PSQL_START"
echo
script_result=1
fi

It simply attempts to start the postmaster directly (instead of using 
pg_ctl), does not check the return code and instead proceeds to check 
the postmaster.pid file and existence of the process.


This however fails to do the trick, because the database is still 
running (in shutdown), so the postmaster does not overwrite the file. 
And of course the PID still matches a running process.


Is there a reason why it's coded like this? I think we should use the 
pg_ctl instead or (at the very least) check the postmaster return code. 
Also, perhaps we should add an explicit timeout, higher than 60 seconds.


regards

--
Tomas Vondra  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-19 Thread Andres Freund


On July 19, 2016 7:14:42 PM PDT, Amit Kapila  wrote:
>On Wed, Jul 20, 2016 at 5:02 AM, Andres Freund 
>wrote:
>> On 2016-07-19 18:09:59 -0500, Kevin Grittner wrote:
>>> As far as I can see, to do this the way that Andres and Amit
>>> suggest involves tying in to indexam.c and other code in incredibly
>>> ugly ways.
>>
>> Could you explain the problem you're seing?
>>
>> Isn't pretty much all all that we need to do:
>> 1) add a InitSnapshotToast(Snapshot originMVCCSnap), which sets
>SnapshotData->lsn
>>to the the origin snapshot's lsn
>> 2) adapt TestForOldSnapshot() to accept both HeapTupleSatisfiesMVCC
>and
>>HeapTupleSatisfiesToast?
>>
>
>I also think so.  However, it is not clear what is the best place to
>initialize toast snapshot.  One idea could be to do it in
>GetSnapshotData() after capturing the required information for the
>valid value of old_snapshot_threshold.  Do you have something else in
>mind?

There's very few callsites using toast snapshots. I'd just do it there. Don't 
think we ever use GetSnapshotData for them.

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


-- 
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: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-19 Thread Amit Kapila
On Wed, Jul 20, 2016 at 5:02 AM, Andres Freund  wrote:
> On 2016-07-19 18:09:59 -0500, Kevin Grittner wrote:
>> As far as I can see, to do this the way that Andres and Amit
>> suggest involves tying in to indexam.c and other code in incredibly
>> ugly ways.
>
> Could you explain the problem you're seing?
>
> Isn't pretty much all all that we need to do:
> 1) add a InitSnapshotToast(Snapshot originMVCCSnap), which sets 
> SnapshotData->lsn
>to the the origin snapshot's lsn
> 2) adapt TestForOldSnapshot() to accept both HeapTupleSatisfiesMVCC and
>HeapTupleSatisfiesToast?
>

I also think so.  However, it is not clear what is the best place to
initialize toast snapshot.  One idea could be to do it in
GetSnapshotData() after capturing the required information for the
valid value of old_snapshot_threshold.  Do you have something else in
mind?


-- 
With Regards,
Amit Kapila.
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] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-19 Thread Noah Misch
On Tue, Jul 19, 2016 at 06:09:59PM -0500, Kevin Grittner wrote:
> On Mon, Jul 18, 2016 at 9:10 PM, Noah Misch  wrote:
> > On Sat, Jul 16, 2016 at 06:48:08PM -0400, Noah Misch wrote:
> >> This PostgreSQL 9.6 open item is past due for your status update.  Kindly 
> >> send
> >> a status update within 24 hours, and include a date for your subsequent 
> >> status
> >> update.  Refer to the policy on open item ownership:
> >> http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com
> >
> > IMMEDIATE ATTENTION REQUIRED.  This PostgreSQL 9.6 open item is long past 
> > due
> > for your status update.  Please reacquaint yourself with the policy on open
> > item ownership[1] and then reply immediately.  If I do not hear from you by
> > 2016-07-20 03:00 UTC, I will transfer this item to release management team
> > ownership without further notice.
> >
> > [1] 
> > http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com
> 
> As far as I can see, to do this the way that Andres and Amit
> suggest involves tying in to indexam.c and other code in incredibly
> ugly ways.  I think it is entirely the wrong way to go, as I can't
> find a way to make it look remotely sane.  The question is whether
> I should do it the way that I think is sane, or whether someone
> else wants to show me what I'm missing by producing at least a
> rough patch along these lines.

This does not qualify as a status update, because it does not include a date
for your subsequent status update.


-- 
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] Question about an inconsistency - 3

2016-07-19 Thread Tom Lane
"pet...@gmail.com"  writes:
> In file postgres/postgresql-9.4.4/src/timezone/zic.c
> function stringzone line 2091we have

> if (stringrule(result, stdrp, dstrp->r_stdoff, zp->z_gmtoff) != 0)

> Is it ok to have as the 3rd argument dstrp->r_stdoff or should we
> have stdrp->r_stdoff? In line 2085 dstrp is used in both arguments.

Doubt it, because if the coding were like that there would be no need
to pass the third argument separately at all; the subroutine could just
access rp->r_stdoff for itself.  But this isn't our code, so if you
want an authoritative answer you should ask at
https://mm.icann.org/mailman/listinfo/tz

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] Question about an inconsistency - 2

2016-07-19 Thread Tom Lane
"pet...@gmail.com"  writes:
> In file postgresql-9.4.4/src/backend/utils/adt/format_type.c
> function format_type_internal line 159, shouldn’t be used
> array_base_type instead of type_oid?

IIRC, that was intentional.  Supposing there's a pg_type row with
a corrupted typelem value, it's more useful to point at the bogus
row than to report the junk value with no context.  I suppose you
could argue that it should be more like

elog(ERROR, "cache lookup failed for type %u (typelem of %u)",
 array_base_type, type_oid);

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] [BUG] pg_basebackup from disconnected standby fails

2016-07-19 Thread Michael Paquier
On Tue, Jul 19, 2016 at 9:08 PM, Amit Kapila  wrote:
> On Tue, Jul 19, 2016 at 10:43 AM, Michael Paquier
>  wrote:
>> On Sat, Jul 16, 2016 at 9:20 PM, Amit Kapila  wrote:
>>> On Wed, Jul 13, 2016 at 8:56 AM, Michael Paquier
>>>  wrote:
 Another way that just popped into my mind is to add dedicated fields
 to XLogCtl that set the stop LSN of a backup the way it should be
 instead of using minRecoveryPoint. In short we'd update those fields
 in CreateRestartPoint and UpdateMinRecoveryPoint under
 XLogCtl->info_lck. The good thing is that this lock is already taken
 there. See patch (2) accomplishing that.
>>>
>>> How is it different/preferable then directly using
>>> XLogCtl->replayEndRecPtr and XLogCtl->replayEndTLI for stop backup
>>> purpose?  Do you see any problem if we go with what Kyotaro-san has
>>> proposed in the initial patch [1] (aka using
>>> XLogCtl->lastReplayedEndRecPtr and XLogCtl->lastReplayedTLI as stop
>>> backup location)?
>>
>> Re-reading this thread from scratch and scratching my mind, I am
>> actually not getting why we bumped into the topic of making
>> minRecoveryPoint updates more aggressive instead of the first proposal
>> :)
>>
>> Knowing that we have no way to be sure if pg_control has been backed
>> up last or not, using the last replay LSN and TLI would be the most
>> simple solution, so let's do this for back-branches.
>>
>
> Why only for back-branches? Do you have better solution for head?

Yes, I mentioned an idea upthread to set up the minimum recovery point
saved in the backup to the last replayed LSN. Though that's not
acceptable for 9.6 as this requires changing the output of
pg_stop_backup() with a new field containing the bytes of pg_control.
I am not sure how others feel about that, so for now and the
back-branches we could just document that updating the field after
taking a backup closes any holes, and prevents to open a hot standby
for read-only connections too early.

>> It is an
>> annoyance to not be able to ensure that backups are taken while the
>> master is stopped or if there is no activity that updates relation
>> pages.
>>
>> The thing that is really annoying btw is that there will be always a
>> gap between minRecoveryPoint and the actual moment where a backup
>> finishes because there is no way to rely on the XLOG_BACKUP_END
>> record.
>
> Sorry, but I am not able to understand what you mean by above.  What
> kind of relation you are trying to show between minRecoveryPoint and
> backup finish point?

When taking a backup from a standby, there is no XLOG_BACKUP_END that
can be used to determine when a hot standby is open for read-only
connections.
-- 
Michael


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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-19 Thread Andres Freund
On 2016-07-19 18:09:59 -0500, Kevin Grittner wrote:
> As far as I can see, to do this the way that Andres and Amit
> suggest involves tying in to indexam.c and other code in incredibly
> ugly ways.

Could you explain the problem you're seing?

Isn't pretty much all all that we need to do:
1) add a InitSnapshotToast(Snapshot originMVCCSnap), which sets 
SnapshotData->lsn
   to the the origin snapshot's lsn
2) adapt TestForOldSnapshot() to accept both HeapTupleSatisfiesMVCC and
   HeapTupleSatisfiesToast?

I mean the only difference between toast / plain heap table WRT
old_snapshot_threshold is that we don't use a mvcc snapshot.

> I think it is entirely the wrong way to go, as I can't
> find a way to make it look remotely sane.  The question is whether
> I should do it the way that I think is sane, or whether someone
> else wants to show me what I'm missing by producing at least a
> rough patch along these lines.

I'll, but I'd prefer you explaining the problem first. Maybe it's me
missing the obvious problem.

Andres


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


[HACKERS] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-19 Thread Kevin Grittner
On Mon, Jul 18, 2016 at 9:10 PM, Noah Misch  wrote:
> On Sat, Jul 16, 2016 at 06:48:08PM -0400, Noah Misch wrote:
>> On Wed, Jul 13, 2016 at 03:57:02PM -0500, Kevin Grittner wrote:
>>> On Wed, Jul 13, 2016 at 12:48 PM, Andres Freund  wrote:
 On 2016-07-13 10:06:52 -0500, Kevin Grittner wrote:
> On Wed, Jul 13, 2016 at 7:57 AM, Amit Kapila  
> wrote:
>> On Tue, Jul 12, 2016 at 8:34 PM, Kevin Grittner  
>> wrote:
>>> On Fri, Jul 8, 2016 at 1:52 PM, Andres Freund  
>>> wrote:
>>>
 I'm a bit confused, why aren't we simply adding LSN interlock
 checks for toast? Doesn't look that hard? Seems like a much more
 natural course of fixing this issue?
>>>
>>> I took some time trying to see what you have in mind, and I'm
>>> really not "getting it".
>>
>> Isn't it possible if we initialize lsn and whenTaken in SnapshotToast
>> when old_snapshot_threshold > 0 and add a check for
>> HeapTupleSatisfiesToast in TestForOldSnapshot()?
>
> With that approach, how will we know *not* to generate an error
> when reading the chain of tuples for a value we are deleting.  Or
> positioning to modify an index on toast data.  Etc., etc. etc.

 I'm not following. How is that different in the toast case than in the
 heap case?
>>>
>>> A short answer is that a normal table's heap doesn't go through
>>> systable_getnext_ordered().  That function is used both for cases
>>> where the check should not be made, like toast_delete_datum(), and
>>> cases where it should, like toast_fetch_datum().
>>>
>>> Since this keeps coming up, I'll produce a patch this way.  I'm
>>> skeptical, but maybe it will look better than I think it will.  I
>>> should be able to post that by Friday.
>>
>> This PostgreSQL 9.6 open item is past due for your status update.  Kindly 
>> send
>> a status update within 24 hours, and include a date for your subsequent 
>> status
>> update.  Refer to the policy on open item ownership:
>> http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com
>
> IMMEDIATE ATTENTION REQUIRED.  This PostgreSQL 9.6 open item is long past due
> for your status update.  Please reacquaint yourself with the policy on open
> item ownership[1] and then reply immediately.  If I do not hear from you by
> 2016-07-20 03:00 UTC, I will transfer this item to release management team
> ownership without further notice.
>
> [1] 
> http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com

As far as I can see, to do this the way that Andres and Amit
suggest involves tying in to indexam.c and other code in incredibly
ugly ways.  I think it is entirely the wrong way to go, as I can't
find a way to make it look remotely sane.  The question is whether
I should do it the way that I think is sane, or whether someone
else wants to show me what I'm missing by producing at least a
rough patch along these lines.

--
Kevin Grittner
EDB: 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] Document that vacuum can't truncate if old_snapshot_threshold >= 0

2016-07-19 Thread Kevin Grittner
On Wed, Jul 13, 2016 at 4:14 PM, Andres Freund  wrote:

> Currently, if old_snapshot_threshold is enabled, vacuum is prevented
> from truncating tables:
> static bool
> should_attempt_truncation(LVRelStats *vacrelstats)
> {
> BlockNumber possibly_freeable;
>
> possibly_freeable = vacrelstats->rel_pages - 
> vacrelstats->nonempty_pages;
> if (possibly_freeable > 0 &&
> (possibly_freeable >= REL_TRUNCATE_MINIMUM ||
>   possibly_freeable >= vacrelstats->rel_pages / 
> REL_TRUNCATE_FRACTION) &&
> old_snapshot_threshold < 0)
> return true;
> else
> return false;
> }
>
> (note the old_snapshot_threshold < 0 condition).
>
> That appears to not be mentioned in a comment, the commit message or the
> the docs. I think this definitely needs to be prominently documented.
>
> FWIW, afaics that's required because new pages don't have an LSN, so we
> can't necessarily detect that a truncated and re-extended relation,
> wouldn't be valid. Although I do wonder if there isn't a less invasive
> way to do that.

There would be no problem on re-extended pages because they would
have a new enough LSN to cause a "snapshot too old" error; it is
when they are missing that the problem exists.  The possible
alternative that looked best to me was to leave a "guard page" to
cover sequential scans, with LSN set to (at least) the latest of
itself or any truncated page.  I think WAL would need to be
generated to cover the LSN update.  If you combined that with
treating an index leaf tuple pointing to a missing page as
"snapshot too old" I think things would work, but it's not clear to
me that it's worth the complexity.

Attached pushed.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4e8c982..c9e0ec2 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -2092,6 +2092,15 @@ include_dir 'conf.d'
 
 
 
+ When this feature is enabled, freed space at the end of a relation
+ cannot be released to the operating system, since that could remove
+ information needed to detect the snapshot too old
+ condition.  All space allocated to a relation remains associated with
+ that relation for reuse only within that relation unless explicitly
+ freed (for example, with VACUUM FULL).
+
+
+
  This setting does not attempt to guarantee that an error will be
  generated under any particular circumstances.  In fact, if the
  correct results can be generated from (for example) a cursor which
diff --git a/src/backend/commands/vacuumlazy.c 
b/src/backend/commands/vacuumlazy.c
index 4075f4d..231e92d 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -1663,6 +1663,15 @@ lazy_cleanup_index(Relation indrel,
  * Don't even think about it unless we have a shot at releasing a goodly
  * number of pages.  Otherwise, the time taken isn't worth it.
  *
+ * Also don't attempt it if we are doing early pruning/vacuuming, because a
+ * scan which cannot find a truncated heap page cannot determine that the
+ * snapshot is too old to read that page.  We might be able to get away with
+ * truncating all except one of the pages, setting its LSN to (at least) the
+ * maximum of the truncated range if we also treated an index leaf tuple
+ * pointing to a missing heap page as something to trigger the "snapshot too
+ * old" error, but that seems fragile and seems like it deserves its own patch
+ * if we consider it.
+ *
  * This is split out so that we can test whether truncation is going to be
  * called for before we actually do it.  If you change the logic here, be
  * careful to depend only on fields that lazy_scan_heap updates on-the-fly.

-- 
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] An unlikely() experiment

2016-07-19 Thread Andres Freund
On 2015-12-20 14:21:14 +1300, David Rowley wrote:
> On 20 December 2015 at 03:06, Andres Freund  wrote:
> > One way to do this would be to add elog_on() / ereport_on() macros,
> > directly containing the error message. Like
> > #define elog_on(cond, elevel, ...) \
> > do { \
> >if (unlikely(cond)) \
> >{ \
> > elog(elevel, __VA_ARGS__) \
> >} \
> > } while(0)
> >
> 
> Interesting idea. Would you think that would be something we could do a
> complete replace on, or are you thinking just for the hotter code paths?

More or less complete. Generally, logging shouldn't be a hot code
path. A single wrong branch won't be noticeable in case we're logging
something, not to speak of an actual error case.  As far as I can see
there's unfortunately no way to declare a branch unlikely from inside
that branch, otherwise I'd have said we should just stick an unlikely
equivalent in the elog/ereport definition itself.

- Andres


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


[HACKERS] Question about an inconsistency - 2

2016-07-19 Thread pet...@gmail.com
Hi,

In file postgresql-9.4.4/src/backend/utils/adt/format_type.c
function format_type_internal line 159, shouldn’t be used
array_base_type instead of type_oid?

In line 153 it is searched for array_base_type and thus
shouldn’t we use it (ie., array_base_type) to report that the
type is not found. It is similar to the case of searching for
type_oid in lines 129-135.

Thanks,
Petru Florin Mihancea




signature.asc
Description: Message signed with OpenPGP using GPGMail


[HACKERS] Question about an inconsistency - 3

2016-07-19 Thread pet...@gmail.com

Hi,

In file postgres/postgresql-9.4.4/src/timezone/zic.c
function stringzone line 2091we have

if (stringrule(result, stdrp, dstrp->r_stdoff, zp->z_gmtoff) != 0)

Is it ok to have as the 3rd argument dstrp->r_stdoff or should we
have stdrp->r_stdoff? In line 2085 dstrp is used in both arguments.

Not very sure because I do not understand the semantics of the
invoked operation, but I thought it would be better to ask.

Thanks,
Petru Florin Mihancea


signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: [HACKERS] sslmode=require fallback

2016-07-19 Thread Peter Eisentraut
On 7/19/16 3:32 PM, Magnus Hagander wrote:
> There are definitely cases where it's useful. I'm only arguing for
> changing the default. 

I don't understand why you want to change the default.  Is it for
performance?  Has it been measured?

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] Adjust recovery test file name

2016-07-19 Thread Alvaro Herrera
Masahiko Sawada wrote:
> Hi all,
> 
> The file 006_logical_decoding_timelines.pl was removed by the commit c1543a8.
> But currently 005_***.pl and 007_***.pl exist on source tree.
> Should we change its file number to 006?

I don't think we need to bother about this.  Whenever somebody submits a
new test script they can fill the 006 gap.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] sslmode=require fallback

2016-07-19 Thread Magnus Hagander
On Tue, Jul 19, 2016 at 9:24 PM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:

> On 7/19/16 10:00 AM, Magnus Hagander wrote:
> > What could actually be useful there is to explicitly put hostnossl on
> > the localhost entries. With the current defaults on the clients, that
> > wouldn't break anything, and it would leave people without the
> > performance issues that you run into in the default deployments. And for
> > localhost it really does't make sense to encrypt -- for the local LAN
> > segment that can be argued, but for localhost...
>
> But even on localhost you ideally want a way to confirm that the server
> you are connecting to is the right one, so you might want certificates.
> Plus the server might want certificates from the clients.  (See also the
> occasional discussion about supporting SSL over Unix-domain sockets.)
>
>
There are definitely cases where it's useful. I'm only arguing for changing
the default.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] sslmode=require fallback

2016-07-19 Thread Peter Eisentraut
On 7/19/16 10:00 AM, Magnus Hagander wrote:
> What could actually be useful there is to explicitly put hostnossl on
> the localhost entries. With the current defaults on the clients, that
> wouldn't break anything, and it would leave people without the
> performance issues that you run into in the default deployments. And for
> localhost it really does't make sense to encrypt -- for the local LAN
> segment that can be argued, but for localhost...

But even on localhost you ideally want a way to confirm that the server
you are connecting to is the right one, so you might want certificates.
Plus the server might want certificates from the clients.  (See also the
occasional discussion about supporting SSL over Unix-domain sockets.)

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] sslmode=require fallback

2016-07-19 Thread Magnus Hagander
On Tue, Jul 19, 2016 at 8:53 PM, Christoph Berg  wrote:

> Makes sense. Is this something that should be implemented in postgresql,
> or via pg_createcluster?
>
>
Personally I'd like to see pg_createcluster et al mimic upstream as close
as possible, so I'd advocate these changes being made upstream in
PostgreSQL iteslf.

//Magnus




>
> Am 19. Juli 2016 16:00:05 MESZ, schrieb Magnus Hagander <
> mag...@hagander.net>:
>>
>>
>>
>> On Sun, Jul 17, 2016 at 10:07 PM, Christoph Berg  wrote:
>>
>>> Re: Peter Eisentraut 2016-07-17 <
>>> d6b22200-0e65-d17e-b227-b63d81720...@2ndquadrant.com>
>>> > On 7/15/16 3:07 PM, Andrew Dunstan wrote:
>>> > > Do those packagers who install dummy certificates and turn SSL on
>>> also
>>> > > change their pg_hba.conf.sample files to use hostssl?. That could go
>>> a
>>> > > long way towards encouraging people.
>>> >
>>> > Debian, which I guess sort of started this, does not, but there are
>>> > allusions to it in the TODO list.
>>>
>>> I guess we should actually do that if we had any non-local(host)
>>> entries in there by default, but we don't touch the default
>>> pg_hba.conf from pg_createcluster.
>>>
>>
>> What could actually be useful there is to explicitly put hostnossl on the
>> localhost entries. With the current defaults on the clients, that wouldn't
>> break anything, and it would leave people without the performance issues
>> that you run into in the default deployments. And for localhost it really
>> does't make sense to encrypt -- for the local LAN segment that can be
>> argued, but for localhost...
>>
>>
>> --
>>  Magnus Hagander
>>  Me: http://www.hagander.net/
>>  Work: http://www.redpill-linpro.com/
>>
>


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] sslmode=require fallback

2016-07-19 Thread Christoph Berg
Makes sense. Is this something that should be implemented in postgresql, or via 
pg_createcluster?

Am 19. Juli 2016 16:00:05 MESZ, schrieb Magnus Hagander :
>On Sun, Jul 17, 2016 at 10:07 PM, Christoph Berg 
>wrote:
>
>> Re: Peter Eisentraut 2016-07-17 <
>> d6b22200-0e65-d17e-b227-b63d81720...@2ndquadrant.com>
>> > On 7/15/16 3:07 PM, Andrew Dunstan wrote:
>> > > Do those packagers who install dummy certificates and turn SSL on
>also
>> > > change their pg_hba.conf.sample files to use hostssl?. That could
>go a
>> > > long way towards encouraging people.
>> >
>> > Debian, which I guess sort of started this, does not, but there are
>> > allusions to it in the TODO list.
>>
>> I guess we should actually do that if we had any non-local(host)
>> entries in there by default, but we don't touch the default
>> pg_hba.conf from pg_createcluster.
>>
>
>What could actually be useful there is to explicitly put hostnossl on
>the
>localhost entries. With the current defaults on the clients, that
>wouldn't
>break anything, and it would leave people without the performance
>issues
>that you run into in the default deployments. And for localhost it
>really
>does't make sense to encrypt -- for the local LAN segment that can be
>argued, but for localhost...
>
>
>-- 
> Magnus Hagander
> Me: http://www.hagander.net/
> Work: http://www.redpill-linpro.com/


Re: [HACKERS] One process per session lack of sharing

2016-07-19 Thread Andres Freund
On 2016-07-19 14:18:22 +0300, amatv...@bitec.ru wrote:
> Hi
> 
> 
> > Using TLS will slow down things noticeably though. So if we were to go
> > there, we'd have to make up for some constant slowdown.
> I can not understand why?
> 
> I've read
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms686749(v=vs.85).aspx
> and
> http://david-grs.github.io/tls_performance_overhead_cost_linux/
> """
> The results are quite straightforward: no overhead at all.
> """
> 
>  0x00404f40 <+0>: incDWORD PTR [rip+0x202382]
>  vs
>  0x00404f50 <+0>: incDWORD PTR fs:0xfffc

Not really true IIRC. For one segment offset stuff is encoded more
widely, and for another, it'll generate more uops in many
microarchitectures.  Also, we actually *do* qualify for the exception in
the blog you linked above: We have a fair amount of dynamically linked
code.


-- 
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] One process per session lack of sharing

2016-07-19 Thread Robert Haas
On Mon, Jul 18, 2016 at 8:56 PM, Craig Ringer  wrote:
> Since I got started with Pg, I've taken it as given that PostgreSQL Will
> Never Use Threads, Don't Even Talk About It. As taboo as query hints or more
> so. Is this actually a serious option?

I'm sure that depends on who you ask.  But for myself, yes, I think
it's a serious option.  I think that the sort of minimal conversion
that I discussed above could be done and made stable enough to label
as "we have this experimental option..." by one competent developer in
the course of one release cycle without otherwise unduly disrupting
development.  From that base, we could consider patches to optimize
the thread model case, and maybe after gaining some experience and
letting it shake out for a release or two we'd decide that the thread
model is ready to be officially supported.  I bet there would be a lot
of interest in the thread model from the user and developer
communities.  It would probably be a significant win on Windows -
where I understand that the ratio of process creation cost : thread
creation cost is much worse than it is on Linux - and it would
probably open up numerous possible optimizations for parallel query.
It would probably also have some downsides and likely some horrible
bugs, but that's why you start it out as an experimental feature.

In short, I believe the conventional wisdom on this topic is
misguided.  Most of the previous discussions of using threading have
assumed that we'd go through all of the backend-private stuff and make
it thread-safe.  That's a bad plan, first because it's not very
well-defined, second because it could slow down parts of the system
that rely on the absence of synchronization primitives in certain code
paths, and third because it requires a single mammoth act of
development on a scale that would be extremely hard to make
successful.  However, the method that I'm proposing is a completely
different kettle of fish.  It is not, as Tom points out, entirely
without danger, but it allows the first patch to be a mostly
mechanical transformation and then allows incremental development on
top of that framework.  For that reason, I believe it's at least an
order of magnitude less impractical than the "go through and make
everything thread-safe" approach.  Unlike that approach, it also makes
it realistically possible to support both models.

None of that means that it will necessarily work out, but I'm bullish.
Even though parallel query has already had more bug reports than I
would have liked, it's good evidence that you can make architectural
changes that touch almost every part of the system without necessarily
breaking everything.  You just have to go slow, be methodical, and
budget time to fix the bugs.

-- 
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] Updating our timezone code in the back branches

2016-07-19 Thread Tom Lane
Greg Stark  writes:
> On Mon, Jul 18, 2016 at 8:09 PM, Tom Lane  wrote:
>> There are also several bug fixes that affect interpretation of dates after
>> 2037, a year that's getting closer all the time.

> Does this represent a data incompatibility for databases that could
> contain such dates already? That is, would this be changing the dates
> their database contains?

Hard to say.  Those bugs might affect the way a stored timestamp would be
printed, but I don't really care to do the legwork that would be needed
to identify exactly what the consequences would be.  In practice, I doubt
that the effects would be much different from a change in DST law that
might happen between now and 2037 --- anybody who's predicting now what
their local DST laws will be by then is pretty far out on a limb anyway :-(

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] Updating our timezone code in the back branches

2016-07-19 Thread Tom Lane
Magnus Hagander  writes:
> On Mon, Jul 18, 2016 at 9:09 PM, Tom Lane  wrote:
>> So I think it behooves us to apply these changes to the back branches
>> while we can still do it in a leisurely fashion, rather than waiting
>> until our hands are forced.  I'd like to do it in the next week or so
>> so that we can get in a little bit of buildfarm and manual testing
>> before the next set of back-branch releases in August.

> This seems reasonable. Except the 2016e one which hasn't seen master yet.
> But it would definitely make sense to try to get 2016e into 9.6 ASAP, so we
> can get it into the next beta/rc (not the one on it's way now, but the one
> after that).

I think that the 2016e change is not actually relevant to us; it's just
introducing a no-op DST transition into the data files in 2037.  If our
version of zic were being used to produce zone files that Qt would read,
it might be important, but that seems pretty unlikely.  So we should sync
it eventually but I'm not in much of a hurry.

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] Updating our timezone code in the back branches

2016-07-19 Thread Greg Stark
On Mon, Jul 18, 2016 at 8:09 PM, Tom Lane  wrote:
> There are also several bug fixes that affect interpretation of dates after
> 2037, a year that's getting closer all the time.

Does this represent a data incompatibility for databases that could
contain such dates already? That is, would this be changing the dates
their database contains?

We don't really have a solution for that kind of change but we should
highlight it fairly prominently and it would be nice if we gave people
a query that would search for any affected dates in a column.

-- 
greg


-- 
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] Updating our timezone code in the back branches

2016-07-19 Thread Magnus Hagander
On Mon, Jul 18, 2016 at 9:09 PM, Tom Lane  wrote:

> When I updated our copy of the IANA timezone library back in March
> (commit 1c1a7cbd6), I noted that we ought to consider back-patching
> those changes once they'd settled out in HEAD.  Now that the code
> has survived a couple of months of beta testing, I think it is time
> to do that.
>
> I went through the IANA announcement archives
> http://mm.icann.org/pipermail/tz-announce/
> to get a summary of what's changed since the tzcode 2010c release
> that we last synced with.  Attached is a list of code changes that
> seem potentially significant to us.  There are at least two ticking
> time bombs in there, namely zic feature additions that IANA has not
> yet started to use in the tzdata files, but presumably will start
> using at some point, else why would they put them in?  (These are
> the extension to allow four DST transitions per year and the addition
> of %z escapes in TZ abbreviations.)  Whenever they do start using them,
> we'll be unable to apply tzdata updates to unpatched back branches,
> because our old copy of zic will fail to compile the tzdata files.
>
> There are also several bug fixes that affect interpretation of dates after
> 2037, a year that's getting closer all the time.  And there are some
> changes that affect reading of zic data files, which could affect
> unpatched back branches that are built with --with-system-tzdata,
> since those might be fed updated data files even if we do nothing.
>
> So I think it behooves us to apply these changes to the back branches
> while we can still do it in a leisurely fashion, rather than waiting
> until our hands are forced.  I'd like to do it in the next week or so
> so that we can get in a little bit of buildfarm and manual testing
> before the next set of back-branch releases in August.
>
> Thoughts, objections?
>


This seems reasonable. Except the 2016e one which hasn't seen master yet.
But it would definitely make sense to try to get 2016e into 9.6 ASAP, so we
can get it into the next beta/rc (not the one on it's way now, but the one
after that).


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] Partition-wise join for join between (declaratively) partitioned tables

2016-07-19 Thread Ashutosh Bapat
Sorry forgot to mention: this patch applies on top of the v7 patches posted
by Amit Langote on 27th June (
https://www.postgresql.org/message-id/81371428-bb4b-1e33-5ad6-8c5c51b52cb7%40lab.ntt.co.jp
).

On Tue, Jul 19, 2016 at 7:41 PM, Ashutosh Bapat <
ashutosh.ba...@enterprisedb.com> wrote:

>
>
> On Fri, Jul 8, 2016 at 12:11 AM, Robert Haas 
> wrote:
>
>>
>> I haven't reviewed this code yet due to being busy with 9.6, but I
>> think this is a very important query planner improvement with the
>> potential for big wins on queries involving large amounts of data.
>>
>> Suppose we have a pair of equi-partitioned tables.  Right now, if we
>> choose to perform a hash join, we'll have to build a giant hash table
>> with all of the rows from every inner partition and then probe it for
>> every row in every outer partition.  If there are few enough inner
>> rows that the resultant hash table still fits in work_mem, this is
>> somewhat inefficient but not terrible - but if it causes us to have to
>> batch the hash join where we otherwise would not need to do so, then
>> it really sucks.  Similarly, if we decide to merge-join each pair of
>> partitions, a partitionwise join may be able to use an internal sort
>> on some or all partitions whereas if we had to deal with all of the
>> data at the same time we'd need an external sort, possibly multi-pass.
>>
>
> Or we might be able to use indexes directly without need of a MergeAppend.
>
>
>>   And if we choose a nested loop, say over an inner index-scan, we do
>> O(outer rows) index probes with this optimization but O(outer rows *
>> inner partitions) index probes without it.
>>
>> In addition, parallel query can benefit significantly from this kind
>> of optimization.  Tom recently raised the case of an appendrel where
>> every child has a parallel-safe path but not every child has a partial
>> path; currently, we can't go parallel in that case, but it's easy to
>> see that we could handle it by scheduling the appendrel's children
>> across a pool of workers.  If we had this optimization, that sort of
>> thing would be much more likely to be useful, because it could create
>> appendrels where each member is an N-way join between equipartitioned
>> tables.  That's particularly important right now because of the
>> restriction that a partial path must be driven by a Parallel SeqScan,
>> but even after that restriction is lifted it's easy to imagine that
>> the effective degree of parallelism for a single index scan may be
>> limited - so this kind of thing may significantly increase the number
>> of workers that a given query can use productively.
>>
>
> +1.
>
> The attached patch implements the logic to assess whether two partitioned
> tables can be joined using partition-wise join technique described in my
> last
> mail on this thread.
>
> Two partitioned relations are considered for partition-wise join if
> following
> conditions are met (See build_joinrel_part_info() for details):
> 1. Both the partitions have same number of partitions, with same number of
> partition keys and partitioned by same strategy - range or list.
> 2. They have matching datatypes for partition keys (partkey_types_match())
> 3. For list partitioned relations, they have same lists for each pair of
> partitions, paired by position in which they appear.
> 4. For range partitioned relations, they have same bounds for each pair of
> partitions, paired by their position when ordered in ascending fashion on
> the
> upper bounds.
> 5. There exists an equi-join condition for each pair of partition keys,
> paired
> by the position in which they appear.
>
> Partition-wise join technique can be applied under more lenient
> constraints [1]
> e.g. joins between tables with different number of partitions but having
> same
> bounds/lists for the common partitions. I am planning to defer that to a
> later
> version of this feature.
>
> A join executed using partition-wise join technique is itself a relation
> partitioned by the similar partitioning scheme as the joining relations
> with
> the partition keys combined from the joining relations.
>
> A PartitionOptInfo (uses name similar to RelOptInfo or IndexOptInfo)
> structure
> is used to store the partitioning information for a given base or relation.
> In build_simple_rel(), we construct PartitionOptInfo structure for the
> given
> base relation by copying the relation's PartitionDesc and PartitionKey
> (structures from Amit Langote's patch). While doing so, all the partition
> keys
> are stored as expressions. The structure also holds the RelOptInfos of the
> partition relations. For a join relation, most of the PartitionOptInfo is
> copied from either of the joining relations, except the partition keys and
> RelOptInfo of partition relations. Partition keys of the join relations are
> created by combing partition keys from both the joining relations. The
> logic to
> cosnstruct RelOptInfo for the partition-wise join relations is 

Re: [HACKERS] Declarative partitioning

2016-07-19 Thread Ashutosh Bapat
I am seeing following warning with this set of patches.
gram.y:4734:24: warning: assignment from incompatible pointer type [enabled
by default]

On Tue, Jul 5, 2016 at 10:18 AM, Amit Langote  wrote:

> On 2016/07/04 21:31, Ashutosh Bapat wrote:
> > Hi Amit,
> > I observed that the ChangeVarNodes call at line 1229 in
> > get_relation_constraints() (code below) changes the varno in the cached
> > copy of partition key expression, which is undesirable. The reason for
> this
> > seems to be that the RelationGetPartitionCheckQual() returns the copy of
> > partition key expression directly from the cache. This is mostly because
> > get_check_qual_for_range() directly works on cached copy of partition key
> > expressions, which it should never.
>
> Yes, a copyObject() on key->partexprs items seems necessary. Will fix that.
>
> > 1223 /* Append partition check quals, if any */
> > 1224 pcqual = RelationGetPartitionCheckQual(relation);
> > 1225 if (pcqual)
> > 1226 {
> > 1227 /* Fix Vars to have the desired varno */
> > 1228 if (varno != 1)
> > 1229 ChangeVarNodes((Node *) pcqual, 1, varno, 0);
> > 1230
> > 1231 result = list_concat(result, pcqual);
> > 1232 }
> >
> > Because of this, the first time through the partition key expressions are
> > valid, but then onwards they are restamped with the varno of the first
> > partition.
> >
> > Please add testcases to your patch to catch such types of issues.
>
> I will integrate tests into the patch(es) and add some more.
>
> Thanks,
> Amit
>
>
>


-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


Re: [HACKERS] sslmode=require fallback

2016-07-19 Thread Magnus Hagander
On Sun, Jul 17, 2016 at 10:07 PM, Christoph Berg  wrote:

> Re: Peter Eisentraut 2016-07-17 <
> d6b22200-0e65-d17e-b227-b63d81720...@2ndquadrant.com>
> > On 7/15/16 3:07 PM, Andrew Dunstan wrote:
> > > Do those packagers who install dummy certificates and turn SSL on also
> > > change their pg_hba.conf.sample files to use hostssl?. That could go a
> > > long way towards encouraging people.
> >
> > Debian, which I guess sort of started this, does not, but there are
> > allusions to it in the TODO list.
>
> I guess we should actually do that if we had any non-local(host)
> entries in there by default, but we don't touch the default
> pg_hba.conf from pg_createcluster.
>

What could actually be useful there is to explicitly put hostnossl on the
localhost entries. With the current defaults on the clients, that wouldn't
break anything, and it would leave people without the performance issues
that you run into in the default deployments. And for localhost it really
does't make sense to encrypt -- for the local LAN segment that can be
argued, but for localhost...


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] [BUG] pg_basebackup from disconnected standby fails

2016-07-19 Thread Amit Kapila
On Tue, Jul 19, 2016 at 10:43 AM, Michael Paquier
 wrote:
> On Sat, Jul 16, 2016 at 9:20 PM, Amit Kapila  wrote:
>> On Wed, Jul 13, 2016 at 8:56 AM, Michael Paquier
>>  wrote:
>>> Another way that just popped into my mind is to add dedicated fields
>>> to XLogCtl that set the stop LSN of a backup the way it should be
>>> instead of using minRecoveryPoint. In short we'd update those fields
>>> in CreateRestartPoint and UpdateMinRecoveryPoint under
>>> XLogCtl->info_lck. The good thing is that this lock is already taken
>>> there. See patch (2) accomplishing that.
>>
>> How is it different/preferable then directly using
>> XLogCtl->replayEndRecPtr and XLogCtl->replayEndTLI for stop backup
>> purpose?  Do you see any problem if we go with what Kyotaro-san has
>> proposed in the initial patch [1] (aka using
>> XLogCtl->lastReplayedEndRecPtr and XLogCtl->lastReplayedTLI as stop
>> backup location)?
>
> Re-reading this thread from scratch and scratching my mind, I am
> actually not getting why we bumped into the topic of making
> minRecoveryPoint updates more aggressive instead of the first proposal
> :)
>
> Knowing that we have no way to be sure if pg_control has been backed
> up last or not, using the last replay LSN and TLI would be the most
> simple solution, so let's do this for back-branches.
>

Why only for back-branches? Do you have better solution for head?

> It is an
> annoyance to not be able to ensure that backups are taken while the
> master is stopped or if there is no activity that updates relation
> pages.
>
> The thing that is really annoying btw is that there will be always a
> gap between minRecoveryPoint and the actual moment where a backup
> finishes because there is no way to rely on the XLOG_BACKUP_END
> record.
>

Sorry, but I am not able to understand what you mean by above.  What
kind of relation you are trying to show between minRecoveryPoint and
backup finish point?


-- 
With Regards,
Amit Kapila.
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] One process per session lack of sharing

2016-07-19 Thread AMatveev
Hi


> Using TLS will slow down things noticeably though. So if we were to go
> there, we'd have to make up for some constant slowdown.
I can not understand why?

I've read
https://msdn.microsoft.com/en-us/library/windows/desktop/ms686749(v=vs.85).aspx
and
http://david-grs.github.io/tls_performance_overhead_cost_linux/
"""
The results are quite straightforward: no overhead at all.
"""

 0x00404f40 <+0>: incDWORD PTR [rip+0x202382]
 vs
 0x00404f50 <+0>: incDWORD PTR fs:0xfffc

It's clear.



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


[HACKERS] Adjust recovery test file name

2016-07-19 Thread Masahiko Sawada
Hi all,

The file 006_logical_decoding_timelines.pl was removed by the commit c1543a8.
But currently 005_***.pl and 007_***.pl exist on source tree.
Should we change its file number to 006?

Please find attached patch renames 007_sync_rep.pl to 006_sync_rep.pl.

Regards,

--
Masahiko Sawada


0001-Rename-file-number-of-sync_rep.pl.patch
Description: binary/octet-stream

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


[HACKERS] Any need of GRANT/REVOKE CREATE TABLE | POLICY | ETC

2016-07-19 Thread Haribabu Kommi
Hi All,

During the discussion of supporting multi tenancy with the help of
row level security, because of some problems of executing any
policy that was created by an unprivileged user [1].

To avoid that problem, If we have some kind of new mechanism to
GRANT/REVOKE only CREATE POLICY from all the unprivileged
users by providing other CREATE access to them.

I just want to know is there any other such requirements that if such
option is available, it would be good or not? I don't know whether
this option is possible or not? If any such requirements are present
other than POLICY, i would like to analyze and propose a patch for
the same.

[1] - https://www.postgresql.org/message-id/21902.1455052932%40sss.pgh.pa.us

Regards,
Hari Babu
Fujitsu Australia


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