[BUGS] BUG #8210: UTF8 column names corrupted by server
The following bug has been logged on the website: Bug reference: 8210 Logged by: Martin Schaefer Email address: martin.schae...@cadcorp.com PostgreSQL version: 9.2.1 Operating system: Windows 8 Description: The following code: const wchar_t *strName = L"id_äß"; wstring strCreate = wstring(L"create table test_umlaut(") + strName + L" integer primary key)"; PGconn *pConn = PQsetdbLogin("", "", NULL, NULL, "dev503", "postgres", "**"); if (!pConn) FAIL; if (PQsetClientEncoding(pConn, "UTF-8")) FAIL; PGresult *pResult = PQexec(pConn, "drop table test_umlaut"); if (pResult) PQclear(pResult); pResult = PQexec(pConn, ToUtf8(strCreate.c_str()).c_str()); if (pResult) PQclear(pResult); pResult = PQexec(pConn, "select * from test_umlaut"); if (!pResult) FAIL; if (PQresultStatus(pResult)!=PGRES_TUPLES_OK) FAIL; if (PQnfields(pResult)!=1) FAIL; const char *fName = PQfname(pResult,0); ShowW("Name: ", strName); ShowA("in UTF8: ", ToUtf8(strName).c_str()); ShowA("from DB: ", fName); ShowW("in UTF16: ", ToWide(fName).c_str()); PQclear(pResult); PQreset(pConn); (ShowA/W call OutputDebugStringA/W, and ToUtf8/ToWide use WideCharToMultiByte/MultiByteToWideChar with CP_UTF8.) generates this output: Name: id_äß in UTF8: id_äß from DB: id_ã¤ãÿ in UTF16: id_??? The back-end treats the name as if it were in ANSI encoding, not in UTF-8, when it lower-cases the name. The resulting column name is corrupted. I’m using PostgreSQL 9.2.1, compiled by Visual C++ build 1600, 64-bit The database uses: ENCODING = 'UTF8' LC_COLLATE = 'English_United Kingdom.1252' LC_CTYPE = 'English_United Kingdom.1252' -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #8198: ROW() literals not supported in an IN clause
On Wednesday, June 05, 2013 5:34 AM Rafał Rzepecki wrote: > On Tue, Jun 4, 2013 at 12:35 PM, Amit Kapila > wrote: > > On Saturday, June 01, 2013 9:37 PM > > > >> Row type literals constructed with ROW() cause an error when used in > >> an IN clause (string literals casted appropriately are allowed). > This > >> is especially problematic since many client libraries use these > >> literals to pass values of row-type arguments, hence making it > >> impossible to use them in IN-clause queries. > >> > > If I'm right, the proper fix would be (patch 0001; caution, not > tested): > > --- a/src/backend/parser/parse_expr.c > +++ b/src/backend/parser/parse_expr.c > @@ -1203,10 +1203,9 @@ transformAExprIn(ParseState *pstate, A_Expr *a) > Node *rexpr = (Node *) lfirst(l); > Node *cmp; > > - if (haveRowExpr) > + if (haveRowExpr && IsA(lexpr, RowExpr)) > { > - if (!IsA(lexpr, RowExpr) || > - !IsA(rexpr, RowExpr)) > + if (!IsA(rexpr, RowExpr)) > ereport(ERROR, > > (errcode(ERRCODE_SYNTAX_ERROR), >errmsg("arguments of row IN must all > be row expressions"), > > > Since the restriction seems a rather arbitrary (at least I fail to see > any reason for it), it can be removed altogether (patch 0002, not > tested as well): > > --- a/src/backend/parser/parse_expr.c > +++ b/src/backend/parser/parse_expr.c > @@ -1203,20 +1203,12 @@ transformAExprIn(ParseState *pstate, A_Expr *a) > Node *rexpr = (Node *) lfirst(l); > Node *cmp; > > - if (haveRowExpr) > - { > - if (!IsA(lexpr, RowExpr) || > - !IsA(rexpr, RowExpr)) > - ereport(ERROR, > - > (errcode(ERRCODE_SYNTAX_ERROR), > - errmsg("arguments of row IN must > all be row expressions"), > - > parser_errposition(pstate, a->location))); > + if (IsA(lexpr, RowExpr) && IsA(rexpr, RowExpr)) > cmp = make_row_comparison_op(pstate, > > a->name, > (List *) > copyObject(((RowExpr *) lexpr)->args), > > ((RowExpr *) rexpr)->args, > > a->location); > - } > else > cmp = (Node *) make_op(pstate, >a- > >name, > I had tried, both your patches have passed all regression tests (tested on Windows). I feel fixing it in a way similar to your Patch-1 would be more appropriate as with Patch-1 it can generate meaningful error message for some cases like below: postgres=# select * from the_table where ROW('abc','def') in (row('foo','bar')::the_row,12); ERROR: arguments of row IN must all be row expressions LINE 1: select * from the_table where ROW('abc','def') in (row('foo'... With Regards, Amit Kapila. -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #8198: ROW() literals not supported in an IN clause
On Tue, Jun 4, 2013 at 12:35 PM, Amit Kapila wrote: > On Saturday, June 01, 2013 9:37 PM > >> Row type literals constructed with ROW() cause an error when used in an >> IN >> clause (string literals casted appropriately are allowed). This is >> especially problematic since many client libraries use these literals >> to >> pass values of row-type arguments, hence making it impossible to use >> them in >> IN-clause queries. >> >> To wit: >> divide=# create type the_row as (mfg text, id text); >> CREATE TYPE >> divide=# create table the_table (widget the_row); >> >> >> CREATE TABLE >> >> >> divide=# insert into the_table values(row('foo', 'bar')::the_row); >> >> >> INSERT 0 1 >> >> >> divide=# insert into the_table values('(bar,baz)'::the_row); >> >> >> INSERT 0 1 >> divide=# select * from the_table; >> widget >> --- >> (foo,bar) >> (bar,baz) >> (2 rows) >> >> divide=# select * from the_table where widget in >> ('(foo,bar)'::the_row); >> widget >> --- >> (foo,bar) >> (1 row) >> >> divide=# select * from the_table where widget in >> (row('foo','bar')::the_row); >> ERROR: arguments of row IN must all be row expressions >> LINE 1: select * from the_table where widget in (row('foo','bar')::t... > > The similar query for equal ('=') operator works fine. > select * from the_table where widget = (row('foo','bar')::the_row); > > The reason for above is that in function transformAExprOp(..), it uses > make_row_comparison_op() to operate on expressions only if both left and > right are row expressions, else it will use make_op() to operate on > expressions. Refer code below in function transformAExprOp() > else if (lexpr && IsA(lexpr, RowExpr) && > rexpr && IsA(rexpr, RowExpr)) > { > > result = make_row_comparison_op(pstate, > > a->name, > > ((RowExpr *) lexpr)->args, > > ((RowExpr *) rexpr)->args, > > a->location); > } > else > { > > result = (Node *) make_op(pstate, > a->name, > lexpr, > rexpr, > > a->location); > } > > However for IN clause, if any one expr (left or right) is RowExpr, then it > will try to use make_row_comparison_op, which result in error. > Refer below code of function transformAExprIn(): > if (haveRowExpr) > { > if (!IsA(lexpr, RowExpr) || > !IsA(rexpr, RowExpr)) > ereport(ERROR, > > (errcode(ERRCODE_SYNTAX_ERROR), >errmsg("arguments of row IN must all be > row expressions"), > parser_errposition(pstate, > a->location))); > cmp = make_row_comparison_op(pstate, > >a->name, > (List *) > copyObject(((RowExpr *) lexpr)->args), > >((RowExpr *) rexpr)->args, > >a->location); > } > else > cmp = (Node *) make_op(pstate, >a->name, > > copyObject(lexpr), >rexpr, > > a->location); > > Changing the functionality of transformAExprIn() similar to > transformAExprOp() will fix this issue, but not sure if there is any other > side effect of same. Thanks for the analysis! This problem seems to have been introduced in 3d376fce8dd4 [1] (almost eight years ago! I guess not many people use row types...). I'm pretty sure the original intent was to afford some extra checks so that conditions such as "ROW(1, 2) IN ((ROW(3, 4), ROW(5, 6, 7))" would get rejected at parsing time (CCing the original author; please confirm). If I'm right, the proper fix would be (patch 0001; caution, not tested): --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -1203,10 +1203,9
Re: [BUGS] Memory-leak in BackgroundWriter(and Checkpointer)
Tom, Stephen, Heikki and Andres, You do FAST work! Thanks for your prompt responses. --- Naoya Anzai NEC Soft,Ltd. http://www.necsoft.com/eng/ -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] Memory-leak in BackgroundWriter(and Checkpointer)
* Tom Lane (t...@sss.pgh.pa.us) wrote: > I'm already working on back-patching the attached. Works for me, Thanks! Stephen signature.asc Description: Digital signature
Re: [BUGS] Memory-leak in BackgroundWriter(and Checkpointer)
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> I think the proposed fix is fine code-wise; the real problem here is >> crummy commenting. GetRunningTransactionLocks isn't documented as >> returning a palloc'd array, and why the heck do we have a long comment >> about its implementation in LogStandbySnapshot? > Certainly good questions and better comments would have helped here. I > can go back and rework the patch either way. I'm already working on back-patching the attached. regards, tom lane diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c index 615278b8ca2adf3057e5f21057c3cedfc66480aa..c704412366d5bc4b6512e9ab673855c46f7d993f 100644 *** a/src/backend/storage/ipc/standby.c --- b/src/backend/storage/ipc/standby.c *** LogStandbySnapshot(void) *** 865,880 /* * Get details of any AccessExclusiveLocks being held at the moment. - * - * XXX GetRunningTransactionLocks() currently holds a lock on all - * partitions though it is possible to further optimise the locking. By - * reference counting locks and storing the value on the ProcArray entry - * for each backend we can easily tell if any locks need recording without - * trying to acquire the partition locks and scanning the lock table. */ locks = GetRunningTransactionLocks(&nlocks); if (nlocks > 0) LogAccessExclusiveLocks(nlocks, locks); /* * Log details of all in-progress transactions. This should be the last --- 865,875 /* * Get details of any AccessExclusiveLocks being held at the moment. */ locks = GetRunningTransactionLocks(&nlocks); if (nlocks > 0) LogAccessExclusiveLocks(nlocks, locks); + pfree(locks); /* * Log details of all in-progress transactions. This should be the last diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 8cd871f4b40d7bfc8b0aaa7650241d5865c79ffe..273c72230274b46ba6a46bb8b295ef5375aaa36d 100644 *** a/src/backend/storage/lmgr/lock.c --- b/src/backend/storage/lmgr/lock.c *** GetLockStatusData(void) *** 3398,3415 } /* ! * Returns a list of currently held AccessExclusiveLocks, for use ! * by GetRunningTransactionData(). */ xl_standby_lock * GetRunningTransactionLocks(int *nlocks) { PROCLOCK *proclock; HASH_SEQ_STATUS seqstat; int i; int index; int els; - xl_standby_lock *accessExclusiveLocks; /* * Acquire lock on the entire shared lock data structure. --- 3398,3423 } /* ! * Returns a list of currently held AccessExclusiveLocks, for use by ! * LogStandbySnapshot(). The result is a palloc'd array, ! * with the number of elements returned into *nlocks. ! * ! * XXX This currently takes a lock on all partitions of the lock table, ! * but it's possible to do better. By reference counting locks and storing ! * the value in the ProcArray entry for each backend we could tell if any ! * locks need recording without having to acquire the partition locks and ! * scan the lock table. Whether that's worth the additional overhead ! * is pretty dubious though. */ xl_standby_lock * GetRunningTransactionLocks(int *nlocks) { + xl_standby_lock *accessExclusiveLocks; PROCLOCK *proclock; HASH_SEQ_STATUS seqstat; int i; int index; int els; /* * Acquire lock on the entire shared lock data structure. *** GetRunningTransactionLocks(int *nlocks) *** 3467,3472 --- 3475,3482 } } + Assert(index <= els); + /* * And release locks. We do this in reverse order for two reasons: (1) * Anyone else who needs more than one of the locks will be trying to lock -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] Memory-leak in BackgroundWriter(and Checkpointer)
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Meh. I'm not impressed with permanently allocating an array large > enough to hold all the locks GetRunningTransactionLocks > might return --- that's potentially much larger than the other array, > and in fact I don't think we have a hard limit on its size at all. Well, sure, which is why I didn't actually do that- but I did end up having to make it resize when necessary, which isn't entirely ideal either. > Besides which, it's not like there is *no* cleanup for > GetRunningTransactionData --- it has a lock that has to be released ... That's true.. I guess my general feeling is that it'd be good to do this all one way or the other- having it use a static variable into which we stick the pointer to some reused space for one and then doing a palloc for the other which needs to be pfree'd struck me as odd. > I think the proposed fix is fine code-wise; the real problem here is > crummy commenting. GetRunningTransactionLocks isn't documented as > returning a palloc'd array, and why the heck do we have a long comment > about its implementation in LogStandbySnapshot? Certainly good questions and better comments would have helped here. I can go back and rework the patch either way. Thanks, Stephen signature.asc Description: Digital signature
Re: [BUGS] Memory-leak in BackgroundWriter(and Checkpointer)
* Andres Freund (and...@2ndquadrant.com) wrote: > Seems more consistent with the rest of the code too. But anyway, I am > fine with fixing it either way. Patch attached which mirrors what GetRunningTransactionData() (the other function called from LogStandbySnapshot) does, more-or-less- uses a static variable to keep track of memory allocated for the data being returned to the caller. Looks like this will need to be back-patched to 9.0. Barring objections or better ideas, I'll do that in a few hours. Thanks, Stephen colordiff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c new file mode 100644 index 8cd871f..1fb7884 *** a/src/backend/storage/lmgr/lock.c --- b/src/backend/storage/lmgr/lock.c *** GetLockStatusData(void) *** 3401,3415 * Returns a list of currently held AccessExclusiveLocks, for use * by GetRunningTransactionData(). */ xl_standby_lock * GetRunningTransactionLocks(int *nlocks) { PROCLOCK *proclock; HASH_SEQ_STATUS seqstat; int i; int index; ! int els; ! xl_standby_lock *accessExclusiveLocks; /* * Acquire lock on the entire shared lock data structure. --- 3401,3445 * Returns a list of currently held AccessExclusiveLocks, for use * by GetRunningTransactionData(). */ + #define INIT_STANDBY_LOCK_ARR_SIZE 32 /* Initial return workspace size */ xl_standby_lock * GetRunningTransactionLocks(int *nlocks) { + /* result workspace */ + static xl_standby_locks *accessExclusiveLocks; + PROCLOCK *proclock; HASH_SEQ_STATUS seqstat; int i; int index; ! ! /* ! * Allocating space for all locks is overkill. We only need space for ! * access exclusive locks, but we can't count the number of locks without ! * locking all of the lock partitions. We don't really want to do ! * allocations while holding all those locks, so instead do a bit of ! * allocation up front, to hopefully avoid having to do them later. ! */ ! if (accessExclusiveLocks == NULL) ! { ! /* ! * First call ! * ! * Note: We are tracking the allocated space for locks in ->nlocks, ! * not the actual number of locks found, that's handled with index ! * below. Also, allocating the structure will start us off with space ! * for one, so discount that from the malloc request. ! */ ! accessExclusiveLocks = (xl_standby_locks *) ! malloc(sizeof(xl_standby_locks) + ! ((INIT_STANDBY_LOCK_ARR_SIZE - 1) * sizeof(xl_standby_lock))); ! if (accessExclusiveLocks == NULL) ! ereport(ERROR, ! (errcode(ERRCODE_OUT_OF_MEMORY), ! errmsg("out of memory"))); ! ! accessExclusiveLocks->nlocks = INIT_STANDBY_LOCK_ARR_SIZE; ! } /* * Acquire lock on the entire shared lock data structure. *** GetRunningTransactionLocks(int *nlocks) *** 3419,3433 for (i = 0; i < NUM_LOCK_PARTITIONS; i++) LWLockAcquire(FirstLockMgrLock + i, LW_SHARED); - /* Now we can safely count the number of proclocks */ - els = hash_get_num_entries(LockMethodProcLockHash); - - /* - * Allocating enough space for all locks in the lock table is overkill, - * but it's more convenient and faster than having to enlarge the array. - */ - accessExclusiveLocks = palloc(els * sizeof(xl_standby_lock)); - /* Now scan the tables to copy the data */ hash_seq_init(&seqstat, LockMethodProcLockHash); --- 3449,3454 *** GetRunningTransactionLocks(int *nlocks) *** 3459,3467 if (!TransactionIdIsValid(xid)) continue; ! accessExclusiveLocks[index].xid = xid; ! accessExclusiveLocks[index].dbOid = lock->tag.locktag_field1; ! accessExclusiveLocks[index].relOid = lock->tag.locktag_field2; index++; } --- 3480,3508 if (!TransactionIdIsValid(xid)) continue; ! /* ! * Check if we need to allocate more space in our workspace due to ! * the number of access exclusive locks actually found. If we do ! * run out, double the size of our return area to hopefully avoid ! * having to do this again any time soon. ! */ ! if (index >= accessExclusiveLocks->nlocks) ! { ! accessExclusiveLocks->nlocks *= 2; ! ! accessExclusiveLocks = (xl_standby_locks *) ! realloc(accessExclusiveLocks, ! sizeof(xl_standby_locks) + ! ((accessExclusiveLocks->nlocks - 1) * ! sizeof(xl_standby_lock))); ! if (accessExclusiveLocks == NULL) ! ereport(ERROR, ! (errcode(ERRCODE_OUT_OF_MEMORY), ! errmsg("out of memory"))); ! } ! accessExclusiveLocks->locks[index].xid = xid; ! accessExclusiveLocks->locks[index].dbOid = lock->tag.locktag_field1; ! accessExclusiveLocks->locks[index].relOid = lock->tag.locktag_field2; index++; } *** GetRunningTransactionLocks(int *nlocks) *** 3477,3484 for (i = NUM_LOCK_PARTITIONS; --i >= 0;) LWLockRelease(FirstLockMgrLock + i); *nlocks = index; ! return acce
Re: [BUGS] Memory-leak in BackgroundWriter(and Checkpointer)
Stephen Frost writes: > * Andres Freund (and...@2ndquadrant.com) wrote: >> Seems more consistent with the rest of the code too. But anyway, I am >> fine with fixing it either way. > And this is really the other point- having LogStandbySnapshot() need to > clean up after GetRunningTransactionLocks() but not > GetRunningTransactionData() would strike me as very odd. Meh. I'm not impressed with permanently allocating an array large enough to hold all the locks GetRunningTransactionLocks might return --- that's potentially much larger than the other array, and in fact I don't think we have a hard limit on its size at all. Besides which, it's not like there is *no* cleanup for GetRunningTransactionData --- it has a lock that has to be released ... I think the proposed fix is fine code-wise; the real problem here is crummy commenting. GetRunningTransactionLocks isn't documented as returning a palloc'd array, and why the heck do we have a long comment about its implementation in LogStandbySnapshot? regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
[BUGS] About omitting ideographic space to create array.
To respecting PIC The version I used : postgresql-jdbc-9.1-901.jdbc4 When one of a Postgresql db table has array column which contains Strings. And some of these Array slices contain ideographic space(U+3000 in Unicode6.0). In such a case returned result lacks this ideographic space. My use case is this. An array column has strings of names of image files. And one of these name contains an ideographic space. When a server program got data without this ideographic space and created a hyperlink using this data, our user found the link in a html page dose not work. Because the real file's name contains that ideographic space and the hyperlink dose not. I think an abstract class named AbstractJdbc2Array in the package org.postgresql.jdbc2 dose this mal-effect at line 451-455 as below, // white space else if (!insideString && Character.isWhitespace(chars[i])) { continue; } I know there are some other Whitespace characters like (CR, LF, HT etc.), I tihink at least ideographic space shoud not be ommited. Sincerely Shuhei Aoyama from Tokyo. -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
[BUGS] BUG #8202: can't install database
The following bug has been logged on the website: Bug reference: 8202 Logged by: mrenda delta pamungkas Email address: mre...@sisdh.com PostgreSQL version: 8.4.17 Operating system: windows 7 64 bit Description: http://img822.imageshack.us/img822/5497/postgresqld.png -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #8192: On very large tables the concurrent update with vacuum lag the hot_standby replica
On 04/06/13 16:29, Andres Freund wrote: Hi, On 2013-06-04 16:20:12 +0100, Federico Campoli wrote: Well, if you check the output in that file you can see that 'apply' is progressing, so it's not stuck in some specific area. Is the startup process cpu bound during that time? If so, any chance to get a profile? Please find attached the profile file and the postgresql log generated in the test run on my sandbox. Unfortunately a gprof profile isn't meaningful without the generating binary as far as I know. Could you either generate the callgraph or just generate a perf profile like: # start_test # perf record -a # ctrl-C # perf report > somefile And then send somefile? Greetings, Andres Freund This is the gprof output for the profile. Many thanks Federico -- Federico Campoli Database Administrator brandwatch.com gprof_out.txt.gz Description: GNU Zip compressed data -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #8192: On very large tables the concurrent update with vacuum lag the hot_standby replica
Hi, On 2013-06-04 16:20:12 +0100, Federico Campoli wrote: > >Well, if you check the output in that file you can see that 'apply' is > >progressing, so it's not stuck in some specific area. > >Is the startup process cpu bound during that time? If so, any chance to > >get a profile? > > Please find attached the profile file and the postgresql log generated in > the test run on my sandbox. Unfortunately a gprof profile isn't meaningful without the generating binary as far as I know. Could you either generate the callgraph or just generate a perf profile like: # start_test # perf record -a # ctrl-C # perf report > somefile And then send somefile? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] Memory-leak in BackgroundWriter(and Checkpointer)
* Andres Freund (and...@2ndquadrant.com) wrote: > On 2013-06-04 16:23:00 +0300, Heikki Linnakangas wrote: > > I can't get too excited about the overhead of a single palloc here. It's a > > fairly heavy operation anyway, and only runs once per checkpoint. And we > > haven't heard any actual complaints of latency hiccups with > > wal_level=hot_standby. Perhaps, but we don't always hear about things that we should- this long standing memory leak being one of them. > Seems more consistent with the rest of the code too. But anyway, I am > fine with fixing it either way. And this is really the other point- having LogStandbySnapshot() need to clean up after GetRunningTransactionLocks() but not GetRunningTransactionData() would strike me as very odd. Thanks, Stephen signature.asc Description: Digital signature
Re: [BUGS] Memory-leak in BackgroundWriter(and Checkpointer)
On 2013-06-04 16:23:00 +0300, Heikki Linnakangas wrote: > On 04.06.2013 15:27, Stephen Frost wrote: > >* Naoya Anzai (anzai-na...@mxu.nes.nec.co.jp) wrote: > >>I've found a memory-leak bug in PostgreSQL 9.1.9's background > >>writer process. > > > >This looks legit, but probably not the right approach to fixing it. > >Looks like it'd be better to work out a way to use a static variable to > >reuse the same memory, ala what GetRunningTransactionData() does, and > >avoid having to do allocation while holding all the locks (or at least, > >not very often). > > I can't get too excited about the overhead of a single palloc here. It's a > fairly heavy operation anyway, and only runs once per checkpoint. And we > haven't heard any actual complaints of latency hiccups with > wal_level=hot_standby. I think we will have to resort to running it more frequently in the not too far away future, its way to easy to get into a situation where all of the checkpoints/xl_running_xact's are suboverflowed delaying consistency considerably. Seems more consistent with the rest of the code too. But anyway, I am fine with fixing it either way. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] Memory-leak in BackgroundWriter(and Checkpointer)
On 04.06.2013 15:27, Stephen Frost wrote: * Naoya Anzai (anzai-na...@mxu.nes.nec.co.jp) wrote: I've found a memory-leak bug in PostgreSQL 9.1.9's background writer process. This looks legit, but probably not the right approach to fixing it. Looks like it'd be better to work out a way to use a static variable to reuse the same memory, ala what GetRunningTransactionData() does, and avoid having to do allocation while holding all the locks (or at least, not very often). I can't get too excited about the overhead of a single palloc here. It's a fairly heavy operation anyway, and only runs once per checkpoint. And we haven't heard any actual complaints of latency hiccups with wal_level=hot_standby. - Heikki -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #8192: On very large tables the concurrent update with vacuum lag the hot_standby replica
On 2013-06-04 13:57:58 +0100, Federico Campoli wrote: > On 02/06/13 01:17, Jeff Janes wrote: > >On Thursday, May 30, 2013, wrote: > > > >The following bug has been logged on the website: > > > >Bug reference: 8192 > >Logged by: Federico Campoli > >Email address: feder...@brandwatch.com > >PostgreSQL version: 9.2.4 > >Operating system: Debian 6.0 > >Description: > > > >/* > > > >Description: > > > >It seems on very large tables the concurrent update with vacuum (or > >autovacuum), > >when the slave is in hot standby mode, generates long loops in read on a > >single wal segment during the recovery process. > > > >This have two nasty effects. > >A massive read IO peak and the replay lag increasing as the recovery > >process > >hangs for long periods on a pointless loop. > > > > > >Are you observing a loop, and if so how are you observing it? What is > >it that is looping? > > I'm sorry, just guessing it could be a loop. > The read remains stuck on the same segment. Well, if you check the output in that file you can see that 'apply' is progressing, so it's not stuck in some specific area. Is the startup process cpu bound during that time? If so, any chance to get a profile? > In warm standby everything is fine no lag at all. > > At moment as workaround I've switched to warm standby the slaves as, despite > the low wal generation on the master ~200Mb/minute the slave accumulates a > massive lag when the autovacuum processes starts with hot standby (the peak > was 400 GB and was still increasing before switching to warm standby). By switching to a warm standby you mean disabling hot_standby=on in the standbys or changing wal_level? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #8192: On very large tables the concurrent update with vacuum lag the hot_standby replica
On 02/06/13 01:17, Jeff Janes wrote: On Thursday, May 30, 2013, wrote: The following bug has been logged on the website: Bug reference: 8192 Logged by: Federico Campoli Email address: feder...@brandwatch.com PostgreSQL version: 9.2.4 Operating system: Debian 6.0 Description: /* Description: It seems on very large tables the concurrent update with vacuum (or autovacuum), when the slave is in hot standby mode, generates long loops in read on a single wal segment during the recovery process. This have two nasty effects. A massive read IO peak and the replay lag increasing as the recovery process hangs for long periods on a pointless loop. Are you observing a loop, and if so how are you observing it? What is it that is looping? I'm sorry, just guessing it could be a loop. The read remains stuck on the same segment. On my testbox I have at least 1 minute to 20 Mb/s. On the live system the peak is 124 Mb/s for 2 to 3 minutes without any progress in the wal reply. I've attached the part of postgresql's log with debug2 from my sandbox when that happens. In warm standby everything is fine no lag at all. At moment as workaround I've switched to warm standby the slaves as, despite the low wal generation on the master ~200Mb/minute the slave accumulates a massive lag when the autovacuum processes starts with hot standby (the peak was 400 GB and was still increasing before switching to warm standby). The database is ~ 4 TB costantly updated. Many thanks Federico -- Federico Campoli Database Administrator brandwatch.com debug_output.gz Description: GNU Zip compressed data -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] Memory-leak in BackgroundWriter(and Checkpointer)
* Naoya Anzai (anzai-na...@mxu.nes.nec.co.jp) wrote: > I've found a memory-leak bug in PostgreSQL 9.1.9's background > writer process. This looks legit, but probably not the right approach to fixing it. Looks like it'd be better to work out a way to use a static variable to reuse the same memory, ala what GetRunningTransactionData() does, and avoid having to do allocation while holding all the locks (or at least, not very often). Thanks, Stephen signature.asc Description: Digital signature
Re: [BUGS] BUG #8198: ROW() literals not supported in an IN clause
On Saturday, June 01, 2013 9:37 PM > Row type literals constructed with ROW() cause an error when used in an > IN > clause (string literals casted appropriately are allowed). This is > especially problematic since many client libraries use these literals > to > pass values of row-type arguments, hence making it impossible to use > them in > IN-clause queries. > > To wit: > divide=# create type the_row as (mfg text, id text); > CREATE TYPE > divide=# create table the_table (widget the_row); > > > CREATE TABLE > > > divide=# insert into the_table values(row('foo', 'bar')::the_row); > > > INSERT 0 1 > > > divide=# insert into the_table values('(bar,baz)'::the_row); > > > INSERT 0 1 > divide=# select * from the_table; > widget > --- > (foo,bar) > (bar,baz) > (2 rows) > > divide=# select * from the_table where widget in > ('(foo,bar)'::the_row); > widget > --- > (foo,bar) > (1 row) > > divide=# select * from the_table where widget in > (row('foo','bar')::the_row); > ERROR: arguments of row IN must all be row expressions > LINE 1: select * from the_table where widget in (row('foo','bar')::t... The similar query for equal ('=') operator works fine. select * from the_table where widget = (row('foo','bar')::the_row); The reason for above is that in function transformAExprOp(..), it uses make_row_comparison_op() to operate on expressions only if both left and right are row expressions, else it will use make_op() to operate on expressions. Refer code below in function transformAExprOp() else if (lexpr && IsA(lexpr, RowExpr) && rexpr && IsA(rexpr, RowExpr)) { result = make_row_comparison_op(pstate, a->name, ((RowExpr *) lexpr)->args, ((RowExpr *) rexpr)->args, a->location); } else { result = (Node *) make_op(pstate, a->name, lexpr, rexpr, a->location); } However for IN clause, if any one expr (left or right) is RowExpr, then it will try to use make_row_comparison_op, which result in error. Refer below code of function transformAExprIn(): if (haveRowExpr) { if (!IsA(lexpr, RowExpr) || !IsA(rexpr, RowExpr)) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("arguments of row IN must all be row expressions"), parser_errposition(pstate, a->location))); cmp = make_row_comparison_op(pstate, a->name, (List *) copyObject(((RowExpr *) lexpr)->args), ((RowExpr *) rexpr)->args, a->location); } else cmp = (Node *) make_op(pstate, a->name, copyObject(lexpr), rexpr, a->location); Changing the functionality of transformAExprIn() similar to transformAExprOp() will fix this issue, but not sure if there is any other side effect of same. With Regards, Amit Kapila. -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
[BUGS] Memory-leak in BackgroundWriter(and Checkpointer)
Hi All. I've found a memory-leak bug in PostgreSQL 9.1.9's background writer process. When following parameter is set, it occurs every time CHECKPOINT runs. wal_level = hot_standby I've also confirmed REL9_1_STABLE and it is not fixed yet. In additional, it also occurs in 9.3.beta1's checkpointer. I created a patch (attached file) for 9.1.9, so could anyone confirm? Best regards. --- Naoya Anzai NEC Soft,Ltd. http://www.necsoft.com/eng/ standby.patch Description: Binary data -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs