From: Alvaro Herrera on 9 Feb 2010 20:38 Andrew Dunstan wrote: > > > Tom Lane wrote: > >Andrew Dunstan <andrew(a)dunslane.net> writes: > >>Jeff Davis wrote: > >>>So, I think ASCII is the natural choice here. > > > >>It's not worth hanging up this facility over this issue, ISTM. > >>If we want something more that ASCII then a base64 or hex > >>encoded string could possibly meet the need in the first > >>instance. > > > >Yeah, that would serve people who want to push either binary or > >non-ASCII data through the pipe. It would leave all risks of encoding > >problems on the user's head, though. > > True. It's a workaround, but I think it's acceptable at this stage. > We need to get some experience with this facility before we can > refine it. Hmm? If we decide now that it's not going to have encoding conversion, we won't able to change it later. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 9 Feb 2010 20:41 Alvaro Herrera <alvherre(a)commandprompt.com> writes: > Andrew Dunstan wrote: >> True. It's a workaround, but I think it's acceptable at this stage. >> We need to get some experience with this facility before we can >> refine it. > Hmm? If we decide now that it's not going to have encoding conversion, > we won't able to change it later. How so? If the feature currently allows only ASCII data, the behavior would be upward compatible with a future version that is able to accept and convert non-ASCII characters. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Simon Riggs on 14 Feb 2010 08:46 On Thu, 2010-02-11 at 00:52 +0100, Joachim Wieland wrote: > On Mon, Feb 8, 2010 at 5:16 PM, Alvaro Herrera > <alvherre(a)commandprompt.com> wrote: > > These are the on-disk notifications, right? It seems to me a bit > > wasteful to store channel name always as NAMEDATALEN bytes. Can we > > truncate it at its strlen? > > Attached is a new and hopefully more or less final patch for LISTEN / NOTIFY. > > The following items have been addressed in this patch: > > - only store strlen(channel) instead of NAMEDATALEN bytes on disk > - limit to 7-bit ASCII > - forbid 2PC and LISTEN/NOTIFY for now > - documentation changes > - add missing tab completion for NOTIFY > - fix pg_notify() behavior with respect to NULLs, too long and too > short parameters > - rebased to current HEAD, OID conflicts resolved Some minor review comments without having taken in much of previous discussion: * Patch doesn't apply cleanly anymore, close. * In async.c you say "new async notification model". Please say "async notification model in 9.0 is". In (2) you say there is a central queue, but don't describe purpose of queue or what it contains. Some other typos in header comments. * There is no mention of what to do with pg_notify at checkpoint. Look at how pg_subtrans handles this. Should pg_notify do the same? * Is there a lazy backend avoidance scheme as exists for relcache invalidation messages? see storage/ipc/sinval... OK, I see asyncQueueFillWarning() but nowhere else says it exists and there aren't any comments in it to say what it does or why * What do you expect the DBA to do when they receive a queue fill warning? Where is that written down? * Not clear of the purpose of backendSendsNotifications. In AtCommit_NotifyAfterCommit() the logic seems strange. Code is /* Allow transactions that have not executed LISTEN/UNLISTEN/NOTIFY to * return as soon as possible */ if (!pendingActions && !backendSendsNotifications) return; but since backendSendsNotifications is set true earlier there is no fast path. * AtSubCommit_Notify doesn't seem to have a fastpath when no notify commands have been executed. * In Send_Notify() you throw ERROR while still holding the lock. It seems better practice to drop the lock then ERROR. * Why is Send_Notify() a separate function? It's only called from one point in the code. * We know that sometimes a FATAL error can occur without properly processing the abort. Do we depend upon this never happening? * Can we make NUM_ASYNC_BUFFERS = 8? 4 just seems too small * backendSendsNotifications is future tense. The variable should be called something like hasSentNotifications. Couple of other variables with similar names Hope that helps -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Simon Riggs on 14 Feb 2010 12:41 On Sun, 2010-02-14 at 17:22 +0100, Joachim Wieland wrote: > > * There is no mention of what to do with pg_notify at checkpoint. > Look > > at how pg_subtrans handles this. Should pg_notify do the same? > > Actually we don't care... We even hope that the pg_notify pages are > not flushed at all. Notifications don't survive a server restart > anyway and upon restart we just delete whatever is in the directory. Suspected that was true, just checking it was commented somewhere. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 14 Feb 2010 13:47
Joachim Wieland <joe(a)mcknight.de> writes: > + #define ERRCODE_TOO_MANY_ENTRIES MAKE_SQLSTATE('5','4', '0','3','1') Do you have any evidence that there is actually a DB2 error code matching this, or is this errcode just invented? The one page Google finds doesn't list it: http://publib.boulder.ibm.com/iseries/v5r1/ic2924/index.htm?info/rzala/rzalastc.html regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |