Re: [HACKERS] proposal: make NOTIFY list de-duplication optional

2016-02-20 Thread Filip Rembiałkowski
On Fri, Feb 19, 2016 at 10:09 PM, Catalin Iacob 
wrote:

> On 2/9/16, Tom Lane  wrote:
> > FWIW, I think it would be a good thing if the NOTIFY statement syntax
> were
> > not remarkably different from the syntax used in the pg_notify() function
> > call.  To do otherwise would certainly be confusing.  So on the whole
> > I'd go with the "NOTIFY channel [ , payload [ , mode ] ]" option.
>
> I'm quite interested in getting this addressed in time for 9.6 as I'll
> be using NOTIFY extensively in a project and I agree with Craig that
> the deduplication is frustrating both because you sometimes want every
> event and because it can apparently cause O(n^2) behaviour (which I
> didn't know before this thread). If another use case for suppressing
> deduplication is needed, consider publishing events like "inserted
> tuple", "deleted tuple" from triggers and a transaction that does
> "insert, delete, insert" which the client then sees as "insert,
> delete, oops nothing else".
>
> Tom's proposal allows for more flexible modes than just the ALL and
> DISTINCT keywords and accommodates the concern that DISTINCT will lead
> to bug reports about not really being distinct due to savepoints.
>
> Android has a similar thing for push notifications to mobile devices
> which they call collapse:
> https://developers.google.com/cloud-messaging/concept-options, search
> for collapse_key.
>
> So I propose NOTIFY channel [ , payload [ , collapse_mode ] ] with
> collapse mode being:
>
> * 'never'
>   ** Filip's proposed behaviour for the ALL option
>   ** if specified, every notification is queued regardless what's in the
> queue
>
> * 'maybe'
>   ** vague word allowing for flexibility in what the server decides to do
>   ** current behaviour
>   ** improves performance for big transactions if a row trigger
> creates the same payload over and over one after the other due to the
> current optimization of checking the tail of the list
>   ** has performance problems O(n^2) for big transactions with
> different payloads
>   *** the performance problems can be addressed by a different
> patch which uses a hash table, or decides to collapse less
> aggressively (Tom's check last 100 idea), or whatever else
>   *** in the meantime the 'never' mode acts as a good workaround
>
> In the future we might support an 'always' collapse_mode which would
> really be always, including across savepoints. Or an
> 'only_inside_savepoints' which guarantees the current behaviour.
>
> Filip, do you agree with Tom's proposal? Do you plan to rework the
> patch on these lines? If you are I'll try to review it, if not I could
> give it a shot as I'm interested in having this in 9.6.
>

I see that Tom's remarks give more flexibility, and your refinement makes
sense.

I was stuck because both syntaxes have their ugliness. NOTIFY allows the
payload to be NULL:
NOTIFY chan01;

How would this look like in "never" mode?
NOTIFY chan01, NULL, 'never'; -- seems very cryptic.


Re: [HACKERS] proposal: make NOTIFY list de-duplication optional

2016-02-18 Thread Filip Rembiałkowski
Another update - separated new internal function to satisfy opr_sanity.sql
diff --git a/contrib/tcn/tcn.c b/contrib/tcn/tcn.c
index 7352b29..3a6d4f5 100644
--- a/contrib/tcn/tcn.c
+++ b/contrib/tcn/tcn.c
@@ -160,7 +160,7 @@ triggered_change_notification(PG_FUNCTION_ARGS)
 	strcpy_quoted(payload, SPI_getvalue(trigtuple, tupdesc, colno), '\'');
 }
 
-Async_Notify(channel, payload->data);
+Async_Notify(channel, payload->data, false);
 			}
 			ReleaseSysCache(indexTuple);
 			break;
diff --git a/doc/src/sgml/ref/notify.sgml b/doc/src/sgml/ref/notify.sgml
index 4dd5608..933c76c 100644
--- a/doc/src/sgml/ref/notify.sgml
+++ b/doc/src/sgml/ref/notify.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  
 
-NOTIFY channel [ , payload ]
+NOTIFY [ ALL | DISTINCT ] channel [ , payload ]
 
  
 
@@ -105,6 +105,10 @@ NOTIFY channel [ , ALL is specified (contrary to DISTINCT, the
+   default), the server will deliver all notifications, including duplicates.
+   Removal of duplicate notifications takes place within transaction block,
+   finished with COMMIT, END or SAVEPOINT.
   
 
   
@@ -190,6 +194,12 @@ NOTIFY channel [ , NOTIFY command if you need to work with
 non-constant channel names and payloads.

+   
+There is a three-argument version, pg_notify(text,
+text, boolean). The third argument acts like
+the ALL keyword when set to true, and
+DISTINCT when set to false. 
+   
   
  
 
@@ -210,6 +220,21 @@ Asynchronous notification "virtual" with payload "This is the payload" received
 LISTEN foo;
 SELECT pg_notify('fo' || 'o', 'pay' || 'load');
 Asynchronous notification "foo" with payload "payload" received from server process with PID 14728.
+
+/* Identical messages from same (sub-) transaction can be eliminated - unless you use the ALL keyword */
+LISTEN bar;
+BEGIN;
+NOTIFY bar, 'Coffee please';
+NOTIFY bar, 'Coffee please';
+NOTIFY bar, 'Milk please';
+NOTIFY ALL bar, 'Milk please';
+SAVEPOINT s;
+NOTIFY bar, 'Coffee please';
+COMMIT;
+Asynchronous notification "bar" with payload "Coffee please" received from server process with PID 31517.
+Asynchronous notification "bar" with payload "Milk please" received from server process with PID 31517.
+Asynchronous notification "bar" with payload "Milk please" received from server process with PID 31517.
+Asynchronous notification "bar" with payload "Coffee please" received from server process with PID 31517.
 
  
 
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index c39ac3a..54d1680 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -524,7 +524,42 @@ pg_notify(PG_FUNCTION_ARGS)
 	/* For NOTIFY as a statement, this is checked in ProcessUtility */
 	PreventCommandDuringRecovery("NOTIFY");
 
-	Async_Notify(channel, payload);
+	Async_Notify(channel, payload, false);
+
+	PG_RETURN_VOID();
+}
+
+
+/*
+ * pg_notify_3args
+ *SQL function to send a notification event, 3-argument version
+ */
+Datum
+pg_notify_3args(PG_FUNCTION_ARGS)
+{
+	const char *channel;
+	const char *payload;
+	bool use_all;
+
+	if (PG_ARGISNULL(0))
+		channel = "";
+	else
+		channel = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+	if (PG_ARGISNULL(1))
+		payload = "";
+	else
+		payload = text_to_cstring(PG_GETARG_TEXT_PP(1));
+
+	if (PG_ARGISNULL(2))
+		use_all = false;
+	else
+		use_all = PG_GETARG_BOOL(2);
+
+	/* For NOTIFY as a statement, this is checked in ProcessUtility */
+	PreventCommandDuringRecovery("NOTIFY");
+
+	Async_Notify(channel, payload, use_all);
 
 	PG_RETURN_VOID();
 }
@@ -540,7 +575,7 @@ pg_notify(PG_FUNCTION_ARGS)
  *		^^
  */
 void
-Async_Notify(const char *channel, const char *payload)
+Async_Notify(const char *channel, const char *payload, bool use_all)
 {
 	Notification *n;
 	MemoryContext oldcontext;
@@ -570,9 +605,10 @@ Async_Notify(const char *channel, const char *payload)
 	 errmsg("payload string too long")));
 	}
 
-	/* no point in making duplicate entries in the list ... */
-	if (AsyncExistsPendingNotify(channel, payload))
-		return;
+	if (!use_all)
+		/* remove duplicate entries in the list */
+		if (AsyncExistsPendingNotify(channel, payload))
+			return;
 
 	/*
 	 * The notification list needs to live until end of transaction, so store
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b307b48..7203f4a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8528,11 +8528,12 @@ DropRuleStmt:
  *
  */
 
-NotifyStmt: NOTIFY ColId notify_payload
+NotifyStmt: NOTIFY all_or_distinct ColId notify_payload
 {
 	NotifyStmt *n = makeNode(NotifyStmt);
-	n->conditionname = $2;
-	n->payload = $3;
+	n->use_all = $2;
+	n->conditionname = $3;
+	n->payload = $4;
 	$$ = (Node *)n;
 }
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 045f7f0..0e50561 100644
--- 

Re: [HACKERS] proposal: make NOTIFY list de-duplication optional

2016-02-15 Thread Filip Rembiałkowski
Small update. I had to add one thing in /contrib/tcn/.
diff --git a/contrib/tcn/tcn.c b/contrib/tcn/tcn.c
index 7352b29..3a6d4f5 100644
--- a/contrib/tcn/tcn.c
+++ b/contrib/tcn/tcn.c
@@ -160,7 +160,7 @@ triggered_change_notification(PG_FUNCTION_ARGS)
 	strcpy_quoted(payload, SPI_getvalue(trigtuple, tupdesc, colno), '\'');
 }
 
-Async_Notify(channel, payload->data);
+Async_Notify(channel, payload->data, false);
 			}
 			ReleaseSysCache(indexTuple);
 			break;
diff --git a/doc/src/sgml/ref/notify.sgml b/doc/src/sgml/ref/notify.sgml
index 4dd5608..933c76c 100644
--- a/doc/src/sgml/ref/notify.sgml
+++ b/doc/src/sgml/ref/notify.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  
 
-NOTIFY channel [ , payload ]
+NOTIFY [ ALL | DISTINCT ] channel [ , payload ]
 
  
 
@@ -105,6 +105,10 @@ NOTIFY channel [ , ALL is specified (contrary to DISTINCT, the
+   default), the server will deliver all notifications, including duplicates.
+   Removal of duplicate notifications takes place within transaction block,
+   finished with COMMIT, END or SAVEPOINT.
   
 
   
@@ -190,6 +194,12 @@ NOTIFY channel [ , NOTIFY command if you need to work with
 non-constant channel names and payloads.

+   
+There is a three-argument version, pg_notify(text,
+text, boolean). The third argument acts like
+the ALL keyword when set to true, and
+DISTINCT when set to false. 
+   
   
  
 
@@ -210,6 +220,21 @@ Asynchronous notification "virtual" with payload "This is the payload" received
 LISTEN foo;
 SELECT pg_notify('fo' || 'o', 'pay' || 'load');
 Asynchronous notification "foo" with payload "payload" received from server process with PID 14728.
+
+/* Identical messages from same (sub-) transaction can be eliminated - unless you use the ALL keyword */
+LISTEN bar;
+BEGIN;
+NOTIFY bar, 'Coffee please';
+NOTIFY bar, 'Coffee please';
+NOTIFY bar, 'Milk please';
+NOTIFY ALL bar, 'Milk please';
+SAVEPOINT s;
+NOTIFY bar, 'Coffee please';
+COMMIT;
+Asynchronous notification "bar" with payload "Coffee please" received from server process with PID 31517.
+Asynchronous notification "bar" with payload "Milk please" received from server process with PID 31517.
+Asynchronous notification "bar" with payload "Milk please" received from server process with PID 31517.
+Asynchronous notification "bar" with payload "Coffee please" received from server process with PID 31517.
 
  
 
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index c39ac3a..38a8246 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -510,6 +510,7 @@ pg_notify(PG_FUNCTION_ARGS)
 {
 	const char *channel;
 	const char *payload;
+	bool use_all;
 
 	if (PG_ARGISNULL(0))
 		channel = "";
@@ -521,10 +522,15 @@ pg_notify(PG_FUNCTION_ARGS)
 	else
 		payload = text_to_cstring(PG_GETARG_TEXT_PP(1));
 
+	if (PG_NARGS() > 2 && ! PG_ARGISNULL(2))
+		use_all = PG_GETARG_BOOL(2);
+	else
+		use_all = false;
+
 	/* For NOTIFY as a statement, this is checked in ProcessUtility */
 	PreventCommandDuringRecovery("NOTIFY");
 
-	Async_Notify(channel, payload);
+	Async_Notify(channel, payload, use_all);
 
 	PG_RETURN_VOID();
 }
@@ -540,7 +546,7 @@ pg_notify(PG_FUNCTION_ARGS)
  *		^^
  */
 void
-Async_Notify(const char *channel, const char *payload)
+Async_Notify(const char *channel, const char *payload, bool use_all)
 {
 	Notification *n;
 	MemoryContext oldcontext;
@@ -570,9 +576,10 @@ Async_Notify(const char *channel, const char *payload)
 	 errmsg("payload string too long")));
 	}
 
-	/* no point in making duplicate entries in the list ... */
-	if (AsyncExistsPendingNotify(channel, payload))
-		return;
+	if (!use_all)
+		/* remove duplicate entries in the list */
+		if (AsyncExistsPendingNotify(channel, payload))
+			return;
 
 	/*
 	 * The notification list needs to live until end of transaction, so store
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b307b48..7203f4a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8528,11 +8528,12 @@ DropRuleStmt:
  *
  */
 
-NotifyStmt: NOTIFY ColId notify_payload
+NotifyStmt: NOTIFY all_or_distinct ColId notify_payload
 {
 	NotifyStmt *n = makeNode(NotifyStmt);
-	n->conditionname = $2;
-	n->payload = $3;
+	n->use_all = $2;
+	n->conditionname = $3;
+	n->payload = $4;
 	$$ = (Node *)n;
 }
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 045f7f0..0e50561 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -599,7 +599,7 @@ standard_ProcessUtility(Node *parsetree,
 NotifyStmt *stmt = (NotifyStmt *) parsetree;
 
 PreventCommandDuringRecovery("NOTIFY");
-Async_Notify(stmt->conditionname, stmt->payload);
+Async_Notify(stmt->conditionname, stmt->payload, stmt->use_all);
 			}
 			break;
 
diff --git 

Re: [HACKERS] proposal: make NOTIFY list de-duplication optional

2016-02-09 Thread Filip Rembiałkowski
On Tue, Feb 9, 2016 at 12:15 AM, Merlin Moncure  wrote:

> I wonder if the third argument
> should be a boolean however.  If we make it 'text, 'send mode',
> instead, we could leave some room for more specialization of the
> queuing behavior.
>
> For example, we've had a couple of requests over the years to have an
> 'immediate' mode which dumps the notification immediately to the
> client without waiting for tx commit. This may or may not be a good
> idea, but if it was ultimately proved to be, it could be introduced as
> an alternate mode without adding an extra function.

But then it becomes disputable if SQL syntax change makes sense.

---we had this,
 NOTIFY channel [ , payload ]
---and in this patch we have this
NOTIFY [ ALL | DISTINCT ] channel [ , payload ]
 ---  but maybe we should have this?
NOTIFY channel [ , payload [ , mode ] ]

I'm not sure which direction is better with non-standard SQL additions.
Recycling keywords or adding more commas?


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


Re: [HACKERS] proposal: make NOTIFY list de-duplication optional

2016-02-08 Thread Filip Rembiałkowski
On Mon, Feb 8, 2016 at 1:52 PM, Craig Ringer  wrote:

> Would it be correct to say that if ALL is specified then a message is queued
> no matter what. If DISTINCT is specified then it is only queued if no
> message with the same channel and argument is already queued for delivery.
Yes, exactly.

> Using DISTINCT can never decrease the total number of messages to be sent.
This sentence does not sound true. DISTINCT is the default, old
behaviour. It *can* decrease total number of messages (by
deduplication)

> I've found the deduplication functionality of NOTIFY very frustrating in the 
> past
> and I see this as a significant improvement. Sometimes the *number of times*
> something happened is significant too...
yep, same idea here.




Here is my next try, after suggestions from -perf and -hackers list:

* no GUC

* small addition to NOTIFY grammar: NOTIFY ALL/DISTINCT

* corresponding, 3-argument version of pg_notify(text,text,bool)

* updated the docs to include new syntax and clarify behavior

* no hashtable in AsyncExistsPendingNotify
 (I don't see much sense in that part; and it can be well done
separately from this)
diff --git a/doc/src/sgml/ref/notify.sgml b/doc/src/sgml/ref/notify.sgml
index 4dd5608..933c76c 100644
--- a/doc/src/sgml/ref/notify.sgml
+++ b/doc/src/sgml/ref/notify.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  
 
-NOTIFY channel [ , payload ]
+NOTIFY [ ALL | DISTINCT ] channel [ , payload ]
 
  
 
@@ -105,6 +105,10 @@ NOTIFY channel [ , ALL is specified (contrary to DISTINCT, the
+   default), the server will deliver all notifications, including duplicates.
+   Removal of duplicate notifications takes place within transaction block,
+   finished with COMMIT, END or SAVEPOINT.
   
 
   
@@ -190,6 +194,12 @@ NOTIFY channel [ , NOTIFY command if you need to work with
 non-constant channel names and payloads.

+   
+There is a three-argument version, pg_notify(text,
+text, boolean). The third argument acts like
+the ALL keyword when set to true, and
+DISTINCT when set to false. 
+   
   
  
 
@@ -210,6 +220,21 @@ Asynchronous notification "virtual" with payload "This is the payload" received
 LISTEN foo;
 SELECT pg_notify('fo' || 'o', 'pay' || 'load');
 Asynchronous notification "foo" with payload "payload" received from server process with PID 14728.
+
+/* Identical messages from same (sub-) transaction can be eliminated - unless you use the ALL keyword */
+LISTEN bar;
+BEGIN;
+NOTIFY bar, 'Coffee please';
+NOTIFY bar, 'Coffee please';
+NOTIFY bar, 'Milk please';
+NOTIFY ALL bar, 'Milk please';
+SAVEPOINT s;
+NOTIFY bar, 'Coffee please';
+COMMIT;
+Asynchronous notification "bar" with payload "Coffee please" received from server process with PID 31517.
+Asynchronous notification "bar" with payload "Milk please" received from server process with PID 31517.
+Asynchronous notification "bar" with payload "Milk please" received from server process with PID 31517.
+Asynchronous notification "bar" with payload "Coffee please" received from server process with PID 31517.
 
  
 
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index c39ac3a..38a8246 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -510,6 +510,7 @@ pg_notify(PG_FUNCTION_ARGS)
 {
 	const char *channel;
 	const char *payload;
+	bool use_all;
 
 	if (PG_ARGISNULL(0))
 		channel = "";
@@ -521,10 +522,15 @@ pg_notify(PG_FUNCTION_ARGS)
 	else
 		payload = text_to_cstring(PG_GETARG_TEXT_PP(1));
 
+	if (PG_NARGS() > 2 && ! PG_ARGISNULL(2))
+		use_all = PG_GETARG_BOOL(2);
+	else
+		use_all = false;
+
 	/* For NOTIFY as a statement, this is checked in ProcessUtility */
 	PreventCommandDuringRecovery("NOTIFY");
 
-	Async_Notify(channel, payload);
+	Async_Notify(channel, payload, use_all);
 
 	PG_RETURN_VOID();
 }
@@ -540,7 +546,7 @@ pg_notify(PG_FUNCTION_ARGS)
  *		^^
  */
 void
-Async_Notify(const char *channel, const char *payload)
+Async_Notify(const char *channel, const char *payload, bool use_all)
 {
 	Notification *n;
 	MemoryContext oldcontext;
@@ -570,9 +576,10 @@ Async_Notify(const char *channel, const char *payload)
 	 errmsg("payload string too long")));
 	}
 
-	/* no point in making duplicate entries in the list ... */
-	if (AsyncExistsPendingNotify(channel, payload))
-		return;
+	if (!use_all)
+		/* remove duplicate entries in the list */
+		if (AsyncExistsPendingNotify(channel, payload))
+			return;
 
 	/*
 	 * The notification list needs to live until end of transaction, so store
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b307b48..7203f4a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8528,11 +8528,12 @@ DropRuleStmt:
  *
  */
 
-NotifyStmt: NOTIFY ColId notify_payload
+NotifyStmt: NOTIFY all_or_distinct ColId notify_payload
 {
 		

Re: [HACKERS] proposal: make NOTIFY list de-duplication optional

2016-02-07 Thread Filip Rembiałkowski
On Sun, Feb 7, 2016 at 11:54 AM, Vik Fearing <v...@2ndquadrant.fr> wrote:
> On 02/07/2016 03:42 AM, Filip Rembiałkowski wrote:

> You left the duplicate behavior with subtransactions, but didn't mention
> it in the documentation.  If I do  NOTIFY DISTINCT chan, 'msg';  then I
> expect only distinct notifications but I'll get duplicates if I'm in a
> subtransaction.  Either the documentation or the code needs to be fixed.

agreed

>
> I seem to remember some discussion about not using DEFAULT parameters in
> system functions so you should leave the old function alone and create a
> new function with your use_all parameter.  I don't recall the exact
> reason why so hopefully someone else will enlighten me.

I'm quite new to this; how do I pinpoint proper OID for a new catalog
object (function, in this case)?
Is there a better way than browsing fmgr files and guessing next available oid?

>
> There is also no mention in the documentation about what happens if I do:
>
> NOTIFY ALL chan, 'msg';
> NOTIFY ALL chan, 'msg';
> NOTIFY DISTINCT chan, 'msg';
> NOTIFY ALL chan, 'msg';
>
> Without testing, I'd say I'd get two messages, but it should be
> explicitly mentioned somewhere.

If it's four separate transactions, LISTEN'er should get four.
If it's in one transaction,  LISTEN'er should get three.










> --
> Vik Fearing  +33 6 46 75 15 36
> 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] proposal: make NOTIFY list de-duplication optional

2016-02-07 Thread Filip Rembiałkowski
On Sun, Feb 7, 2016 at 4:37 PM, Vik Fearing  wrote:

>>> There is also no mention in the documentation about what happens if I do:
>>>
>>> NOTIFY ALL chan, 'msg';
>>> NOTIFY ALL chan, 'msg';
>>> NOTIFY DISTINCT chan, 'msg';
>>> NOTIFY ALL chan, 'msg';
>>>
>>> Without testing, I'd say I'd get two messages, but it should be
>>> explicitly mentioned somewhere.
>>
>> If it's four separate transactions, LISTEN'er should get four.
>
> The question was for one transaction, I should have been clearer about that.
>
>> If it's in one transaction,  LISTEN'er should get three.
>
> This is surprising to me, I would think it would get only two.  What is
> your rationale for three?
>

It is a single transaction, but four separate commands.

>>> NOTIFY ALL chan, 'msg';
-- send the message, save in the list/hash
>>> NOTIFY ALL chan, 'msg';
-- ALL was specified, send the message even if it is on the list/hash
>>> NOTIFY DISTINCT chan, 'msg';
-- default mode, skip message because it's in the list/hash
>>> NOTIFY ALL chan, 'msg';
-- ALL was specified, send the message even if it is hashed/saved


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


Re: [HACKERS] proposal: make NOTIFY list de-duplication optional

2016-02-06 Thread Filip Rembiałkowski
+1

... and a patch (only adding ALL keyword, no hash table implemented yet).



On Sat, Feb 6, 2016 at 2:35 PM, Brendan Jurd  wrote:
> On Sat, 6 Feb 2016 at 12:50 Tom Lane  wrote:
>>
>> Robert Haas  writes:
>> > I agree with what Merlin said about this:
>> >
>> > http://www.postgresql.org/message-id/CAHyXU0yoHe8Qc=yc10ahu1nfia1tbhsg+35ds-oeueuapo7...@mail.gmail.com
>>
>> Yeah, I agree that a GUC for this is quite unappetizing.
>
>
> How would you feel about a variant for calling NOTIFY?
>
> The SQL syntax could be something like "NOTIFY [ALL] channel, payload" where
> the ALL means "just send the notification already, nobody cares whether
> there's an identical one in the queue".
>
> Likewise we could introduce a three-argument form of pg_notify(text, text,
> bool) where the final argument is whether you are interested in removing
> duplicates.
>
> Optimising the remove-duplicates path is still probably a worthwhile
> endeavour, but if the user really doesn't care at all about duplication, it
> seems silly to force them to pay any performance price for a behaviour they
> didn't want, no?
>
> Cheers,
> BJ
diff --git a/doc/src/sgml/ref/notify.sgml b/doc/src/sgml/ref/notify.sgml
index 4dd5608..c148859 100644
--- a/doc/src/sgml/ref/notify.sgml
+++ b/doc/src/sgml/ref/notify.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  
 
-NOTIFY channel [ , payload ]
+NOTIFY [ ALL | DISTINCT ] channel [ , payload ]
 
  
 
@@ -105,6 +105,8 @@ NOTIFY channel [ , ALL is specified (contrary to DISTINCT, the
+   default), the server will deliver all notifications, including duplicates.
   
 
   
@@ -184,11 +186,14 @@ NOTIFY channel [ , 
 To send a notification you can also use the function
-pg_notify(text,
-text). The function takes the channel name as the
-first argument and the payload as the second. The function is much easier
-to use than the NOTIFY command if you need to work with
-non-constant channel names and payloads.
+pg_notify(channel text,
+payload text ,
+use_all boolean).
+The function takes the channel name as the first argument and the payload
+as the second. The third argument, false by default, represents
+the ALL keyword. The function is much easier to use than the
+NOTIFY command if you need to work with non-constant
+channel names and payloads.

   
  
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 923fe58..9df5301 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -965,3 +965,10 @@ RETURNS jsonb
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'jsonb_set';
+
+CREATE OR REPLACE FUNCTION
+  pg_notify(channel text, payload text, use_all boolean DEFAULT false)
+RETURNS void
+LANGUAGE INTERNAL
+VOLATILE
+AS 'pg_notify';
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index c39ac3a..d374a00 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -510,6 +510,7 @@ pg_notify(PG_FUNCTION_ARGS)
 {
 	const char *channel;
 	const char *payload;
+	bool use_all;
 
 	if (PG_ARGISNULL(0))
 		channel = "";
@@ -521,10 +522,12 @@ pg_notify(PG_FUNCTION_ARGS)
 	else
 		payload = text_to_cstring(PG_GETARG_TEXT_PP(1));
 
+	use_all = PG_GETARG_BOOL(2);
+
 	/* For NOTIFY as a statement, this is checked in ProcessUtility */
 	PreventCommandDuringRecovery("NOTIFY");
 
-	Async_Notify(channel, payload);
+	Async_Notify(channel, payload, use_all);
 
 	PG_RETURN_VOID();
 }
@@ -540,7 +543,7 @@ pg_notify(PG_FUNCTION_ARGS)
  *		^^
  */
 void
-Async_Notify(const char *channel, const char *payload)
+Async_Notify(const char *channel, const char *payload, bool use_all)
 {
 	Notification *n;
 	MemoryContext oldcontext;
@@ -570,9 +573,10 @@ Async_Notify(const char *channel, const char *payload)
 	 errmsg("payload string too long")));
 	}
 
-	/* no point in making duplicate entries in the list ... */
-	if (AsyncExistsPendingNotify(channel, payload))
-		return;
+	if (!use_all)
+		/* remove duplicate entries in the list */
+		if (AsyncExistsPendingNotify(channel, payload))
+			return;
 
 	/*
 	 * The notification list needs to live until end of transaction, so store
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b307b48..7203f4a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8528,11 +8528,12 @@ DropRuleStmt:
  *
  */
 
-NotifyStmt: NOTIFY ColId notify_payload
+NotifyStmt: NOTIFY all_or_distinct ColId notify_payload
 {
 	NotifyStmt *n = makeNode(NotifyStmt);
-	n->conditionname = $2;
-	n->payload = $3;
+	n->use_all = $2;
+	n->conditionname = $3;
+	n->payload = $4;
 	$$ = (Node *)n;
 }
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 045f7f0..0e50561 

[HACKERS] proposal: make NOTIFY list de-duplication optional

2016-02-05 Thread Filip Rembiałkowski
- new GUC in "Statement Behaviour" section, notify_duplicate_removal
(default true)

Initial discussion in this thread:
http://www.postgresql.org/message-id/CAP_rwwmpzk9=sbjrztod05bdctyc43wnknu_m37dygvl4sa...@mail.gmail.com

Rationale: for some legitimate use cases, duplicate removal is not
required, and it gets O(N^2) cost on large COPY/ insert transactions.
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 392eb70..9fb5504 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6095,6 +6095,24 @@ SET XML OPTION { DOCUMENT | CONTENT };
   
  
 
+ 
+  notify_duplicate_removal (bool)
+  
+   notify_duplicate_removal configuration parameter
+  
+  
+  
+   
+Try to remove duplicate messages while processing NOTIFY.  When
+on (the default), the server will avoid duplicate messages
+(with same channel and payload).  Setting this variable to
+off can increase performance in some situations - at the
+cost of having duplicate messages in NOTIFY queue.  See  for more information.
+   
+  
+ 
+
  
 
  
