Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Friday, August 02, 2013 4:34 AM Dimitri Fontaine wrote: > Andres Freund writes: > > They would need a setting that disables ALTER (DATABASE|USER) ... SET > > ... as well though. At least for some settings. > > > > I don't think enforcing things on that level makes much sense. > > If only we could trigger some actions when a command is about to be > executed, in a way that it's easy for the user to implement whatever > policy he fancies… > > Oh, maybe I should finish preparing those patches for Event Triggers to > be fully usable in 9.4 then ;) I think this can be one useful way to disable. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] HeapTupleSatisfiesDirty fails to test HEAP_XMAX_IS_LOCKED_ONLY for TransactionIdIsInProgress(...)
Hi all
Andres and I were going over a patch yesterday and found an unexpected
bug in tqual.c while attempting to trigger a hypothesized bug in that patch.
A SELECT ... FOR SHARE will incorrectly block on another open
transaction that ran SELECT ... FOR SHARE and still holds the locks if
it has to follow a ctid chain from the current snapshot through a
committed update to a share-locked tuple.
This also affects uniqueness checks in btrees, where it can cause
unnecessary waiting. It's also an issue with FOR KEY UPDATE, in that it
can cause an update to block when it doesn't have to.
The attached test case runs under isolationtester to exersise the
problem. I've tested it against 9.2, 9.3, and HEAD, but Andres looked
over the code and says the underlying bug goes back to the commit of
MVCC, it's just become easier to trigger. To reliably test this with
isolationtester I had to horribly abuse pg_advisory_lock(...) so that I
could force the first SELECT ... FOR UPDATE to acquire its snapshot
before the UPDATE runs.
A backtrace of the point where it's incorrectly blocked is attached.
What's happening is that the test for TransactionIdIsInProgress
unconditionally sets snapshot->xmax, even if xmax was only set for
locking purposes. This will cause the caller to wait for the xid in xmax
when it doesn't need to.
It should be testing HEAP_XMAX_IS_LOCKED_ONLY and only setting
snapshot->xmax if the tuple is really being deleted by an open tx.
Note that HeapTupleSatisfiesDirty tests the infomask for
HEAP_XMAX_IS_MULTI and handles multixacts separately, and in that case
it _does_ already test HEAP_XMAX_IS_LOCKED_ONLY.
When you run the attached test case it should block indefinitely. After
applying the attached patch it'll return as expected.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
# SELECT ... FOR UPDATE SKIP LOCKED
#
# Sanity checks for SELECT ... FOR UPDATE SKIP LOCKED.
setup
{
DROP TABLE IF EXISTS skip_locked;
CREATE TABLE skip_locked (
id integer PRIMARY KEY,
value integer not null
);
INSERT INTO skip_locked
SELECT x,x FROM generate_series(1,10) x;
}
teardown
{
DROP TABLE skip_locked;
}
# The session that does the SKIP LOCKED queries
session "sl1"
step "sl1_begin" {
BEGIN ISOLATION LEVEL READ COMMITTED;
}
step "sl1_selectblocking" {
SELECT xmin, xmax, ctid, * FROM skip_locked WHERE pg_advisory_lock(0) is not null FOR SHARE NOWAIT;
}
teardown{ COMMIT; }
# A session that's used for an UPDATE of the rows to be locked, for when we're testing ctid
# chain following.
session "upd"
step "upd_getlock" {
SELECT pg_advisory_lock(0);
}
step"upd_doupdate" {
BEGIN ISOLATION LEVEL READ COMMITTED;
UPDATE skip_locked SET value = value WHERE id % 2 = 0;
COMMIT;
}
step "upd_releaselock" {
SELECT pg_advisory_unlock(0);
}
# A session that acquires locks that sl1 is supposed to skip
session "lk1"
step"lk1_doforshare" {
BEGIN ISOLATION LEVEL READ COMMITTED;
SELECT id FROM skip_locked WHERE id % 2 = 0 FOR SHARE;
}
teardown {
COMMIT;
}
permutation "upd_getlock" "sl1_begin" "sl1_selectblocking" "upd_doupdate" "lk1_doforshare" "upd_releaselock"
diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
new file mode 100644
index da2ce18..0c72115
*** a/src/backend/utils/time/tqual.c
--- b/src/backend/utils/time/tqual.c
*** HeapTupleSatisfiesDirty(HeapTuple htup,
*** 1012,1018
if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmax(tuple)))
{
! snapshot->xmax = HeapTupleHeaderGetRawXmax(tuple);
return true;
}
--- 1012,1019
if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmax(tuple)))
{
! if (!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
! snapshot->xmax = HeapTupleHeaderGetRawXmax(tuple);
return true;
}
#0 0x003e46af6937 in semop () at ../sysdeps/unix/syscall-template.S:81
#1 0x00600697 in PGSemaphoreLock (sema=0x7fe7027674d0,
interruptOK=interruptOK@entry=1 '\001') at pg_sema.c:415
#2 0x0063e5c6 in ProcSleep (locallock=locallock@entry=0x1e30560,
lockMethodTable=lockMethodTable@entry=0x85e4c0 ) at
proc.c:1092
#3 0x006398f9 in WaitOnLock (locallock=locallock@entry=0x1e30560,
owner=owner@entry=0x1ef6e28) at lock.c:1586
#4 0x0063abeb in LockAcquireExtended
(locktag=locktag@entry=0x7fff26f0d680, lockmode=lockmode@entry=5,
sessionLock=sessionLock@entry=0 '\000', dontWait=dontWait@entry=0 '\000',
reportMemoryError=reportMemoryError@entry=1 '\001') at lock.c:957
#5 0x0063afb1 in LockAcquire (locktag=locktag@entry=0x7fff26f0d680,
lockmode=lockmode@entry=5, sessionLock=sessionLock@entry=0 '\000',
dontWait=dontWait@entry=0 '\000') at lock.c:662
#6 0x00639350 in XactLockTableWait (xid=124785) at lmgr.c:495
#7 0x0057c0e8 in EvalPlanQualFetch (estate=estate@entry=0x1f13e60,
relation=0x7fe70018f1d8, lockmode=lockmode@entry=0,
tid=tid@entry=0x7
Re: [HACKERS] Should we automatically run duplicate_oids?
On Mon, Jul 8, 2013 at 06:25:44PM -0700, Peter Geoghegan wrote: > When rebasing a patch that I'm working on, I occasionally forget to > update the oid of any pg_proc.h entries I may have created. Of course > this isn't a real problem; when I go to initdb, I immediately > recognize what has happened. All the same, it seems like there is a > case to be made for having this run automatically at build time, and > having the build fail on the basis of there being a duplicate - this > is something that fails reliably, but only when someone has added > another pg_proc.h entry, and only when that other person happened to > choose an oid in a range of free-in-git-tip oids that I myself > fancied. > > Sure, I ought to remember to check this anyway, but it seems > preferable to make this process more mechanical. I can point to commit > 55c1687a as a kind of precedent, where the process of running > check_keywords.pl was made to run automatically any time gram.c is > rebuilt. Granted, that's a more subtle problem than the one I'm > proposing to solve, but I still see this as a modest improvement. FYI, attached is the pgtest script I always run before I do a commit; it also calls src/tools/pgtest. It has saved me from erroneous commits many times. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + : . traprm [ "$1" = "-d" ] && DOCS="Y" && shift export QUIET=$(($QUIET + 1)) . cd_pgtop chown -R postgres . echo "Checking SGML" cd doc/src/sgml make check > $TMP/0 2>&1 if grep -v 'fully-tagged' < $TMP/0 | egrep -qi 'Error|Warning' thenecho "SGML error" cat $TMP/0 exit 1 fi [ $(pwd) != '/pgsql/8.4/doc/src/sgml' ] && make check-tabs # Run only at night to check for HISTORY build problems # in HISTORY.html. if [ ! -t 0 -o "$DOCS" = "Y" ] thenmake INSTALL.html > $TMP/0 2>&1 if egrep -qi 'Error|Warning' < $TMP/0 thenecho "SGML error" cat $TMP/0 exit 1 fi make HISTORY.html > $TMP/0 2>&1 if grep -q 'Error' < $TMP/0 thenecho "SGML error" cat $TMP/0 exit 1 fi fi # fails on /bin/sh cd - echo "Checking duplicate oids" cd src/include/catalog duplicate_oids > $TMP/0 if [ -s $TMP/0 ] thenecho "Duplicate system oids" cat $TMP/0 exit 1 fi cd - # supress assembler warning (aspg /pg/tools/pgtest "$@"; echo "$?" > $TMP/ret) | # use only one grep so we don't buffer output egrep -v ': Warning: using `%|^SPI.c:.*: warning: |^ppport.h:[[:digit:]][[:digit:]]*: warning: |^/usr/libdata/perl5/5.00503/i386-bsdos/CORE/patchlevel.h|plperl.c:.*: warning: (implicit|passing)|variable .(fast|ptr|cmsg|fe_copy). might be clobbered|warning: unused variable .yyg.|gzwrite'"'"' discards' rm -fr src/test/regress/tmp_check bell exit $(cat $TMP/ret) -- 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] Regarding BGworkers
Amit Kapila escribió: > > On Friday, August 02, 2013 4:19 AM Michael Paquier wrote: > >On Fri, Aug 2, 2013 at 1:22 AM, Robert Haas wrote: > >> On Sun, Jul 28, 2013 at 1:26 AM, Amit kapila > >> wrote: > >>> 2. Shouldn't function > >>> do_start_bgworker()/StartOneBackgroundWorker(void) be moved to > >>> bgworker.c > >>> as similar functions AutoVacWorkerMain()/PgArchiverMain() are in > >>> their respective files. > > >> Yes, perhaps so. Other votes? > > > StartOneBackgroundWorker uses StartWorkerNeeded and HaveCrashedWorker, and > > IMO, we should not expose that outside the postmaster. > > How about exposing Set/Get for these from bgworker? That seems more mess than just keeping that function in postmaster.c. I agree with moving the other one. -- Álvaro Herrerahttp://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] Regarding BGworkers
On Friday, August 02, 2013 4:19 AM Michael Paquier wrote: On Fri, Aug 2, 2013 at 1:22 AM, Robert Haas wrote: On Sun, Jul 28, 2013 at 1:26 AM, Amit kapila wrote: >>> 2. Shouldn't function >>> do_start_bgworker()/StartOneBackgroundWorker(void) be moved to bgworker.c >>> as similar functions AutoVacWorkerMain()/PgArchiverMain() are in their respective files. >> Yes, perhaps so. Other votes? > StartOneBackgroundWorker uses StartWorkerNeeded and HaveCrashedWorker, and IMO, we should not expose that outside the postmaster. How about exposing Set/Get for these from bgworker? > On the contrary, > moving do_start_bgworker would be fine, as it uses nothing exclusive to the postmaster as far as I saw, and it would also make it more consistent with > the other features. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] No more need for pg_clearxlogtail?
On Mon, Jul 8, 2013 at 09:16:32AM +, Heikki Linnakangas wrote: > This has one user-visible change: switching to a new WAL segment with > pg_switch_xlog() now fills the remaining unused portion of the segment with > zeros. This potentially adds some overhead, but it has been a very common > practice by DBA's to clear the "tail" of the segment with an external > pg_clearxlogtail utility anyway, to make the WAL files compress better. > With this patch, it's no longer necessary to do that. So this removes the need to add pg_clearxlogtail to contrib? Great. Thanks. -- 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
> On Thu, Aug 1, 2013 at 8:27 PM, Stephen Frost wrote: > > The point above is that we will always need some amount of external > > config file and, as such, we should probably consider which items should > > really only be set in the *config* files and which can be set in either > > place. I think this idea might make some sense: mark the settings (add a flag bit in the guc.c tables) that can be changed via alter setting. Or perhaps add the bit to ones that can *not* be changed. I don't think we need to promise exact compatibility on the set of settings that can be changed via ALTER SYSTEM. If you can change setting XYZ in 9.4, and we find that it wasn't such a good idea and have to disallow it in 9.5, well, too bad. (Or perhaps even a minor version. Am I sacrilegious?) -- Álvaro Herrerahttp://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] strange IS NULL behaviour
Peter Eisentraut wrote:
> On 7/4/13 5:06 PM, Alvaro Herrera wrote:
> > FWIW if changing the behavior of NOT NULL constraints is desired, I
> > still have the patch to catalogue them around, if anyone wants to play
> > around. I haven't gotten around to finishing it up, yet :-(
>
> If your latest patch isn't publicly available, I'd like to see it. I
> might work on that later this year.
Here it is. Note that it is quite incomplete: there are parts that are
not even close to compiling. I think I was trying to figure out how to
determine whether a column was of a composite type.
I might get back to it eventually, so if you start working on it, do let
me know.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/commands/constraint.c b/src/backend/commands/constraint.c
index 8ac8373..74f0766 100644
--- a/src/backend/commands/constraint.c
+++ b/src/backend/commands/constraint.c
@@ -14,12 +14,16 @@
#include "postgres.h"
#include "catalog/index.h"
+#include "catalog/pg_constraint.h"
+#include "commands/constraint.h"
#include "commands/trigger.h"
#include "executor/executor.h"
#include "utils/builtins.h"
#include "utils/rel.h"
+#include "utils/syscache.h"
#include "utils/tqual.h"
+static char *tryExtractNotNull_internal(Node *node, Relation rel);
/*
* unique_key_recheck - trigger function to do a deferred uniqueness check.
@@ -188,3 +192,166 @@ unique_key_recheck(PG_FUNCTION_ARGS)
return PointerGetDatum(NULL);
}
+
+/*
+ * Create a Constraint node representing a "col IS NOT NULL" expression
+ * using a ColumnRef within, for the given relation and column.
+ *
+ * If the constraint name is not provided, it is generated.
+ */
+Constraint *
+createCheckNotNullConstraint(Oid nspid, char *constraint_name,
+ const char *relname, const char *colname)
+{
+ ColumnRef *colref;
+ NullTest *nulltest;
+
+ colref = (ColumnRef *) makeNode(ColumnRef);
+ colref->fields = list_make1(makeString(pstrdup(colname)));
+
+ nulltest = (NullTest *) makeNode(NullTest);
+ nulltest->argisrow = false; /* FIXME -- may be bogus! */
+ nulltest->nulltesttype = IS_NOT_NULL;
+ nulltest->arg = (Expr *) colref;
+
+ return makeCheckConstraint(nspid, constraint_name, relname, colname,
+ nulltest);
+}
+
+/*
+ * Create a Constraint node representing "col IS DISTINCT FROM NULL". This is
+ * just like the above, but this is intended to be used for columns with
+ * composite types, which have slightly different rules.
+ */
+Constraint *
+createCheckDistinctNotNullConstraint(Oid nspid, char *constraint_name,
+ const char *relname, const char *colname,
+ Oid column_type)
+{
+ Constraint *check;
+ ColumnRef *colref;
+ A_Expr *expr;
+
+ colref = (ColumnRef *) makeNode(ColumnRef);
+ colref->fields = list_make1(makeString(pstrdup(colname)));
+
+ expr = makeSimpleA_Expr(AEXPR_DISTINCT, "=", colref, makeNullAConst(-1),
+ -1);
+
+ return makeCheckConstraint(nspid, constraint_name, relname, colname, expr);
+
+ return check;
+}
+
+static Constraint *
+makeCheckConstraint(Oid nspid, char *constraint_name, const char *relname,
+ const char *colname, Node *expr)
+{
+ Constraint *check = makeNode(Constraint);
+
+ check->contype = CONSTR_CHECK;
+ check->location = -1;
+ check->conname = constraint_name ? constraint_name :
+ ChooseConstraintName(relname, colname, "not_null", nspid,
+ NIL);
+ check->raw_expr = raw_expr;
+ check->cooked_expr = NULL;
+
+}
+
+/*
+ * Given a CHECK constraint, examine it and determine whether it is CHECK (col
+ * IS NOT NULL). If it is, return the column name for which it is. Otherwise
+ * return NULL.
+ */
+char *
+tryExtractNotNullFromCheckConstr(Constraint *constr)
+{
+ char *retval;
+
+ Assert(constr->contype == CONSTR_CHECK);
+
+ if (constr->raw_expr != NULL)
+ {
+ retval = tryExtractNotNull_internal(constr->raw_expr, NULL);
+ if (retval != NULL)
+ return retval;
+ }
+
+ if (constr->cooked_expr != NULL)
+ {
+ return tryExtractNotNull_internal(stringToNode(constr->cooked_expr),
+ NULL);
+ }
+
+ return NULL;
+}
+
+/*
+ * As above, but use a pg_constraint row as input.
+ *
+ * tupdesc is pg_constraint's tuple descriptor, and rel is the relation the
+ * constraint is for.
+ */
+char *
+tryExtractNotNullFromCatalog(HeapTuple constrTup, TupleDesc tupdesc,
+ Relation rel)
+{
+ Datum val;
+ bool isnull;
+ char *conbin;
+ Node *node;
+
+ val = SysCacheGetAttr(CONSTROID, constrTup, Anum_pg_constraint_conbin,
+ &isnull);
+ if (isnull)
+ elog(ERROR, "null conbin for constraint %u",
+ HeapTupleGetOid(constrTup));
+ conbin = TextDatumGetCString(val);
+ node = (Node *) stringToNode(conbin);
+
+ return tryExtractNotNull_internal(node, rel);
+}
+
+/*
+ * Worker for the above
+ */
+static char *
+tryExtractNotNull_internal(Node *node, Relation rel)
+{
+ if (IsA(node, NullTest))
+ {
+ NullTest *nulltest = (NullTest *) node;
+
+ if (nulltest->
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thu, Aug 1, 2013 at 8:27 PM, Stephen Frost wrote: > The point above is that we will always need some amount of external > config file and, as such, we should probably consider which items should > really only be set in the *config* files and which can be set in either > place. What settings did you have in mind? Which ones ought to be only be settable in config files in your estimation? -- Peter Geoghegan -- 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Andres Freund (and...@2ndquadrant.com) wrote: > FWIW, I think you've just put the final nail in the coffin of this > patch by raising the barriers unreasonably high. For my 2c, I don't think it's an unreasonable idea to actually *consider* what options are available through this mechanism rather than just presuming that it's a good idea to be able to modify anything, including things that you wouldn't be able to fix after a restart w/o hacking around in $PGDATA. I also don't believe that limiting the set of options which can be modified through this system is a particularly difficult thing to implement. > > * Andres Freund (and...@2ndquadrant.com) wrote: > On 2013-08-01 21:06:49 -0400, Stephen Frost wrote: > > > Even trying to do this completely will guarantee that this patch will > > > never, ever, suceed. There simply is no way to reliably detect problems > > > that have complex interactions with the rest of the system. > > > > The patch will never be able to completely remove the need for external > > config files, without changes to PG to deal with these conditions > > better. > > That's not the goal of the patch as far as I understand it. The point above is that we will always need some amount of external config file and, as such, we should probably consider which items should really only be set in the *config* files and which can be set in either place. > I think this chain of argument doesn't have much for it. There are > litteraly dozens of ways to break postgres from SQL which we don't even > try to defend against. This is a strawman. An admin doing "DELETE FROM pg_class;" or using COPY to overwrite files in PG's data dir and doing "ALTER SYSTEM SET shared_buffers = '2GB';", "ALTER SYSTEM SET port = 123;" or even "ALTER SYSTEM SET data_directory = '/new/path/for/db';" (how would doing that even make sense..?) are not nearly the same. On the flip side, there's not nearly as much risk around allowing log_line_prefix and friends to be set through ALTER SYSTEM SET because it's pretty unlikely that such a misconfiguration would cause PG to not start. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Hi, FWIW, I think you've just put the final nail in the coffin of this patch by raising the barriers unreasonably high. > * Andres Freund (and...@2ndquadrant.com) wrote: > > I agree that we need to do reasonable checks, like running GUC > > validators, but we simply can't control the overall system state. And > > it's not like this are errors that you couldn't get before. And we > > should (that's something to improve on) report the relevant guc + file > > in many cases. > > You could get the errors before, sure, but when you did, you could read > the log output and go modify the *configuration files* (which things in > $PGDATA are *not*) and fix it and get the system back online. If the > files in $PGDATA have to be modified to get the system online then they > are configuration files and should be in /etc. That doesn't seem to be a logical consequence to me. On 2013-08-01 21:06:49 -0400, Stephen Frost wrote: > > Even trying to do this completely will guarantee that this patch will > > never, ever, suceed. There simply is no way to reliably detect problems > > that have complex interactions with the rest of the system. > > The patch will never be able to completely remove the need for external > config files, without changes to PG to deal with these conditions > better. That's not the goal of the patch as far as I understand it. > > We can improve the detection rate of problems after some real world > > experience. Don't make this unneccesarily complex. > > Actually, putting it out there as "this can be used to modify anything > and means you can trivially make PG unstartable" is actually the wrong > move to make, imv. Consider that, to deal with the issues caused, we'd > have to *remove* things from being modifyable through this function. > That's a whole lot harder to do from a backward-compatibility view than > adding things later as we improve PG to be able to still come up enough > to be useful even with configuration issues. I think this chain of argument doesn't have much for it. There are litteraly dozens of ways to break postgres from SQL which we don't even try to defend against. Starting from DELETE FROM pg_class, ending with COPYing files into the datadir. This is a database, not a children's toy, and the feature *is* superuser only. Anyway, I don't see much point arguing this further. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [9.3 bug] disk space in pg_xlog increases during archive recovery
On Fri, Aug 2, 2013 at 12:24 AM, MauMau wrote: > From: "Fujii Masao" > > However, isn't StandbyRequested true (= standby_mode set to on) to enable >>> warm standby? >>> >> >> We can set up warm-standby by using pg_standby even if standby_mode = off. >> > > I see. However, I understand that pg_standby is a legacy feature, and the > current way to setup a warm standby is to set standby=on and > restore_command in recovery.conf. So I think it might not be necessary to > support cascading replication with pg_standby, if supporting it would > prevent better implementation of new features. You are right about that, you should stick with the core features as much as you can and limit the use of wrapper utilities. Since 9.1 and the apparition of a built-in standby mode inside Postgres (with the apparition of the GUC parameter hot_standby), it seems better to use that instead of pg_standby, which would likely (personal opinion, feel free to scream at me) be removed in a future release. > > > I'm afraid people set max_wal_senders>0 and hot_standby=on >>> even on the primary server to make the contents of postgresql.conf >>> identical >>> on both the primary and the standby for easier configuration. If so, >>> normal >>> archive recovery (PITR, not the standby recovery) would face the original >>> problem -- unnecessary WAL accumulation in pg_xlog/. So I'm wonder if >>> AllowCascadeReplication() is enough. >>> >> >> One idea is to add new GUC parameter like enable_cascade_replication >> and then prevent WAL file from being kept in pg_xlog if that parameter is >> off. >> But we cannot apply such change into the already-released version 9.2. >> > > Yes, I thought the same, too. Adding a new GUC to enable cascading > replication now would be a usability degradation. But I have no idea of > any better way. It seems we need to take either v1 or v2 of the patch I > sent. If we consider that we don't have to support cascading replication > for a legacy pg_standby, v1 patch is definitely better because the standby > server would not keep restored archive WAL in pg_xlog when cascading > replication is not used. Otherwise, we have to take v2 patch. > By reading this thread, -1 for the addition of a new GUC parameter related to cascading, it looks like an overkill for the possible gain. And +1 for the removal of WAL file once it is replayed in archive recovery if cascading replication is not allowed. However, what about wal_keep_segments? Doesn't the server keep segments even if replication is not allowed? If yes, the immediate WAL file drop after replay needs to take care of that... -- Michael
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
* David Johnston (pol...@yahoo.com) wrote: > How about some form of persistence mechanism so that, before making these > kinds of changes, the admin can "save" the current configuration. Then, in > a worse case-scenario, they could run something like "pg_ctl > --restore-persisted-configuration ..." to reset everything back the last > known good configuration. Yeah, I had considered the possibility that we would provide a tool to manage the config in $PGDATA but I'm not really thrilled with that idea either. > A single-version save-restore routine for the configuration. When restoring > you would want to keep the "current/non-working" configuration and > associated logging information - maybe archived somewhere along with the a > copy of the last known working version. This would provide some level of > audit capability as well as a convenient way for someone to take that > archive and send it off to someone more knowledgeable for assistance. > Having it auto-run at boot time - possibly to a different archive area than > when run manually - would be possible as well; so you'd have both the last > good boot configuration as well as whatever point-in-time configurations you > wish to save. Yeah, there's a lot of work involved in writing a whole system for managing multiple configurations with history, diffs, etc.. Problems which, really, our existing text-based config w/ a tool like puppet have already solved. Thanks, Stephen signature.asc Description: Digital signature
[HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Andres Freund-3 wrote > Even trying to do this completely will guarantee that this patch will > never, ever, suceed. There simply is no way to reliably detect problems > that have complex interactions with the rest of the system. > > We can improve the detection rate of problems after some real world > experience. Don't make this unneccesarily complex. Instead of prevention some thought to recovery should be considered then. How about some form of persistence mechanism so that, before making these kinds of changes, the admin can "save" the current configuration. Then, in a worse case-scenario, they could run something like "pg_ctl --restore-persisted-configuration ..." to reset everything back the last known good configuration. A single-version save-restore routine for the configuration. When restoring you would want to keep the "current/non-working" configuration and associated logging information - maybe archived somewhere along with the a copy of the last known working version. This would provide some level of audit capability as well as a convenient way for someone to take that archive and send it off to someone more knowledgeable for assistance. Having it auto-run at boot time - possibly to a different archive area than when run manually - would be possible as well; so you'd have both the last good boot configuration as well as whatever point-in-time configurations you wish to save. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Proposal-for-Allow-postgresql-conf-values-to-be-changed-via-SQL-tp5729917p5765968.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Andres Freund (and...@2ndquadrant.com) wrote: > I agree that we need to do reasonable checks, like running GUC > validators, but we simply can't control the overall system state. And > it's not like this are errors that you couldn't get before. And we > should (that's something to improve on) report the relevant guc + file > in many cases. You could get the errors before, sure, but when you did, you could read the log output and go modify the *configuration files* (which things in $PGDATA are *not*) and fix it and get the system back online. If the files in $PGDATA have to be modified to get the system online then they are configuration files and should be in /etc. > Even trying to do this completely will guarantee that this patch will > never, ever, suceed. There simply is no way to reliably detect problems > that have complex interactions with the rest of the system. The patch will never be able to completely remove the need for external config files, without changes to PG to deal with these conditions better. > We can improve the detection rate of problems after some real world > experience. Don't make this unneccesarily complex. Actually, putting it out there as "this can be used to modify anything and means you can trivially make PG unstartable" is actually the wrong move to make, imv. Consider that, to deal with the issues caused, we'd have to *remove* things from being modifyable through this function. That's a whole lot harder to do from a backward-compatibility view than adding things later as we improve PG to be able to still come up enough to be useful even with configuration issues. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-01 20:45:38 -0400, Stephen Frost wrote: > * Andres Freund (and...@2ndquadrant.com) wrote: > > I personally consider readers of this list persons... And even people > > not interested in internals will have to look in there if they set > > something stupid before. Like setting max_connections higher than the > > currently configured kernel's max number of semaphores. Or a good number > > of other settings. > > And that's actually one of the issues that I have with this overall > approach.. If configurations can be set in 'data' areas which prevent > the system from starting, that's *bad*. Very bad. It means people will > not be able to trust PG to manage the configuration sanely and will have > a lot of distrust and fear of the ALTER SYSTEM capability. I agree that we need to do reasonable checks, like running GUC validators, but we simply can't control the overall system state. And it's not like this are errors that you couldn't get before. And we should (that's something to improve on) report the relevant guc + file in many cases. > Requiring users to go monkey around inside of a system data directory to > clean things up in order to get PG to come up is a situation we should > do our best to prevent from ever happening. Even trying to do this completely will guarantee that this patch will never, ever, suceed. There simply is no way to reliably detect problems that have complex interactions with the rest of the system. We can improve the detection rate of problems after some real world experience. Don't make this unneccesarily complex. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Andres Freund (and...@2ndquadrant.com) wrote: > I personally consider readers of this list persons... And even people > not interested in internals will have to look in there if they set > something stupid before. Like setting max_connections higher than the > currently configured kernel's max number of semaphores. Or a good number > of other settings. And that's actually one of the issues that I have with this overall approach.. If configurations can be set in 'data' areas which prevent the system from starting, that's *bad*. Very bad. It means people will not be able to trust PG to manage the configuration sanely and will have a lot of distrust and fear of the ALTER SYSTEM capability. Requiring users to go monkey around inside of a system data directory to clean things up in order to get PG to come up is a situation we should do our best to prevent from ever happening. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-01 20:33:49 -0400, Stephen Frost wrote: > * Andres Freund (and...@2ndquadrant.com) wrote: > > People know what to expect from .d directories, that's why I suggested > > it, don't feel really strongly about it. I dislike naming the > > subdirectories pgconf/... et al though, we should reference the original > > files name, instead of introducing new abbreviations. > > *People* should not be messing around with files under $PGDATA- that's > half the point. conf.d or other names which look like configuration > directories that an admin should modify are a *bad* idea. I personally consider readers of this list persons... And even people not interested in internals will have to look in there if they set something stupid before. Like setting max_connections higher than the currently configured kernel's max number of semaphores. Or a good number of other settings. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Andres Freund (and...@2ndquadrant.com) wrote: > People know what to expect from .d directories, that's why I suggested > it, don't feel really strongly about it. I dislike naming the > subdirectories pgconf/... et al though, we should reference the original > files name, instead of introducing new abbreviations. *People* should not be messing around with files under $PGDATA- that's half the point. conf.d or other names which look like configuration directories that an admin should modify are a *bad* idea. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-01 14:37:45 -0400, Stephen Frost wrote: > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > Andres Freund wrote: > > > > > Postgresql.auto.conf.d is what I'd propose, but the decision about > > > that seems to be one of the smaller problems around this feature... > > > That naming seems to make it sensible to extend other files (hba, > > > ident) similarly at a later point. > > > > I don't like this particular naming proposal, but I'm glad this patch > > finally seems to be getting somewhere. > > Yeah, also not a fan. We don't have any 'conf.d' directories in PGDATA > and I don't think we should start now. I liked Josh's suggestions of > something like "system_conf". For multiple independent config files, we > could use directories under that (eg: 'pgconf', 'pghba', 'pgident'..). People know what to expect from .d directories, that's why I suggested it, don't feel really strongly about it. I dislike naming the subdirectories pgconf/... et al though, we should reference the original files name, instead of introducing new abbreviations. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Andres Freund writes: > They would need a setting that disables ALTER (DATABASE|USER) ... SET > ... as well though. At least for some settings. > > I don't think enforcing things on that level makes much sense. If only we could trigger some actions when a command is about to be executed, in a way that it's easy for the user to implement whatever policy he fancies… Oh, maybe I should finish preparing those patches for Event Triggers to be fully usable in 9.4 then ;) Regards, -- Dimitri Fontaine 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] new "row-level lock" error messages
Robert Haas escribió:
> The fact that there are no tests of this functionality is probably not
> a good thing. We should add some. At the moment, the following test
> case crashes, and it looks like this commit is responsible:
>
> create table test_update2 (id integer);
> DECLARE test_update_cursor CURSOR FOR SELECT id, MIN(id) FROM
> test_update2 GROUP By id HAVING MIN(id) < 1 FOR UPDATE;
The attached patch fixes it, and I think it makes more sense than the
original coding to start with.
I will commit this tomorrow, adding a few test cases while at it.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 9ff8050..49bd930 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1962,7 +1962,8 @@ preprocess_rowmarks(PlannerInfo *root)
* CTIDs invalid. This is also checked at parse time, but that's
* insufficient because of rule substitution, query pullup, etc.
*/
- CheckSelectLocking(parse);
+ CheckSelectLocking(parse, ((RowMarkClause *)
+ linitial(parse->rowMarks))->strength);
}
else
{
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 39036fb..a9d1fec 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -2243,7 +2243,7 @@ LCS_asString(LockClauseStrength strength)
* exported so planner can check again after rewriting, query pullup, etc
*/
void
-CheckSelectLocking(Query *qry)
+CheckSelectLocking(Query *qry, LockClauseStrength strength)
{
if (qry->setOperations)
ereport(ERROR,
@@ -2251,56 +2251,49 @@ CheckSelectLocking(Query *qry)
/*--
translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with UNION/INTERSECT/EXCEPT",
- LCS_asString(((RowMarkClause *)
- linitial(qry->rowMarks))->strength;
+ LCS_asString(strength;
if (qry->distinctClause != NIL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*--
translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with DISTINCT clause",
- LCS_asString(((RowMarkClause *)
- linitial(qry->rowMarks))->strength;
+ LCS_asString(strength;
if (qry->groupClause != NIL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*--
translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with GROUP BY clause",
- LCS_asString(((RowMarkClause *)
- linitial(qry->rowMarks))->strength;
+ LCS_asString(strength;
if (qry->havingQual != NULL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*--
translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with HAVING clause",
- LCS_asString(((RowMarkClause *)
- linitial(qry->rowMarks))->strength;
+ LCS_asString(strength;
if (qry->hasAggs)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*--
translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with aggregate functions",
- LCS_asString(((RowMarkClause *)
- linitial(qry->rowMarks))->strength;
+ LCS_asString(strength;
if (qry->hasWindowFuncs)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*--
translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with window functions",
- LCS_asString(((RowMarkClause *)
- linitial(qry->rowMarks))->strength;
+ LCS_asString(strength;
if (expression_returns_set((Node *) qry->targetList))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*--
translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with set-returning functions in the target list",
- LCS_asString(((RowMarkClause *)
- linitial(qry->rowMarks))->strength;
+ LCS_asString(strength;
}
/*
@@ -2321,7 +2314,7 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
Index i;
LockingClause *allrels;
- CheckSelectLocking(qry);
+ CheckSelectLocking(qry, lc->strength);
/* make a clause we can pass down to subqueries to select all rels */
allrels = makeNode(LockingClause);
diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h
index b24b205..2fef2f7 100644
--- a/src/include/parser/analyze.h
+++ b/src/include/parser/analyze.h
@@ -37,7 +37,7 @@ extern Query *transformStmt(ParseState *pstate, Node *parseTree);
extern bool analyze_requires_snapshot(Node *parseTree);
extern char *LCS_asString(LockClauseStrength strength);
-extern void CheckSelectLocking(Query *qry);
+exter
Re: [HACKERS] Regarding BGworkers
On Fri, Aug 2, 2013 at 1:22 AM, Robert Haas wrote: > On Sun, Jul 28, 2013 at 1:26 AM, Amit kapila > wrote: > > 2. Shouldn't function > > do_start_bgworker()/StartOneBackgroundWorker(void) be moved to bgworker.c > >as similar functions AutoVacWorkerMain()/PgArchiverMain() are in > their respective files. > > Yes, perhaps so. Other votes? > StartOneBackgroundWorker uses StartWorkerNeeded and HaveCrashedWorker, and IMO, we should not expose that outside the postmaster. On the contrary, moving do_start_bgworker would be fine, as it uses nothing exclusive to the postmaster as far as I saw, and it would also make it more consistent with the other features. Regards, -- Michael
Re: [HACKERS] pass-through queries to foreign servers
On Tue, Jul 30, 2013 at 10:22 PM, Tom Lane wrote: > David Fetter writes: >> On Tue, Jul 30, 2013 at 04:40:38PM -0700, David Gudeman wrote: >>> When you write an application involving foreign tables, you frequently >>> end up with queries that are just too inefficient because they bring >>> too much data over from the foreign server. For a trivial example, >>> consider "SELECT count(*) FROM t" where t is a foreign table. This >>> will pull the entire table over the network just to count up the rows. > >> Yes, and this case is a known limitation of our planner >> infrastructure. Aggregates are "special" when it comes to >> generating paths for the planner to evaluate, so there's no current >> way a FDW could supply such info to the planner, and hence no API in >> our FDW code for having FDWs supply that info. That's probably a >> "should fix" but I don't know whether a project that size could be >> done by 9.4. > > Yeah. There's a lot left to be done in the FDW infrastructure. > But not this: > >> All that said, my DBI-Link, back in the bad old days, provided two >> important functions: remote_select(), which returned SETOF RECORD and >> remote_execute(), which returned nothing. It also provided ways to >> control connections to the remote host, introspect remote schemas, >> etc., etc. We need capabilities like that in the FDW API, I believe >> we could have them by 9.4. > > I would argue we *don't* want that. If you want pass-through queries > or explicit connection control, your needs are already met by dblink or > dbi-link. The whole point of FDW is that it's at a higher level of > abstraction than that; which offers greater ease of use and will > eventually offer better optimization than what you can get from dblink > et al. If we start trying to shoehorn things like passthrough queries > into FDW, we'll be crippling the technology. As an example, somebody > on planet postgresql was just recently surprised to find that postgres_fdw > honors transaction rollback. Well, it can do that because users can't > disconnect the connection underneath it, nor issue passthrough > commit/rollback commands. You don't get to have it both ways. > > regards, tom lane Tom, you have a good point about transaction management, but I think we _can_ have it both ways. There are several things that the author of the foreign data wrapper can do to prevent bad things from being done since he has ultimate control of everything that gets sent to the foreign server. For many foreign servers it is enough to check that the string being sent to the foreign server begins with "select ". Or he can prevent pass-through queries when there is an on-going transaction on the foreign server. Or the author of a particular foreign data wrapper can prevent pass-through queries entirely. The point is that this is only a concern for some kinds of foreign servers and even then only for those foreign data wrappers that care about transactions. If they don't implement update/insert/delete, for example, then it doesn't matter. Since there are many other kinds of foreign servers where this could be useful, it should be available, at least as an option. The reason I want it is to do use it for some of the things that David Fetter was talking about --optimizing queries with aggregates and GROUP BY. I have code that currently optimizes these sorts of queries for a particular database engine. I did this several years ago before there were foreign data wrappers so I had to roll my own using table functions. My implementation query rewrite rather than plan optimization (it seemed to me to be too hard to do in the panning phase). See http://unobtainabol.blogspot.com/2013/04/daves-foreign-data-translating-foreign_24.html for a description of what it does. My plan was to generalize my current code to generic SQL databases and to make it work with foreign data wrappers. If there is any interest from the PG community I'll try to get my company to let me contribute this back. But the first thing I need is to implement pass-through queries for foreign servers or I have to duplicate all of the functionality for managing foreign servers and tables. Regards, David Gudeman http://unobtainabol.blogspot.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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Andres Freund wrote: > > > Postgresql.auto.conf.d is what I'd propose, but the decision about > > that seems to be one of the smaller problems around this feature... > > That naming seems to make it sensible to extend other files (hba, > > ident) similarly at a later point. > > I don't like this particular naming proposal, but I'm glad this patch > finally seems to be getting somewhere. Yeah, also not a fan. We don't have any 'conf.d' directories in PGDATA and I don't think we should start now. I liked Josh's suggestions of something like "system_conf". For multiple independent config files, we could use directories under that (eg: 'pgconf', 'pghba', 'pgident'..). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Josh Berkus schrieb: >On 08/01/2013 10:24 AM, Andres Freund wrote: >>> Let's please NOT call it conf.d if it's living in PGDATA and is not >>> meant to be edited by hand. conf.d is for a directory of config >files >>> created by users and external utilities, living in CONFIGDIR. >> >> How nice that that's not what's being discussed here then. conf.d >*IS* >> the thing thats been proposed to be a separate feature from ALTER >> SYSTEM. For the use case you describe. > >Some of the earlier emails sounded like that's exactly what was >proposed. Glad to clarify then. > >*if* we do file-per-setting, what do you think of the directory name >system_set or system_conf then? Postgresql.auto.conf.d is what I'd propose, but the decision about that seems to be one of the smaller problems around this feature... That naming seems to make it sensible to extend other files (hba, ident) similarly at a later point. Please excuse brevity and formatting - I am writing this on my mobile phone. Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 08/01/2013 10:24 AM, Andres Freund wrote: >> Let's please NOT call it conf.d if it's living in PGDATA and is not >> meant to be edited by hand. conf.d is for a directory of config files >> created by users and external utilities, living in CONFIGDIR. > > How nice that that's not what's being discussed here then. conf.d *IS* > the thing thats been proposed to be a separate feature from ALTER > SYSTEM. For the use case you describe. Some of the earlier emails sounded like that's exactly what was proposed. Glad to clarify then. *if* we do file-per-setting, what do you think of the directory name system_set or system_conf then? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Josh Berkus writes: > Let's please NOT call it conf.d if it's living in PGDATA and is not > meant to be edited by hand. conf.d is for a directory of config files > created by users and external utilities, living in CONFIGDIR. +1 Regards, -- Dimitri Fontaine 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] libpq thread locking during SSL connection start
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > pgsecure_open_client() returns -1 if it can't lock the mutex. This is a > problem because the callers are not prepared for that return value. I > think it should return PGRES_POLLING_FAILED instead, after setting an > appropriate error message in conn->errorMessage. Ah, right, adding it there was a bit of a late addition, tbh. > initialize_SSL() fails to set an error message. The return code of -1 > seems fine here. Right, will improve. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] libpq thread locking during SSL connection start
Stephen Frost wrote: > All, > > I wanted to highlight the below commit as being a significant enough > change that it warrents being seen on -hackers and not just > -committers. If you use SSL with libpq, particularly in a threaded > mode/environment, please take a look/test this change. Prior to the > patch, we would crash pretty easily when using client certificates > with multiple threads; I was unable to get it to crash after the > patch. That said, there's also risk that this patch will cause > slow-downs in SSL connections when using threads due to the additional > locking. I'm not sure that's a heavily used case, but none-the-less, > if you do that, please considering testing this patch. Now that I look at it, I think there are a couple of problems with this patch, particularly when pthread_mutex_lock fails. I grant you that that is a very improbable condition, but if so then we should Assert() against it or something like that. (Since I am not sure what would constitute good behavior from Assert() in libpq, I tend to think this is not a good idea.) pgsecure_open_client() returns -1 if it can't lock the mutex. This is a problem because the callers are not prepared for that return value. I think it should return PGRES_POLLING_FAILED instead, after setting an appropriate error message in conn->errorMessage. initialize_SSL() fails to set an error message. The return code of -1 seems fine here. -- Álvaro Herrerahttp://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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-01 10:16:59 -0700, Josh Berkus wrote: > Dimitri, > > > We rehash because the situation did change *a lot*. We just decided that > > the ALTER SYSTEM SET setup will live in PGDATA and will not have to be > > edited by DBA nor sysadmin nor tools ever. We will have a separate > > facility (conf.d) for that. As a result, I don't think there's any > > issues left with one-setting-per-file now. > > Let's please NOT call it conf.d if it's living in PGDATA and is not > meant to be edited by hand. conf.d is for a directory of config files > created by users and external utilities, living in CONFIGDIR. How nice that that's not what's being discussed here then. conf.d *IS* the thing thats been proposed to be a separate feature from ALTER SYSTEM. For the use case you describe. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-01 10:13:37 -0700, Josh Berkus wrote: > On 07/26/2013 12:19 PM, Stephen Frost wrote: > > Agreed. To continue that thought, I find it *very* unlikely that a > > given environment would use *both* a tool like puppet to manage the > > files in their conf.d *and* have people using ALTER SYSTEM SET. You're > > going to do one or the other, almost certainly; not the least of which > > is because those are very likely two different teams and only one of > > them is going to be responsible for the PG system config. > > Ideally, yes. And that's the reason why I think that we will need to > implement a way to disable ALTER SYSTEM SET in postgresql.conf before > 9.4.0 is done. The big-Puppet-management shops will demand it; they do > NOT want their DBAs setting unversioned settings in isolation on one > database server out of 200. They would need a setting that disables ALTER (DATABASE|USER) ... SET ... as well though. At least for some settings. I don't think enforcing things on that level makes much sense. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Dimitri, > We rehash because the situation did change *a lot*. We just decided that > the ALTER SYSTEM SET setup will live in PGDATA and will not have to be > edited by DBA nor sysadmin nor tools ever. We will have a separate > facility (conf.d) for that. As a result, I don't think there's any > issues left with one-setting-per-file now. Let's please NOT call it conf.d if it's living in PGDATA and is not meant to be edited by hand. conf.d is for a directory of config files created by users and external utilities, living in CONFIGDIR. If we're going to have a directory of individual file settings in PGDATA, let's be consistent with the existing command and call it "system_set". -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 07/26/2013 12:19 PM, Stephen Frost wrote: > Agreed. To continue that thought, I find it *very* unlikely that a > given environment would use *both* a tool like puppet to manage the > files in their conf.d *and* have people using ALTER SYSTEM SET. You're > going to do one or the other, almost certainly; not the least of which > is because those are very likely two different teams and only one of > them is going to be responsible for the PG system config. Ideally, yes. And that's the reason why I think that we will need to implement a way to disable ALTER SYSTEM SET in postgresql.conf before 9.4.0 is done. The big-Puppet-management shops will demand it; they do NOT want their DBAs setting unversioned settings in isolation on one database server out of 200. However, I can imagine "hybrid" approaches. For example, my personal main use for conf.d/ is to modify logging settings on a temporary basis. I can imagine DBAs using ALTER SYSTEM SET for this purpose as well, that is just to turn log_min_duration_statement down to 0 and then back up again. In that case, most of the "live" settings would live in /etc/postgresql, but some of the logging settings would be controlled with ALTER SYSTEM SET> That conceptually seems to work fine with the existing design, I just wanted to bring it up as a likely use case. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Josh Berkus writes: > While I find some value in the one-setting-per-file approach, there's > also some major issues with it. And we already argued this out months > ago, and ended up with the current single-file approach. Let's not > rehash the past infinitely, please? We rehash because the situation did change *a lot*. We just decided that the ALTER SYSTEM SET setup will live in PGDATA and will not have to be edited by DBA nor sysadmin nor tools ever. We will have a separate facility (conf.d) for that. As a result, I don't think there's any issues left with one-setting-per-file now. So yes, I think it makes sense to review our position and make the thing as easy as possible to code and maintain. See Andres' list about that, earlier today on this same thread. Regards, -- Dimitri Fontaine 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 08/01/2013 07:47 AM, David Johnston wrote: > Minor request: could someone enlighten me as to why making the directory > location a compile-time option is undesirable. Packagers then can setup > whatever structure they desire when they compile their distributions. In > which case the discussion becomes what is a reasonable default and that can > be made with respect of other defaults that are in place for people that > would self-compile. Hey, that's a good idea. Anyone else? On 08/01/2013 06:32 AM, Greg Stark wrote:> On Thu, Aug 1, 2013 at 1:12 PM, Dimitri Fontaine wrote: >> we should review the implementation choice of the ALTER >> SYSTEM SET facility, and vote for having one-file-per-GUC. > > Zombie crazy design idea arise! > > I think people are going to laugh at us if an open source database > software can't manage a simple flat file database of settings, > especially one that is purely write-only and can be a simple dump of > settings that are set by alter system. While I find some value in the one-setting-per-file approach, there's also some major issues with it. And we already argued this out months ago, and ended up with the current single-file approach. Let's not rehash the past infinitely, please? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] pg_basebackup vs. Windows and tablespaces
On 08/01/2013 12:15 PM, Noah Misch wrote: A "pg_basebackup -Fp" running on the same system as the target cluster will fail in the presence of tablespaces; it would backup each tablespace to its original path, and those paths are in use locally for the very originals we're copying. "pg_basebackup -Ft" does not exhibit that hazard, and I typically recommend it for folks using tablespaces. On Windows, we populate pg_tblspc with NTFS junction points. "pg_basebackup -Fp" reproduces them, and "pg_basebackup -Ft" stores them in the tar archive as symbolic links. Trouble arises for -Ft backups: no Windows tar expander that I've found will recreate the junction points. While -Fp backups are basically usable, commands that copy files on Windows are inconsistent about their support for junction points; duplicating a base backup after the fact is error-prone. Windows users of tablespaces are left with limited options: use "pg_basebackup -Fp" on a different system, or use -Ft but manually recreate the junction points. We can do better; I see a few options: 1. Include in the base backup a file listing symbolic links/junction points, then have archive recovery recreate them. This file would be managed like the backup label file; exclusive backups would actually write it to the master data directory, and non-exclusive backups would incorporate it on the fly. pg_basebackup could also omit the actual links from its backup. Nearly any tar or file copy utility would then suffice. 2. Add a pg_basebackup option like "--destdir" or "--sysroot", meaningful only with -Fp; tablespace backups will be stored relative to it. So if the actual tablespace path is c:/foo, --destdir=c:/backups/today would backup that tablespace to c:/backups/today/c/foo. This facilitates same-server use of -Fp on all platforms. 3. Use path concatenation instead of symbolic links/junction points for tablespaces. More invasive, no doubt. For example, we would need to devise a way for recovery to get the tablespace path. I think #1 is a good bet; it's self-contained and fully heals the situation for Windows users. By itself, #2 helps less than #1 on Windows. It may have independent value. Other ideas, opinions? Thanks for raising this. I agree it's an area that needs work. I like #1, it seems nice and workable. I also like the concept of #2, but I think we need to think about it a bit more. One of the things I like about barman backups is that on recovery you can map where tablespaces go, on a per tablespace basis (it's not very well documented, or wasn't when I last looked, but it does work). I think something like that would be awesome to have for pg_basebackup. So allowing multiple options of the form --map-tablespace c:/foo/bar=d:/baz/blurfl or some such would be great. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Extension Templates S03E11
Hi, Please find attached to this email the latest and greatest version of in-line SQL only extensions support, known as "Extension Templates" and which could be renamed "In-Catalog Extension Templates". I've included a high-level description of the patch in a style that targets the detailed commit messages for features of that source code impact level. The attached patch is known to address all points raised in the previous reviews and to implement the best design we could come up with, thanks to immense helping from Tom, Heikki and Markus. Of course, bugs are all my precious. I'm going to register that patch to the next commitfest. It's not the only patch I intend to register for september though, as I want to get to a usable situation with Event Triggers, so you can expect a series of patches for that, covering what couldn't make it previously. As I think this WIP is about as ready-for-committer as it will ever be, it would be fantastic if we could do a single committer review before CF2013-09 so that I know that it's going to be accepted… or not. Well at least it's in the queue already, we'll see what can be done. Regards, --- Implement in-catalog Extension Template facility. Previously, the only way to CREATE EXTENSION involved installing file system templates in a place generally owned by root: creation scripts, upgrade scripts, main control file and auxilliary control files. This patch implements a way to upload all those resources into the catalogs, so that a PostgreSQL connection is all you need to make an extension available. By design and for security concerns the current Extension Template facility is not able to deal with extensions that need to load a DSO module into the backend. Using any other PL is supported though. An extension created from a template depends on it, and the templates are part of any backup script taken with pg_dump. So that at pg_restore time, when CREATE EXTENSION is executed the templates are already in place. To be able to do that, though, we need a difference in behavior in between the classic file system level templates and the catalog templates: there's no dependency tracking happening at all with file system templates and those can be changed at will even if an extension has been already instanciated from the templates, or even removed. Apart from the dependency tracking, the only other difference between file system templates and catalog templates for extensions is that the later are managed per-database. The file system level templates being managed per major version of PostgreSQL is considered a drawback of that method and not to be immitated by the in-catalog system, more flexible by design. At CREATE EXTENSION time, the file system templates are always prefered to the catalog templates. Also, it's prohibited to make available an extension in the catalogs if an extension of the same name is already available from file system templates. That said, some "race conditions" make it still possible to have the same extension name available as a file system template and a catalog template. Even if only the former will ever get installed, it's been deemed prudent to restrict the in-catalog templates for extensions to superusers only. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support templates.v11.patch.gz 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] pg_basebackup vs. Windows and tablespaces
Noah Misch writes: > A "pg_basebackup -Fp" running on the same system as the target cluster will > fail in the presence of tablespaces; it would backup each tablespace to its I'd like to see that fixed, +1. > 1. Include in the base backup a file listing symbolic links/junction points, > then have archive recovery recreate them. This file would be managed like the > backup label file; exclusive backups would actually write it to the master > data directory, and non-exclusive backups would incorporate it on the fly. > pg_basebackup could also omit the actual links from its backup. Nearly any > tar or file copy utility would then suffice. > > 2. Add a pg_basebackup option like "--destdir" or "--sysroot", meaningful only > with -Fp; tablespace backups will be stored relative to it. So if the actual > tablespace path is c:/foo, --destdir=c:/backups/today would backup that > tablespace to c:/backups/today/c/foo. This facilitates same-server use of -Fp > on all platforms. My understanding is that the second option here would be useful also when you want to create a standby with a different file layout than the master, which in some cases is what you want to do (not HA strictly). Another defect of pg_basebackup is its lack of shandling of tablespaces mounted within $PGDATA, which happens often enough at customers sites, whatever we think about that option. Would your work be extended to cover that too? Regards, -- Dimitri Fontaine 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] replication_reserved_connections
On Sun, Jul 28, 2013 at 2:50 PM, Andres Freund wrote: > On 2013-07-28 02:23:47 +0200, Marko Tiikkaja wrote: >> While you could limit the number of connections for non-replication roles, >> that's not always possible or desirable. I would like to introduce a way to >> reserve connection slots for replication. However, it's not clear how this >> would work. I looked at how superuser_reserved_connections is implented, >> and with small changes I could see how to implement two ideas: >> >> Does anyone see a better way to do this? I'm not too satisfied with either >> of these ideas. > > Personally I think we should just shouldn't allow normal connections for > the backend slots added by max_wal_senders. They are internally *added* > to max_connections, so limiting that seems perfectly fine to me since > the system provides max_connections connections externally. > > Hm... I wonder how that's managed for 9.4's max_worker_processes. See InitProcGlobal(). There are three lists of PGPROC objects. PGPROCs for incoming connections are pulled off of ProcGlobal->freeProcs, the autovacuum and its workers pull from ProcGlobal->autovacFreeProcs, and background workers pull from ProcGlobal->bgworkerFreeProcs. Auxiliary processes have a separate pool of PGPROCs to pull from, but they use linear search rather than a list, for reasons described in the comments in that function. There may be other checks elsewhere that enforce these same limits; not sure. -- 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] Regarding BGworkers
On Sun, Jul 28, 2013 at 1:26 AM, Amit kapila wrote:
> 1. Bgworker.c -
> FindRegisteredWorkerBySlotNumber()
> {
> ..
> /*
> * Copy contents of worker list into shared memory. Record the
> * shared memory slot assigned to each worker. This ensures
> * a 1-to-1 correspondence betwen the postmaster's private list and
> * the array in shared memory.
> */
> ..
> }
> a. Comment in function doesn't seem to be appropriate. It seems
> copy-pasted from function
> BackgroundWorkerShmemInit
> b. all function's except this have function header to explain a bit
> about function, though
> it might not be required here, but not sure so pointed.
Fixed.
> 2. Shouldn't function
> do_start_bgworker()/StartOneBackgroundWorker(void) be moved to bgworker.c
>as similar functions AutoVacWorkerMain()/PgArchiverMain() are in their
> respective files.
Yes, perhaps so. Other votes?
> 3. bgworker.h - file header still contains explanation only as per old
> functionality.
> Not sure, if it needs to be updated for new functionality of
> dynamic workers.
Fixed.
--
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
[HACKERS] pg_basebackup vs. Windows and tablespaces
A "pg_basebackup -Fp" running on the same system as the target cluster will fail in the presence of tablespaces; it would backup each tablespace to its original path, and those paths are in use locally for the very originals we're copying. "pg_basebackup -Ft" does not exhibit that hazard, and I typically recommend it for folks using tablespaces. On Windows, we populate pg_tblspc with NTFS junction points. "pg_basebackup -Fp" reproduces them, and "pg_basebackup -Ft" stores them in the tar archive as symbolic links. Trouble arises for -Ft backups: no Windows tar expander that I've found will recreate the junction points. While -Fp backups are basically usable, commands that copy files on Windows are inconsistent about their support for junction points; duplicating a base backup after the fact is error-prone. Windows users of tablespaces are left with limited options: use "pg_basebackup -Fp" on a different system, or use -Ft but manually recreate the junction points. We can do better; I see a few options: 1. Include in the base backup a file listing symbolic links/junction points, then have archive recovery recreate them. This file would be managed like the backup label file; exclusive backups would actually write it to the master data directory, and non-exclusive backups would incorporate it on the fly. pg_basebackup could also omit the actual links from its backup. Nearly any tar or file copy utility would then suffice. 2. Add a pg_basebackup option like "--destdir" or "--sysroot", meaningful only with -Fp; tablespace backups will be stored relative to it. So if the actual tablespace path is c:/foo, --destdir=c:/backups/today would backup that tablespace to c:/backups/today/c/foo. This facilitates same-server use of -Fp on all platforms. 3. Use path concatenation instead of symbolic links/junction points for tablespaces. More invasive, no doubt. For example, we would need to devise a way for recovery to get the tablespace path. I think #1 is a good bet; it's self-contained and fully heals the situation for Windows users. By itself, #2 helps less than #1 on Windows. It may have independent value. Other ideas, opinions? Thanks, nm -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improve Chinese locale performance
On Sun, Jul 28, 2013 at 5:39 AM, Martijn van Oosterhout wrote: > On Tue, Jul 23, 2013 at 10:34:21AM -0400, Robert Haas wrote: >> I pretty much lost interest in ICU upon reading that they use UTF-16 >> as their internal format. >> >> http://userguide.icu-project.org/strings#TOC-Strings-in-ICU > > The UTF-8 support has been steadily improving: > > For example, icu::Collator::compareUTF8() compares two UTF-8 strings > incrementally, without converting all of the two strings to UTF-16 if > there is an early base letter difference. > > http://userguide.icu-project.org/strings/utf-8 > > For all other encodings you should be able to use an iterator. As to > performance I have no idea. > > The main issue with strxfrm() is its lame API. If it supported > returning prefixes you'd be set, but as it is you need >10MB of memory > just to transform a 10MB string, even if only the first few characers > would be enough to sort... Yep, definitely. And by ">10MB" you mean ">90MB", at least on my Mac, which is really outrageous. -- 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] new "row-level lock" error messages
Robert Haas escribió: > The fact that there are no tests of this functionality is probably not > a good thing. We should add some. No disagreement. > At the moment, the following test > case crashes, and it looks like this commit is responsible: > > create table test_update2 (id integer); > DECLARE test_update_cursor CURSOR FOR SELECT id, MIN(id) FROM > test_update2 GROUP By id HAVING MIN(id) < 1 FOR UPDATE; Grr. Thanks. Will fix. -- Álvaro Herrerahttp://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] new "row-level lock" error messages
On Tue, Jul 23, 2013 at 2:16 PM, Alvaro Herrera wrote: > Alvaro Herrera wrote: >> Peter Eisentraut wrote: >> >> > I would suggest that these changes be undone, except that the old >> > "SELECT FOR ..." be replaced by a dynamic string that reverse-parses the >> > LockingClause to provide the actual clause that was used. >> >> Here's a patch for this. > > Pushed to 9.3 and master. Sample output: > > alvherre=# select * from foo, bar for update of foof for share of bar; > ERROR: relation "foof" in FOR UPDATE clause not found in FROM clause > > alvherre=# select * from foo, bar for update of foo for share of barf; > ERROR: relation "barf" in FOR SHARE clause not found in FROM clause > > Amusingly, the only test in which these error messages appeared, in > contrib/file_fdw, was removed after the two commits that changed the > wording. So there's not a single test which needed to be tweaked for > this change. The fact that there are no tests of this functionality is probably not a good thing. We should add some. At the moment, the following test case crashes, and it looks like this commit is responsible: create table test_update2 (id integer); DECLARE test_update_cursor CURSOR FOR SELECT id, MIN(id) FROM test_update2 GROUP By id HAVING MIN(id) < 1 FOR UPDATE; -- 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] [9.3 bug] disk space in pg_xlog increases during archive recovery
From: "Fujii Masao" However, isn't StandbyRequested true (= standby_mode set to on) to enable warm standby? We can set up warm-standby by using pg_standby even if standby_mode = off. I see. However, I understand that pg_standby is a legacy feature, and the current way to setup a warm standby is to set standby=on and restore_command in recovery.conf. So I think it might not be necessary to support cascading replication with pg_standby, if supporting it would prevent better implementation of new features. I'm afraid people set max_wal_senders>0 and hot_standby=on even on the primary server to make the contents of postgresql.conf identical on both the primary and the standby for easier configuration. If so, normal archive recovery (PITR, not the standby recovery) would face the original problem -- unnecessary WAL accumulation in pg_xlog/. So I'm wonder if AllowCascadeReplication() is enough. One idea is to add new GUC parameter like enable_cascade_replication and then prevent WAL file from being kept in pg_xlog if that parameter is off. But we cannot apply such change into the already-released version 9.2. Yes, I thought the same, too. Adding a new GUC to enable cascading replication now would be a usability degradation. But I have no idea of any better way. It seems we need to take either v1 or v2 of the patch I sent. If we consider that we don't have to support cascading replication for a legacy pg_standby, v1 patch is definitely better because the standby server would not keep restored archive WAL in pg_xlog when cascading replication is not used. Otherwise, we have to take v2 patch. Could you choose either and commit it? Regards MauMau -- 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Hi, On 2013-08-01 15:40:22 +0100, Greg Stark wrote: > Why isn't it enough to just dump out all variables with a source of alter > system to a text file? You can either have a single global lock around that > operation or write it to a new file and move it into place. It saves you from several complications: * No need to iterate over all GUCs, figuring out which come from which source, when writing out the file. * Less logic required when writing out a value, since you have an acceptable input ready. * No need to make sure the autogenerated file is written out in the same order when adding/changing a setting (to make sure you can diff/version control it sensibly) * No locking necessary, without locking concurrent changes can vanish. * Way easier to delete a setting if it ends up stopping postgres from starting. Greetings, Andres Freund PS: .oO(quoting?) -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Minor request: could someone enlighten me as to why making the directory location a compile-time option is undesirable. Packagers then can setup whatever structure they desire when they compile their distributions. In which case the discussion becomes what is a reasonable default and that can be made with respect of other defaults that are in place for people that would self-compile. I get the "supporting users - telling them where to go to find these files" aspect but I believe that ship has already sailed. The goal should be to make it as easy as possible to allow distributions and/or individual users to integrate PostgreSQL into their normal routine as possible. It isn't like we are adding unneeded complexity since it is obvious from the discussion that where files/directories are placed in the file system is a major variable. Enforcing $PGDATA when we know Debian is going to be upset doesn't seem to be that great an idea - it isn't like we are going to suddenly make them realize they have been doing things incorrectly all this time. I am not familiar with all of the configurations but I do recall that the location of postgres.conf and related files is already distribution specific so why shouldn't these extensions be as well? Sorry if this was discussed previously; I'll go look deeper in the thread if someone confirms that indeed it is. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Proposal-for-Allow-postgresql-conf-values-to-be-changed-via-SQL-tp5729917p5765892.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Why isn't it enough to just dump out all variables with a source of alter system to a text file? You can either have a single global lock around that operation or write it to a new file and move it into place. -- greg On 1 Aug 2013 15:19, "Andres Freund" wrote: > On 2013-08-01 15:17:04 +0100, Greg Stark wrote: > > We don't need per guc locking. This is the whole objection Tom had about > > this patch being more complex than it has to be. > > IIRC he objected to using locking *at all* because a simple > one-file-per-setting approach should be used. > > Greetings, > > Andres Freund > > -- > Andres Freund http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Training & Services >
[HACKERS] Re: How to configer the pg_hba record which the database name with "\n" ?
huxm wrote > where there is a > newline(\n) in the name. I can't imagine why you would want to use non-printing characters in a name, especially a database name. Even if the hba.conf file was able to interpret it (which it probably cannot but I do not know for certain) client interfaces are likely to have problems as well. Most of these would not think of interpolating a database identifier in that manner but instead treat the name as a literal value. Even when line-continuations are allowed they are often cosmetic in nature and the resultant newline is discarded during the pre-execution phase of the command interpreter. Arguably having a check constraint on the catalog to prohibit such a name would be more useful than trying to make such a construct functional. I'd guess in the immediate term the users accessing this database would need to have "all" as their target and then you use role-based authorization to limit which specific databases are accessible. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/How-to-configer-the-pg-hba-record-which-the-database-name-with-n-tp5765847p5765889.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] libpq thread locking during SSL connection start
All, I wanted to highlight the below commit as being a significant enough change that it warrents being seen on -hackers and not just -committers. If you use SSL with libpq, particularly in a threaded mode/environment, please take a look/test this change. Prior to the patch, we would crash pretty easily when using client certificates with multiple threads; I was unable to get it to crash after the patch. That said, there's also risk that this patch will cause slow-downs in SSL connections when using threads due to the additional locking. I'm not sure that's a heavily used case, but none-the-less, if you do that, please considering testing this patch. Thanks! Stephen - Forwarded message from Stephen Frost - Date: Thu, 01 Aug 2013 05:33:12 + From: Stephen Frost To: pgsql-committ...@postgresql.org X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 Subject: [COMMITTERS] pgsql: Add locking around SSL_context usage in libpq Add locking around SSL_context usage in libpq I've been working with Nick Phillips on an issue he ran into when trying to use threads with SSL client certificates. As it turns out, the call in initialize_SSL() to SSL_CTX_use_certificate_chain_file() will modify our SSL_context without any protection from other threads also calling that function or being at some other point and trying to read from SSL_context. To protect against this, I've written up the attached (based on an initial patch from Nick and much subsequent discussion) which puts locks around SSL_CTX_use_certificate_chain_file() and all of the other users of SSL_context which weren't already protected. Nick Phillips, much reworked by Stephen Frost Back-patch to 9.0 where we started loading the cert directly instead of using a callback. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/aad2a630b1b163038ea904e16a59e409020f5828 Modified Files -- src/interfaces/libpq/fe-secure.c | 56 -- 1 file changed, 53 insertions(+), 3 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committ...@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers - End forwarded message - signature.asc Description: Digital signature
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-01 15:17:04 +0100, Greg Stark wrote: > We don't need per guc locking. This is the whole objection Tom had about > this patch being more complex than it has to be. IIRC he objected to using locking *at all* because a simple one-file-per-setting approach should be used. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
We don't need per guc locking. This is the whole objection Tom had about this patch being more complex than it has to be. -- greg On 1 Aug 2013 14:55, "Dimitri Fontaine" wrote: > Greg Stark writes: > > I think people are going to laugh at us if an open source database > > software can't manage a simple flat file database of settings, > > especially one that is purely write-only and can be a simple dump of > > settings that are set by alter system. > > So you say it's easier to implement per-GUC locking semantics correctly > when using a single file with multiple units of information that all are > of the same type? Interesting. > > Maybe the storage should actually be a shared catalog, in fact. > > Regards, > -- > Dimitri Fontaine > 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] make --enable-depend the default
On 2013-08-01 08:34:51 -0400, Tom Lane wrote: > Andres Freund writes: > > People, including me, every now and then forget to pass --enable-depend > > to configure (when not using my own environment). Which then leads to > > strange errors that cost time to track down... > > > Thus I'd like to enable dependency tracking by default. Given todays > > computing powers there doesn't seem to be much reason not to do so. > > > Any arguments against? > > Yes: it's a waste of resources for one-shot builds, which is what most > people not reading this list do (and by asking here, you're biasing > your poll). I think the difference is resource usage is minimal enough for those cases to not really matter. We only seem to support gcc's in-line generation, so there's no separate gcc invocation for dependency generation. rm -rf /tmp/pgbuild/* && mkdir -p /tmp/pgbuild/ && cd /tmp/pgbuild && \ ~/src/postgresql/configure --enable-depend && time make -j3 -s All of PostgreSQL successfully made. Ready to install. real 2m33.248s user 3m58.657s sys0m26.282s Without --enable-depend: All of PostgreSQL successfully made. Ready to install. real 2m32.853s user 3m57.639s sys0m25.741s ccached, fully cached: make clean && time make -j3 -s: real0m7.394s user0m8.446s sys 0m3.800s without dependencies: real0m6.484s user0m7.824s sys 0m3.278s So if anything, that's the painpoint. > Personally I always do "make clean", if not "make distclean", after a git > pull. If you're using ccache it's incredibly cheap to just rebuild the > whole tree every time, and I trust the results a lot more than I do > --enable-depend. Funnily I repeatedly had problems that could only be solved by clearing ccache's cache... I am not really worried about rebuilds after a pull, but more about iterative rebuilds while writing code. Including header changes. > [postgres@sss1 pgsql]$ time make -s clean > > real0m1.613s > user0m0.994s > sys 0m0.254s > [postgres@sss1 pgsql]$ time make -s -j8 > In file included from gram.y:13635: > scan.c: In function 'yy_try_NUL_trans': > scan.c:10167: warning: unused variable 'yyg' > All of PostgreSQL successfully made. Ready to install. > > real0m2.483s > user0m6.693s > sys 0m2.123s > > (make installcheck-parallel takes 13.6 seconds on this machine...) Hrmpf. It takes longer than that for me. I need a new laptop... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-01 15:55:25 +0200, Dimitri Fontaine wrote: > Greg Stark writes: > > I think people are going to laugh at us if an open source database > > software can't manage a simple flat file database of settings, > > especially one that is purely write-only and can be a simple dump of > > settings that are set by alter system. Why would using a single-file solution imply that we cannot do the other? > So you say it's easier to implement per-GUC locking semantics correctly > when using a single file with multiple units of information that all are > of the same type? Interesting. > Maybe the storage should actually be a shared catalog, in fact. We can't read those early enough. Think a) of options that can only be set at postmaster start b) think of crash recovery. We can't read the catalog till we're in a consistent state. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Greg Stark writes: > I think people are going to laugh at us if an open source database > software can't manage a simple flat file database of settings, > especially one that is purely write-only and can be a simple dump of > settings that are set by alter system. So you say it's easier to implement per-GUC locking semantics correctly when using a single file with multiple units of information that all are of the same type? Interesting. Maybe the storage should actually be a shared catalog, in fact. Regards, -- Dimitri Fontaine 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thu, Aug 1, 2013 at 1:12 PM, Dimitri Fontaine wrote: > we should review the implementation choice of the ALTER > SYSTEM SET facility, and vote for having one-file-per-GUC. Zombie crazy design idea arise! I think people are going to laugh at us if an open source database software can't manage a simple flat file database of settings, especially one that is purely write-only and can be a simple dump of settings that are set by alter system. -- 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] make --enable-depend the default
* Tom Lane (t...@sss.pgh.pa.us) wrote: > whole tree every time, and I trust the results a lot more than I do > --enable-depend. This is a much more compelling point, imv. We definitely still have issues around dependencies not being completely tracked, even with --enable-depend. Makes one wonder if maybe we should flip this around and *remove* it.. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] make --enable-depend the default
Andres Freund writes: > People, including me, every now and then forget to pass --enable-depend > to configure (when not using my own environment). Which then leads to > strange errors that cost time to track down... > Thus I'd like to enable dependency tracking by default. Given todays > computing powers there doesn't seem to be much reason not to do so. > Any arguments against? Yes: it's a waste of resources for one-shot builds, which is what most people not reading this list do (and by asking here, you're biasing your poll). Personally I always do "make clean", if not "make distclean", after a git pull. If you're using ccache it's incredibly cheap to just rebuild the whole tree every time, and I trust the results a lot more than I do --enable-depend. [postgres@sss1 pgsql]$ time make -s clean real0m1.613s user0m0.994s sys 0m0.254s [postgres@sss1 pgsql]$ time make -s -j8 In file included from gram.y:13635: scan.c: In function 'yy_try_NUL_trans': scan.c:10167: warning: unused variable 'yyg' All of PostgreSQL successfully made. Ready to install. real0m2.483s user0m6.693s sys 0m2.123s (make installcheck-parallel takes 13.6 seconds on this machine...) 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Greg Stark writes: > Greg Smith's argument was about recovery.conf which is a file that > users are expected to edit. A file which user's are not expected to > edit and is maintained by the software is no more a configuration file > than pg_auth or pg_database which are actually being stored in the > database itself. +1 > I think we're fine with allowing users to use both but we should try > to keep the two as separate as possible to avoid confusion. Keeping > the auto.conf inside conf.d sounds like it will actually confuse users > about which files they're supposed to edit and which belong to the > system. +1 I think we need both an ALTER SYSTEM SET implementation using files somewhere in $PGDATA, and a separate conf.d facility that lives alongside wherever the main postgresql.conf is maintained on your OS of choice. Also, now that we have decided to separate away those two fundamentally different aspects of the configuration setup, I join Álvaro and Cédric in thinking that we should review the implementation choice of the ALTER SYSTEM SET facility, and vote for having one-file-per-GUC. Regards, -- Dimitri Fontaine 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
[HACKERS] make --enable-depend the default
Hi, People, including me, every now and then forget to pass --enable-depend to configure (when not using my own environment). Which then leads to strange errors that cost time to track down... Thus I'd like to enable dependency tracking by default. Given todays computing powers there doesn't seem to be much reason not to do so. Any arguments against? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Tuesday, July 30, 2013 10:25 PM Josh Berkus wrote: > Amit, > > > I had already sent the updated patch based on recent suggestions. > > Yes, but there is no consensus yet on certain fundamental issues, such > as: > > 1. what directory should postgresql.auto.conf live in? Current situation is that Greg Smith has provided a usecase for all config files to be present in location other than $PGDATA, but based on further suggestions given by Greg Stark and Stephen Frost it is clear that keeping postgresql.auto.conf will not harm the users of Debian. More to that point, I think if we try to address conf.d (for maintaining config files) along with this Patch, it will not be possible to get every one on same plane. I think apart from Greg Stark and Stephen Frost other people (Tom, Robert) are of opinion that auto file should be placed in $PGDATA, so shouldn't we consider this as consensus and move forward for this point unless Tom or Robert have changed their view based on usecase provided by Greg Smith? > 2. should admins be able to turn it "off"? IIRC there are not many people who want this feature to be turned off. Peter E. had asked seeing the complexity, but in general I don't think This is a blocking point for patch, am I missing something here? > Once we have consensus on these issues, then hopefully we can commit > your patch. I don't *think* anyone is arguing with the code at this > stage. Sure, I understand that it is important to have consensus and agreement on above points. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Misplaced BKI entries in pg_amproc.h
While checking something, I noticed that opfamilies 3626, 3683, 3901 (all btree AM), 3903 (hash) and 3919 (gist) are all defined in the section marked as "gin". (I'm not sure if it helps to deliver a patch - it may be easier for the committer to move the items himself than to check if the diff is correct) // Antonin Houska (Tony) -- 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] REGEXP_MATCHES() strange behavior with '^' and '$' pattern
On Thu, Aug 1, 2013 at 12:25 PM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > > > > On Wed, Jul 31, 2013 at 7:47 PM, Tom Lane wrote: > >> Jeevan Chalke writes: >> > Oops forgot patch. >> > Attached now. >> >> Hmm ... I think the logic change is good, but two demerits for not fixing >> the adjacent comment. >> > > I had a look over comments and somehow I found that OK. > > Anyway, updated comments in this version of patch. > It looks like you have committed the changes with updated comments and more test-cases. Thanks > > Thanks > > >> >> regards, tom lane >> > > > > -- > Jeevan B Chalke > > -- Jeevan B Chalke Senior Software Engineer, R&D EnterpriseDB Corporation The Enterprise PostgreSQL Company Phone: +91 20 30589500 Website: www.enterprisedb.com EnterpriseDB Blog: http://blogs.enterprisedb.com/ Follow us on Twitter: http://www.twitter.com/enterprisedb This e-mail message (and any attachment) is intended for the use of the individual or entity to whom it is addressed. This message contains information from EnterpriseDB Corporation that may be privileged, confidential, or exempt from disclosure under applicable law. If you are not the intended recipient or authorized to receive this for the intended recipient, any use, dissemination, distribution, retention, archiving, or copying of this communication is strictly prohibited. If you have received this e-mail in error, please notify the sender immediately by reply e-mail and delete this message.
Re: [HACKERS] FailedAssertion on initdb with 9.4dev
On Thu, Aug 1, 2013 at 3:52 PM, Amit Langote wrote: > Build using: > CFLAGS="-g -O0" ./configure --with-pam --enable-cassert --enable-debug > --prefix=/home/amit/dev/pginstall/ > > -- > Amit Langote Umm, I guess I forgot to "make clean" before building with the latest HEAD. Now, I rebuilt after "make clean", it no longer gives me the FailedAssertion as reported before. Sorry about the noise! -- Amit Langote -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