diff --git a/doc/src/sgml/ref/notify.sgml b/doc/src/sgml/ref/notify.sgml
index 4dd5608..86a9bed 100644
--- a/doc/src/sgml/ref/notify.sgml
+++ b/doc/src/sgml/ref/notify.sgml
@@ -95,16 +95,17 @@ NOTIFY channel [ , 
If the same channel name is signaled multiple times from the same
-   transaction with identical payload strings, the
-   database server can decide to deliver a single notification only.
-   On the other hand, notifications with distinct payload strings will
-   always be delivered as distinct notifications. Similarly, notifications from
-   different transactions will never get folded into one notification.
-   Except for dropping later instances of duplicate notifications,
+   transaction with identical payload strings, and
+   notify_duplicate_removal is set to true, the database server
+   can decide to deliver a single notification only.  On the other hand,
+   notifications with distinct payload strings will always be delivered as
+   distinct notifications. Similarly, notifications from different
+   transactions will never get folded into one notification.  Except for
+   dropping later instances of duplicate notifications,
NOTIFY guarantees that notifications from the same
transaction get delivered in the order they were sent.  It is also
-   guaranteed that messages from different transactions are delivered in
-   the order in which the transactions committed.
+   guaranteed that messages from different transactions are delivered in the
+   order in which the transactions committed.
   
 
   
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index c39ac3a..a7bc9f1 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -364,8 +364,9 @@ static bool amRegisteredListener = false;
 /* has this backend sent notifications in the current transaction? */
 static bool backendHasSentNotifications = false;
 
-/* GUC parameter */
+/* GUC parameters */
 bool		Trace_notify = false;
+bool		notify_duplicate_removal = true;
 
 /* local function prototypes */
 static bool asyncQueuePagePrecedes(int p, int q);
@@ -570,9 +571,12 @@ Async_Notify(const char *channel, const char *payload)
 	 errmsg("payload string too long")));
 	}
 
-	/* no point in making duplicate entries in the list ... */
-	if (AsyncExistsPendingNotify(channel, payload))
-		return;
+	if (notify_duplicate_removal)
+	{
+		/* check for duplicate entries in the list */
+		if (AsyncExistsPendingNotify(channel, payload))
+			return;
+	}
 
 	/*
 	 * The notification list needs to live until end of transaction, so store
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 83b8388..b737c29 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1617,6 +1617,15 @@ static struct config_bool ConfigureNamesBool[] =
 		false,
 		NULL, NULL, NULL
 	},
+	{
+		{"notify_duplicate_removal", PGC_USERSET, CLIENT_CONN_STATEMENT,
+			gettext_noop("Remove duplicate messages during NOTIFY."),
+			NULL
+		},
+		_duplicate_removal,
+		true,
+		NULL, NULL, NULL
+	},
 
 	/* End-of-list marker */
 	{
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 029114f..2831c1b 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -536,6 +536,7 @@
 #xmloption = 'content'
 #gin_fuzzy_search_limit = 0
 #gin_pending_list_limit = 4MB
+#notify_duplicate_removal = on
 
 # - Locale and Formatting -
 
diff --git a/src/include/commands/async.h b/src/include/commands/async.h
index b4c13fa..c572691 100644
--- a/src/include/commands/async.h
+++ b/src/include/commands/async.h
@@ -23,6 +23,7 @@
 #define NUM_ASYNC_BUFFERS	8
 
 extern bool Trace_notify;
+extern bool notify_duplicate_removal;
 extern volatile sig_atomic_t 

Re: [HACKERS] tiny doc patch

2016-01-28 Thread Filip Rembiałkowski
right.


On Thu, Jan 14, 2016 at 5:46 PM, Robert Haas <robertmh...@gmail.com> wrote:

> On Thu, Jan 14, 2016 at 8:40 AM, Filip Rembiałkowski
> <filip.rembialkow...@gmail.com> wrote:
> > (include RLS option in CREATE USER doc)
> >
> > should go into HEAD and REL9_5_STABLE
>
> Doesn't ALTER USER need the same fix?
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
diff --git a/doc/src/sgml/ref/alter_user.sgml b/doc/src/sgml/ref/alter_user.sgml
index 84a0c52..5962a8e 100644
--- a/doc/src/sgml/ref/alter_user.sgml
+++ b/doc/src/sgml/ref/alter_user.sgml
@@ -31,6 +31,7 @@ ALTER USER role_specification [ WIT
 | INHERIT | NOINHERIT
 | LOGIN | NOLOGIN
 | REPLICATION | NOREPLICATION
+| BYPASSRLS | NOBYPASSRLS
 | CONNECTION LIMIT connlimit
 | [ ENCRYPTED | UNENCRYPTED ] PASSWORD 'password'
 | VALID UNTIL 'timestamp'
diff --git a/doc/src/sgml/ref/create_user.sgml b/doc/src/sgml/ref/create_user.sgml
index 6c690b3..574604f 100644
--- a/doc/src/sgml/ref/create_user.sgml
+++ b/doc/src/sgml/ref/create_user.sgml
@@ -31,6 +31,7 @@ CREATE USER name [ [ WITH ] connlimit
 | [ ENCRYPTED | UNENCRYPTED ] PASSWORD 'password'
 | VALID UNTIL 'timestamp'

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


[HACKERS] tiny doc patch

2016-01-14 Thread Filip Rembiałkowski
(include RLS option in CREATE USER doc)

should go into HEAD and REL9_5_STABLE
diff --git a/doc/src/sgml/ref/create_user.sgml b/doc/src/sgml/ref/create_user.sgml
index 6c690b3..574604f 100644
--- a/doc/src/sgml/ref/create_user.sgml
+++ b/doc/src/sgml/ref/create_user.sgml
@@ -31,6 +31,7 @@ CREATE USER name [ [ WITH ] connlimit
 | [ ENCRYPTED | UNENCRYPTED ] PASSWORD 'password'
 | VALID UNTIL 'timestamp'

-- 
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_dump LOCK TABLE ONLY question

2015-10-30 Thread Filip Rembiałkowski
Please take it as a very naive and basic approach :-)

What could go wrong here?





diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 36863df..57a50b5 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5169,9 +5169,9 @@ getTables(Archive *fout, DumpOptions *dopt, int
*numTables)
 * Read-lock target tables to make sure they aren't DROPPED or altered
 * in schema before we get around to dumping them.
 *
-* Note that we don't explicitly lock parents of the target tables; we
-* assume our lock on the child is enough to prevent schema
-* alterations to parent tables.
+* Note that we don't explicitly lock neither parents nor children of
+* the target tables; we assume our lock on the child is enough to
+* prevent schema alterations to parent tables.
 *
 * NOTE: it'd be kinda nice to lock other relations too, not only
 * plain tables, but the backend doesn't presently allow that.
@@ -5179,11 +5179,18 @@ getTables(Archive *fout, DumpOptions *dopt,
int *numTables)
if (tblinfo[i].dobj.dump && tblinfo[i].relkind == RELKIND_RELATION)
{
resetPQExpBuffer(query);
-   appendPQExpBuffer(query,
- "LOCK TABLE %s IN ACCESS SHARE MODE",
- fmtQualifiedId(fout->remoteVersion,
-   tblinfo[i].dobj.namespace->dobj.name,
-tblinfo[i].dobj.name));
+   if (fout->remoteVersion >= 80400)
+   appendPQExpBuffer(query,
+   "LOCK TABLE ONLY %s IN ACCESS SHARE MODE",
+   fmtQualifiedId(fout->remoteVersion,
+
tblinfo[i].dobj.namespace->dobj.name,
+   tblinfo[i].dobj.name));
+   else
+   appendPQExpBuffer(query,
+   "LOCK TABLE %s IN ACCESS SHARE MODE",
+   fmtQualifiedId(fout->remoteVersion,
+
tblinfo[i].dobj.namespace->dobj.name,
+   tblinfo[i].dobj.name));
ExecuteSqlStatement(fout, query->data);
}





On Fri, Oct 16, 2015 at 5:06 PM, Robert Haas  wrote:
> On Thu, Oct 15, 2015 at 9:13 PM, Jim Nasby  wrote:
>> OTOH, now that the catalog is MVCC capable, do we even still need to lock
>> the objects for a schema-only dump?
>
> Yes.  The MVCC snapshots used for catalog reads are stable only for
> the duration of one particular catalog read.  We're not using the
> transaction snapshot.
>
> --
> 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] proposal: DROP DATABASE variant that kills active sessions

2015-10-16 Thread Filip Rembiałkowski
DROP DATABASE mydb CONCURRENTLY;

That would perform forced shutdown

1) reject any new backends to mydb
2) terminate old backends
3) drop db

40 upvotes here http://dba.stackexchange.com/a/11895/3710 inspired me
to propose this improvement.

If you think it's a good idea please include it as a low-priority TODO item.

thanks


-- 
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_dump LOCK TABLE ONLY question

2015-10-07 Thread Filip Rembiałkowski
Oct 2 2015 01:19 "Michael Paquier" <michael.paqu...@gmail.com> wrote:
>
> On Thu, Oct 1, 2015 at 10:43 PM, Filip Rembiałkowski  <
filip.rembialkow...@gmail.com> wrote:
> > I just want to understand why there is LOCK TABLE not LOCK TABLE ONLY.
>
> It seems to me that you'd still want to use LOCK TABLE particularly if
> the dump is only done on a subset of tables, using --table for
> example.

Right. But please consider this use case, when I have to dunp only given
schema, nothing more and nothing less.

Is --schema option not just for that?

Locking child tables seems a bit counter-intuitive.

COPY does not touch child tables, also.


Re: [HACKERS] pg_dump LOCK TABLE ONLY question

2015-10-01 Thread Filip Rembiałkowski
(sorry I lost the original thread somehow)

tgl wrote:

> Filip wrote:

> > I'm running pg_dump constrained to one schema. It appears that pg_dump runs
> > "LOCK TABLE %s IN ACCESS SHARE MODE" for each table.
> > Naturally it makes sense, but...

> > This schema has a table that serves as parent for thousands of child
> > tables (via INHERITS).

> > So effectively, to dump this schema, I have to LOCK all these tables
> > not only parent.

> They'd all end up locked anyway wouldn't they?

I would like to dump the whole schema in ONLY mode, including table
data for only that schema, excluding data for child tables in other
schemas.

Why would they be locked then?

Which part of pg_dump requires locking child tables?

Per the docs, "COPY only deals with the specific table named; it does
not copy data to or from child tables. "


I just want to understand why there is LOCK TABLE not LOCK TABLE ONLY.


-- 
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_dump LOCK TABLE ONLY question

2015-09-27 Thread Filip Rembiałkowski
Hi.

I'm running pg_dump constrained to one schema. It appears that pg_dump runs

"LOCK TABLE %s IN ACCESS SHARE MODE" for each table.

Naturally it makes sense, but...

This schema has a table that serves as parent for thousands of child
tables (via INHERITS).

So effectively, to dump this schema, I have to LOCK all these tables
not only parent.

pg_dump does it automatically, I checked in current trunk - it does
not add ONLY keyword. Should it?

I wonder if it it counts as a bug.
If not a bug, maybe it counts as a feature request?

Thanks.


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


[HACKERS] improving \dt++ in psql

2014-04-30 Thread Filip Rembiałkowski
Hi


I tried to start writing a patch to add Total Size column to \dt++ output.

in src/bin/psql/describe.c we have this

listTables(
const char *tabtypes,
const char *pattern,
 bool verbose,
 bool showSystem)


I was (as a long time Pg user) dead sure that psql really sometimes
cares about the number of plus signs that you add to meta-command

So why this particular function interface has boolean verbose parameter?

Can't we have higher levels of verbosiness?


thanks,
Filip


-- 
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] Fwd: question on foreign key lock

2012-12-05 Thread Filip Rembiałkowski
Robert, thank you for  the answer.

1. need exclusive lock anyway to add triggers.
Why adding a trigger needs exclusive lock?
Someone would say blocking reads is not needed (since possible trigger
events are: Insert/Update/Delete/Truncate).

2. will create a risk of deadlock.
From user perspective a risk of deadlock is sometimes better than
excessive  locking. Transactional DDL users should be prepared for
exceptions/retries anyway.

3. I made a naive test of simply changing AccessExclusiveLock to
ExclusiveLock, and seeing how many regression tests it breaks. It
breaks none :-)
Current Git head gives me 2 fails/133 tests regardless of this change.


regards,
Filip










On Mon, Nov 12, 2012 at 5:20 PM, Robert Haas robertmh...@gmail.com wrote:
 On Thu, Nov 8, 2012 at 3:45 AM, Filip Rembiałkowski
 filip.rembialkow...@gmail.com wrote:
 maybe this is a better group for this question?

 I can't see why creating foreign key on table A referencing table B,
 generates an AccessExclusiveLock on B.
 It seems (to a layman :-) ) that only writes to B should be blocked.

 I'm really interested if this is either expected effect or any open TODO
 item or suboptimal behavior of postgres.

 This comment explains it:

 /*
  * Grab an exclusive lock on the pk table, so that someone doesn't delete
  * rows out from under us. (Although a lesser lock would do for that
  * purpose, we'll need exclusive lock anyway to add triggers to the pk
  * table; trying to start with a lesser lock will just create a risk of
  * deadlock.)
  */
 pkrel = heap_openrv(fkconstraint-pktable, AccessExclusiveLock);

 Concurrent DDL is something that's been discussed in detail on this
 list in the past; unfortunately, there are some tricky race conditions
 are the shared invalidation queue and SnapshotNow that make it hard to
 implement properly.  I'm hoping to have some time to work on this at
 some point, but it hasn't happened yet.

 --
 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] Fwd: question on foreign key lock

2012-11-08 Thread Filip Rembiałkowski
Hi,

maybe this is a better group for this question?

I can't see why creating foreign key on table A referencing table B,
generates an AccessExclusiveLock on B.
It seems (to a layman :-) ) that only writes to B should be blocked.

I'm really interested if this is either expected effect or any open TODO
item or suboptimal behavior of postgres.

Thanks


-- Forwarded message --
From: Filip Rembiałkowski filip.rembialkow...@gmail.com
Date: Thu, Nov 1, 2012 at 5:33 PM
Subject: question on foreign key lock
To: pgsql-general list pgsql-gene...@postgresql.org


Hello.

Why adding FK creates AccessExclusiveLock on referenced tabble?

{{{
CREATE TABLE A ( id integer, idb integer );
INSERT INTO A (id,idb) SELECT x, x%4 FROM generate_series(1,100) x;

CREATE TABLE B ( id int primary key );
INSERT INTO B VALUES (0),(1),(2),(3);

BEGIN;
ALTER TABLE A ADD CONSTRAINT a_idb_fkey FOREIGN KEY (idb) REFERENCES b;
SELECT * FROM pg_locks l, pg_class c WHERE l.pid = pg_backend_pid() AND
l.locktype='relation' AND l.mode ilike '%exclusive%' AND l.relation=c.oid;
ROLLBACK;
}}}


Last SELECT is showing AccessExclusive on B.
Why not Exclusive?


Thanks,
Filip


Re: [HACKERS] Problems with autovacuum and vacuum

2011-01-01 Thread Filip Rembiałkowski
2010/12/30 JotaComm jota.c...@gmail.com

 Hello,

 Last week I had a serious problem with my PostgreSQL database. My
 autovacuum is OFF, but in September it started to prevent the transaction
 wraparoud; however last week the following message appeared continuously in
 my log:

 WARNING: database production must be vacuumed within 4827083 transactions

 HINT: To avoid a database shutdown, execute a full-database VACUUM in
 production.

 This message appeared for five to six hours; after that, the message
 disappeared from log. Any idea about what could have happened?



probably another wraparaund-forced autovacuum worker did the job, so the
warnings disappeared



 Every day the vacuum is executed on some tables; and on Sundays it's
 executed on all tables. But as the autovacuum is running since September,
 and it runs for a long time, the vacuum was blocked because autovacuum had
 been running on the same table. How should I procede in this case?



hmm. single vacuum process runs for more than 3 months on a table with
10 rows?
this is ... less than 128 rows/second, not good.

I would rather terminate this old process, and start a VACUUM VERBOSE when
the database is less loaded.


How many INS/UPD/DEL you have on this table?



PS. When you fix this, enable autovacuum, to avoid more problems...


Re: [HACKERS] [GENERAL] pg.dropped

2010-01-08 Thread Filip Rembiałkowski
(continued from -general)


W dniu 7 stycznia 2010 22:31 użytkownik Greg Smith
g...@2ndquadrant.comnapisał:

 Filip Rembiałkowski wrote:

 After dropping a column from table, there is still entry in pg_attribute

 fi...@la_dev=# select * from pg_attribute where attrelid = (select oid
 from pg_class where relname='thetable') order by attnum desc limit 1;
 -[ RECORD 1 ]-+--
 attrelid  | 4753849
 attname   | pg.dropped.69
 ...
 attisdropped  | t


 See that last part?  That's what happens when you drop a
 table--attisdropped is set to true.  The server can't just delete the
 pg_attribute entry altogether for various internal reasons, this is what it
 does instead.


When should server delete this row? In my case it looks like it's never
deleted (it lasts even server restart).




  And of course this makes my INSERT not working...


 There's obviously something wrong here, but the fact that the pg_attribute
 entry is still there (but marked dropped) is a not a direct cause of your
 problem.


Thanks, I get it.



-- 
Filip Rembiałkowski
JID,mailto:filip.rembialkow...@gmail.com
http://filip.rembialkowski.net/


Re: [HACKERS] Distinguish view and table problem

2009-12-19 Thread Filip Rembiałkowski
W dniu 19 grudnia 2009 03:20 użytkownik suzhiyang suzhiy...@gmail.comnapisał:

  Sorry, I've not describe my problem precisely.
 I mean that  I want to get relkind or something from a systable by the
 programm but not by sql.

I don't understand how you can get data from table without using SQL. (maybe
I'm just too sql)


 That is, if I execute such sql by exec_simple_query(select xxx from
 pg_xxx), how could I get the result of it in the programm?


Are you programming in C? If so, use the API provided by PostgreSQL,
http://www.postgresql.org/docs/current/static/libpq.html

PS. suzhiyang, please use Reply All when talking on this list.


Re: [HACKERS] Distinguish view and table problem

2009-12-19 Thread Filip Rembiałkowski
W dniu 19 grudnia 2009 16:19 użytkownik Jaime Casanova 
jcasa...@systemguards.com.ec napisał:


 i think he is hacking postgres's source code to make the TODO: allow
 recompilation of views (he send an email about that in another
 thread)...


oh. I didn't realise, that such seemingly simple question can relate to such
hard task.

even Oracle and other big players do not have an ideal solution for this...

good luck suzhiyang!


-- 
Filip Rembiałkowski
JID,mailto:filip.rembialkow...@gmail.com
http://filip.rembialkowski.net/


Re: [HACKERS] Distinguish view and table problem

2009-12-18 Thread Filip Rembiałkowski
2009/12/18 suzhiyang suzhiy...@gmail.com

 How could I get the relkind of a table(view) by its name from pg_class?


pg_class is (quite logically) UNIQUE on (relname, relnamespace)

SELECT c.relkind from pg_class c, pg_namespace n
where c.relnamespace = n.oid
and c.relname = 'thetable'
and n.nspname = 'theschema'



-- 
Filip Rembiałkowski
JID,mailto:filip.rembialkow...@gmail.com
http://filip.rembialkowski.net/


Re: [HACKERS] strange segfault with 8.3.8

2009-12-10 Thread Filip Rembiałkowski
W dniu 7 grudnia 2009 17:43 użytkownik Greg Smith g...@2ndquadrant.comnapisał:

  Filip Rembiałkowski wrote:

 Dec  7 07:24:45 nsXX kernel: postgres[22481]: segfault at 7fda5e1d5000
 ip 7fda604553c3 sp 7fffe41faf28 error 4 in libc-2.9.so
 [7fda603d1000+168000]
 Dec  7 07:24:45 nsXX kernel: grsec: From XX.YY.ZZ.51: Segmentation
 fault occurred at 7fda5e1d5000 in
 /usr/lib/postgresql/8.3/bin/postgres[postgres:22481] uid/euid:103/103
 gid/egid:114/114, parent
 /usr/lib/postgresql/8.3/bin/postgres[postgres:29857] uid/euid:103/103
 gid/egid:114/114

 This is on 8.3.8, latest Ubuntu distribution, server under quite heavy
 load.

 afterwards we had some corrupted pages (*ERROR: compressed data is
 corrupt*).

 Random segfaults only under heavy load with corrupted pages afterwards
 normally means bad RAM.  I'd suggest a round of memtest86+ on the system if
 it's PC hardware.


Just a note for all other who may encouter this problem:

This was not a vanilla Ubuntu - this was vendor-modified version, witch
custom grsecurity patched kernel and probably with custom patched libc.

# uname -a
Linux nsXX.ovh.net
 2.6.31.5-grsec--grs-ipv4-64 #2 SMP Thu Nov 5 12:36:20 UTC 2009
x86_64 GNU/Linux



-- 
Filip Rembiałkowski
JID,mailto:filip.rembialkow...@gmail.com
http://filip.rembialkowski.net/


[HACKERS] strange segfault with 8.3.8

2009-12-07 Thread Filip Rembiałkowski
hello,

from kernel log:

Dec  7 07:24:45 nsXX kernel: postgres[22481]: segfault at 7fda5e1d5000
ip 7fda604553c3 sp 7fffe41faf28 error 4 in libc-2.9.so
[7fda603d1000+168000]
Dec  7 07:24:45 nsXX kernel: grsec: From XX.YY.ZZ.51: Segmentation fault
occurred at 7fda5e1d5000 in
/usr/lib/postgresql/8.3/bin/postgres[postgres:22481] uid/euid:103/103
gid/egid:114/114, parent
/usr/lib/postgresql/8.3/bin/postgres[postgres:29857] uid/euid:103/103
gid/egid:114/114

This is on 8.3.8, latest Ubuntu distribution, server under quite heavy load.

afterwards we had some corrupted pages (*ERROR: compressed data is corrupt*
).

any clues / directions welcome.



-- 
Filip Rembiałkowski
JID,mailto:filip.rembialkow...@gmail.com
http://filip.rembialkowski.net/


Re: [HACKERS] facing problem with createdb.

2009-06-29 Thread Filip Rembiałkowski
2009/6/29 Ms swati chande swat...@yahoo.com


 Hi,

 I built postgresql from source using Windows 2005.


Did you read
http://wiki.postgresql.org/wiki/Running_%26_Installing_PostgreSQL_On_Native_Windows?

Why not use prebuilt versions (see
http://www.postgresql.org/download/windows)?




 After installation, I am trying to create a database using

 createdb.


Please read these:

http://www.postgresql.org/docs/current/static/manage-ag-createdb.html
http://www.postgresql.org/docs/8.3/static/app-createdb.html



 It then asks for a password. What password is it? There are now passwords
 set on my system. Also tried editing pg_hba.conf. But could not solve the
 problem.

 How do I move ahead.



it's not clear what is wrong there, but try this:

1. Add this line to the beginning of your pg_hba.conf:
host all all 127.0.0.1/32 trust

2. restart postgresql server.

3. you should be able to connect without password.


Please post such basic questions to pgsql-general mailing list.

You should REALLY read some manual pages, especially these:
http://www.postgresql.org/docs/8.3/static/reference-client.html


good luck!



-- 
Filip Rembiałkowski
JID,mailto:filip.rembialkow...@gmail.com
http://filip.rembialkowski.net/


Re: [HACKERS] pg_dump restore time and Foreign Keys

2008-06-09 Thread Filip Rembiałkowski
2008/6/9 Simon Riggs [EMAIL PROTECTED]:


 On Mon, 2008-06-09 at 10:57 -0400, Tom Lane wrote:
  Decibel! [EMAIL PROTECTED] writes:
   Actually, in the interest of stating the problem and not the
   solution, what we need is a way to add FKs that doesn't lock
   everything up to perform the key checks.
 
  Ah, finally a useful comment.  I think it might be possible to do an
  add FK concurrently type of command that would take exclusive lock
  for just long enough to add the triggers, then scan the tables with just
  AccessShareLock to see if the existing rows meet the constraint, and
  if so finally mark the constraint valid.  Meanwhile the constraint
  would be enforced against newly-added rows by the triggers, so nothing
  gets missed.  You'd still get a small hiccup in system performance
  from the transient exclusive lock, but nothing like as bad as it is
  now.  Would that solve your problem?

 That's good, but it doesn't solve the original user complaint about
 needing to re-run many, many large queries to which we already know the
 answer.


just a guess, but maybe create FK concurrently feature combined with
synchronized scan feature _does_ resolve original problem.

if you run many create FK concurrently one after another, wouldn't the seq
scan be reused?












 --
  Simon Riggs   www.2ndQuadrant.com
  PostgreSQL Training, Services and Support


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




-- 
Filip Rembiałkowski


Re: [HACKERS] URGENT HELP about 'duration' stats

2007-10-30 Thread Filip Rembiałkowski
2007/10/30, Camilo Porto [EMAIL PROTECTED]:

   I am simulating only 1 client with the Benchmark. Can 1 Client submit
   parallel queries, in single-processor enviroment?
  If this client uses two connections, you can run two queries in paralell.
 The client uses only 1 connection. In this situation is possible that
 the EXECUTOR's duration time become greater than the time period which the
 Query was observed? (as stated in my first topic)?

I guess it's possible under some circumstances...
2007-10-30 16:07:00 GMT [123] LOG:  duration: 99000.000 ms  select longfunc()
2007-10-30 16:07:01 GMT [123] LOG:  duration: 1000.000 ms  select shortfunc()
interval is 1 second, sum of durations 100 seconds :)


AFAIK, timestamps in the front of each line are assigned by log
writer, ie. *in the moment of writing* to the log. I'd better trust
duration: xxx ms  messages. they are calculated in backend directly.


In this log sample you showed us, the sum of durations is circa  625
ms. and the interval between first and last log entry is circa 822 ms.
If you have a test case which shows that much difference you speak of,
could you please present it here, along with your logging settings?



-- 
Filip Rembiałkowski

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [HACKERS] URGENT HELP about 'duration' stats

2007-10-27 Thread Filip Rembiałkowski
2007/10/27, Camilo Porto [EMAIL PROTECTED]:

 The parallelism happens even if my PC has only one processor?

PostgreSQL creates a separate process for each client connection -
whether you have one CPU or more.

 Each query is executed in a separeted Thread?
No threads, at least on Unix. Regular processes (sometimes called backends).

 I am simulating only 1 client with the Benchmark. Can 1 Client submit
 parallel queries, in single-processor enviroment?
If this client uses two connections, you can run two queries in paralell.


this chapter of documentation can be helpful:
http://www.postgresql.org/docs/8.2/static/overview.html

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly


Re: [HACKERS] [FEATURE REQUEST] Streaming Onlinebackup (Maybe OFFTOPIC)

2007-09-07 Thread Filip Rembiałkowski
backup is not replication.
but replicated database can be treated as good source of backup.

please take following remarks:

1) in English you don't capitalize nouns
2) read what other people write to you and try to understand that.
3) this is open source, try to be more cooperative not just cry for a
ready made solution


2007/9/7, apoc9009 [EMAIL PROTECTED]:
 Markus Schiltknecht schrieb:
  Hi,
 
  apoc9009 wrote:
  Thadt is Replication NOT Backup
 
  I've now read all of your messages in this thread, but I simply fail
  to understand why you are that much opposed to the term 'replication'.
  I think the only thing which comes any close to what you're looking
  for is replication (in particular eager multi-master replication).
 What is your Problem in understanding the Word Backup?

 Translation for you:
 A Backup is a File or Set of Files thadt contains the Data of your
 Business critical Informations.
 It should not be Archived on the same place, the same House or the same
 Room.

 A Replication Database has nothing to do with a Backup, it works only
 for Failover if the Primary
 Database has a Mailfunction.

 A good Backuptool is needed if you have Databases with sizes over 1
 Terrabyte. The common
 Backup methods wont Work with Online Productiondatabases and without the
 Problem of Datalosses,
 this is only a Way for small and Mediumsize Databases, not for Hugh
 Databases.

 Keep in Mind: Backup is NOT Replication!

 Write it down 100 times and maybe you understand



 ---(end of broadcast)---
 TIP 4: Have you searched our list archives?

http://archives.postgresql.org



-- 
Filip Rembiałkowski

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [HACKERS] left outer join vs subplan

2007-09-07 Thread Filip Rembiałkowski
2007/9/6, Simon Riggs [EMAIL PROTECTED]:

 The query formulation does seem a fairly common one.

  First query:
  explain analyze
  select *
  from
   a
   left outer join (
   select b.id, sum(b.val)
   from b
   group by b.id
   ) bagg
   on bagg.id = a.id
  where
   a.id  1
  order by a.addon, a.id
  limit 100;


 The value of sum(b.val) is never used in the query, so the aggregate
 itself could be discarded. I suspect there are other conditions you
 aren't showing us that would make this impossible?

The value of sum(b.val) is being output in the select *, so saying
it's never used is an oversimplification. But it's actually not used
in any join, or filter. That should be enough to optimize...


 The aggregate prevents the condition bagg.id = a.id from being pushed
 down so that we know b.id = a.id. If we knew that then we could use b.id
 = ? as an index condition to retrieve the rows.

That's exactly the point... But if we all can see it, maybe it's
possible to code it?


Cheers,
Filip Rembiałkowski

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly