Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On Mon, May 25, 2015 at 10:17 AM, Peter Geoghegan wrote: > While trying to fix a largely unrelated bug, I noticed that the new > build_tlist_index() call for the "excluded" targetlist (used by ON > CONFLICT DO UPDATE queries) does not have its memory subsequently > freed by the caller. Since every other call to build_tlist_index() > does this, and comments above build_tlist_index() encourage it, I > think the new caller should do the same. > > Attached patch adds such a pfree() call. Yep. This looks correct to me. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Run pgindent now?
On Sun, May 24, 2015 at 10:44:10AM -0400, Tom Lane wrote: > Andrew Dunstan writes: > > On 05/23/2015 11:37 PM, Tom Lane wrote: > >> No, pgindent has *always* been wonky about lines that contain a typedef > >> name but are not variable declarations. > > > Well, that sounds like something we should try to patch, doesn't it? > > (No, I'm not volunteering.) > > In the past, the main argument against changing pgindent has been that > it would cause reformatting of settled code. For example, when Bruce > twiddled its right margin limit for comments around 8.1, that caused > literally years worth of back-patching pain. If we can get to an > agreement on re-indenting back branches at the same time as master, > then this problem would go away, and I for one would get a lot more > enthusiastic about trying to improve pgindent rather than just working > around its oddities. > > Not that I'm volunteering either right now. That is something we will need to fix in the BSD indent code. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] Run pgindent now?
On Sun, May 24, 2015 at 08:31:32AM -0400, Bruce Momjian wrote: > > >> Also, does somebody have an idea to keep pgindent from butchering inline > > >> asm like: > > > > > Ah, we have a file /pgtop/src/tools/pgindent/exclude_file_patterns which > > > has excluded files, and s_lock.h is one of them. I think > > > /include/port/atomics/arch-x86.h needs to be added, and the pgindent > > > commit on the file reverted. > > > > Yeah, probably :-( > > OK, will do. I am going to revert and exclude the entire > include/port/atomics directory. Done. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
While trying to fix a largely unrelated bug, I noticed that the new build_tlist_index() call for the "excluded" targetlist (used by ON CONFLICT DO UPDATE queries) does not have its memory subsequently freed by the caller. Since every other call to build_tlist_index() does this, and comments above build_tlist_index() encourage it, I think the new caller should do the same. Attached patch adds such a pfree() call. -- Peter Geoghegan diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 90e13e4..8afe6a3 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -776,6 +776,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) linitial_int(splan->resultRelations), rtoffset); + pfree(itlist); + splan->exclRelTlist = fix_scan_list(root, splan->exclRelTlist, rtoffset); } -- 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] problems on Solaris
On 2015-05-24 21:01:54 -0400, Andrew Dunstan wrote: > Yes, but it wasn't running these tests until a few days ago when its > buildfarm software was upgraded. But barriers are used in other places too... -- 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] problems on Solaris
On 05/24/2015 08:07 PM, Andres Freund wrote: On 2015-05-24 19:44:37 -0400, Andrew Dunstan wrote: Buildfarm members casteroides and protosciurus have been having some problems that seem puzzling. These animals both run on the same machine, but with different compilers. casteroides runs with the Sun Studio 12 compiler, and has twice in the last 3 days demonstrated this error: [5561ce0c.51b7:25] LOG: starting background worker process "test_shm_mq" [5561ce1e.5287:9] PANIC: stuck spinlock (100cb77f4) detected at atomics.c:30 [5561ce1e.5287:10] STATEMENT: SELECT test_shm_mq_pipelined(16384, (select string_agg(chr(32+(random()*95)::int), '') from generate_series(1,27)), 200, 3); [5561ce0c.51b7:26] LOG: server process (PID 21127) was terminated by signal 6 [5561ce0c.51b7:27] DETAIL: Failed process was running: SELECT test_shm_mq_pipelined(16384, (select string_agg(chr(32+(random()*95)::int), '') from generate_series(1,27)), 200, 3); [5561ce0c.51b7:28] LOG: terminating any other active server processes It's not constant - between the two failures was a success. That's indeed rather odd. For one the relevant code does nothing but lock/unlock a spinlock. For another, there's been no recent change to this and casteroides has been running happily for a long time. Yes, but it wasn't running these tests until a few days ago when its buildfarm software was upgraded. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: 9.5 release notes may need ON CONFLICT DO NOTHING compatibility notice for FDW authors
On Sun, May 24, 2015 at 5:16 PM, Peter Geoghegan wrote: > AddForeignUpdateTargets() actually won't be called with ON CONFLICT DO > UPDATE, and so it isn't exactly true that the only obstacle to making > FDWs support ON CONFLICT DO UPDATE is around inference of arbiter > unique indexes on the foreign side. It's *almost* true, though. Attached patch clears this up within the fdw-handler documentation. I think it's worth separately noting from the existing commentary on limitations with FDWs and ON CONFLICT. -- Peter Geoghegan diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml index 2361577..569a22d 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -399,6 +399,13 @@ AddForeignUpdateTargets (Query *parsetree, + Note that AddForeignUpdateTargets will not be called + for INSERT operations with an ON CONFLICT DO + UPDATE clause. Such INSERT operations are + unsupported when a foreign table is targeted. + + + List * PlanForeignModify (PlannerInfo *root, -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: 9.5 release notes may need ON CONFLICT DO NOTHING compatibility notice for FDW authors
On Sun, May 24, 2015 at 4:22 PM, Peter Geoghegan wrote: > As things stand, every other possible ON CONFLICT clause will throw an > error in some way before the FDW is consulted at all, so FDW authors > need not concern themselves with those other cases (unless perhaps we > allow ON CONFLICT DO UPDATE to not require an inference specification > in a last minute behavioral tweak, as suggested by Simon Riggs, making > ON CONFLICT DO UPDATE support by foreign data wrappers a possibility > that must be considered). AddForeignUpdateTargets() actually won't be called with ON CONFLICT DO UPDATE, and so it isn't exactly true that the only obstacle to making FDWs support ON CONFLICT DO UPDATE is around inference of arbiter unique indexes on the foreign side. It's *almost* true, though. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] problems on Solaris
On 2015-05-24 19:44:37 -0400, Andrew Dunstan wrote: > > Buildfarm members casteroides and protosciurus have been having some > problems that seem puzzling. These animals both run on the same machine, but > with different compilers. > > casteroides runs with the Sun Studio 12 compiler, and has twice in the last > 3 days demonstrated this error: > >[5561ce0c.51b7:25] LOG: starting background worker process "test_shm_mq" >[5561ce1e.5287:9] PANIC: stuck spinlock (100cb77f4) detected at > atomics.c:30 >[5561ce1e.5287:10] STATEMENT: SELECT test_shm_mq_pipelined(16384, (select > string_agg(chr(32+(random()*95)::int), '') from generate_series(1,27)), > 200, 3); >[5561ce0c.51b7:26] LOG: server process (PID 21127) was terminated by > signal 6 >[5561ce0c.51b7:27] DETAIL: Failed process was running: SELECT > test_shm_mq_pipelined(16384, (select string_agg(chr(32+(random()*95)::int), > '') from generate_series(1,27)), 200, 3); >[5561ce0c.51b7:28] LOG: terminating any other active server processes > > It's not constant - between the two failures was a success. That's indeed rather odd. For one the relevant code does nothing but lock/unlock a spinlock. For another, there's been no recent change to this and casteroides has been running happily for a long time. > protociurus runs with gcc 3.4.3 and gets this error: > >gcc -Wall -Wmissing-prototypes -Wpointer-arith > -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute > -Wformat-security -fno-strict-aliasing -fwrapv > -Wno-unused-command-line-argument -g -I/usr/local/include -m64 -I. > -I../../../src/interfaces/libpq -I./../regress -I../../../src/include -c -o > specparse.o specparse.c >In file included from /usr/include/sys/vnode.h:47, > from /usr/include/sys/stream.h:22, > from /usr/include/netinet/in.h:66, > from /usr/include/netdb.h:98, > from ../../../src/include/port.h:17, > from ../../../src/include/c.h:1114, > from ../../../src/include/postgres_fe.h:25, > from specparse.y:13: >/usr/include/sys/kstat.h:439: error: syntax error before numeric constant >/usr/include/sys/kstat.h:463: error: syntax error before '}' token >/usr/include/sys/kstat.h:464: error: syntax error before '}' token >In file included from /usr/include/sys/stream.h:22, > from /usr/include/netinet/in.h:66, > from /usr/include/netdb.h:98, > from ../../../src/include/port.h:17, > from ../../../src/include/c.h:1114, > from ../../../src/include/postgres_fe.h:25, > from specparse.y:13: >/usr/include/sys/vnode.h:105: error: syntax error before "kstat_named_t" I'd noticed this one as well. This sounds like a installation problem, not really ours. Dave, any chance you could look into this, or give somebody an account to test what's up? Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] problems on Solaris
Buildfarm members casteroides and protosciurus have been having some problems that seem puzzling. These animals both run on the same machine, but with different compilers. casteroides runs with the Sun Studio 12 compiler, and has twice in the last 3 days demonstrated this error: [5561ce0c.51b7:25] LOG: starting background worker process "test_shm_mq" [5561ce1e.5287:9] PANIC: stuck spinlock (100cb77f4) detected at atomics.c:30 [5561ce1e.5287:10] STATEMENT: SELECT test_shm_mq_pipelined(16384, (select string_agg(chr(32+(random()*95)::int), '') from generate_series(1,27)), 200, 3); [5561ce0c.51b7:26] LOG: server process (PID 21127) was terminated by signal 6 [5561ce0c.51b7:27] DETAIL: Failed process was running: SELECT test_shm_mq_pipelined(16384, (select string_agg(chr(32+(random()*95)::int), '') from generate_series(1,27)), 200, 3); [5561ce0c.51b7:28] LOG: terminating any other active server processes It's not constant - between the two failures was a success. protociurus runs with gcc 3.4.3 and gets this error: gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -g -I/usr/local/include -m64 -I. -I../../../src/interfaces/libpq -I./../regress -I../../../src/include -c -o specparse.o specparse.c In file included from /usr/include/sys/vnode.h:47, from /usr/include/sys/stream.h:22, from /usr/include/netinet/in.h:66, from /usr/include/netdb.h:98, from ../../../src/include/port.h:17, from ../../../src/include/c.h:1114, from ../../../src/include/postgres_fe.h:25, from specparse.y:13: /usr/include/sys/kstat.h:439: error: syntax error before numeric constant /usr/include/sys/kstat.h:463: error: syntax error before '}' token /usr/include/sys/kstat.h:464: error: syntax error before '}' token In file included from /usr/include/sys/stream.h:22, from /usr/include/netinet/in.h:66, from /usr/include/netdb.h:98, from ../../../src/include/port.h:17, from ../../../src/include/c.h:1114, from ../../../src/include/postgres_fe.h:25, from specparse.y:13: /usr/include/sys/vnode.h:105: error: syntax error before "kstat_named_t" /usr/include/sys/vnode.h:107: error: syntax error before "nread" /usr/include/sys/vnode.h:108: error: syntax error before "read_bytes" /usr/include/sys/vnode.h:109: error: syntax error before "nwrite" /usr/include/sys/vnode.h:110: error: syntax error before "write_bytes" /usr/include/sys/vnode.h:111: error: syntax error before "nioctl" /usr/include/sys/vnode.h:112: error: syntax error before "nsetfl" /usr/include/sys/vnode.h:113: error: syntax error before "ngetattr" /usr/include/sys/vnode.h:114: error: syntax error before "nsetattr" /usr/include/sys/vnode.h:115: error: syntax error before "naccess" /usr/include/sys/vnode.h:116: error: syntax error before "nlookup" /usr/include/sys/vnode.h:117: error: syntax error before "ncreate" /usr/include/sys/vnode.h:118: error: syntax error before "nremove" /usr/include/sys/vnode.h:119: error: syntax error before "nlink" /usr/include/sys/vnode.h:120: error: syntax error before "nrename" /usr/include/sys/vnode.h:121: error: syntax error before "nmkdir" /usr/include/sys/vnode.h:122: error: syntax error before "nrmdir" /usr/include/sys/vnode.h:123: error: syntax error before "nreaddir" /usr/include/sys/vnode.h:124: error: syntax error before "readdir_bytes" /usr/include/sys/vnode.h:125: error: syntax error before "nsymlink" /usr/include/sys/vnode.h:126: error: syntax error before "nreadlink" /usr/include/sys/vnode.h:127: error: syntax error before "nfsync" /usr/include/sys/vnode.h:128: error: syntax error before "ninactive" /usr/include/sys/vnode.h:129: error: syntax error before "nfid" /usr/include/sys/vnode.h:130: error: syntax error before "nrwlock" /usr/include/sys/vnode.h:131: error: syntax error before "nrwunlock" /usr/include/sys/vnode.h:132: error: syntax error before "nseek" /usr/include/sys/vnode.h:133: error: syntax error before "ncmp" /usr/include/sys/vnode.h:134: error: syntax error before "nfrlock" /usr/include/sys/vnode.h:135: error: syntax error before "nspace" /usr/include/sys/vnode.h:136: error: syntax error before "nrealvp" /usr/include/sys/vnode.h:137: error: syntax error before "ngetpage" /usr/include/sys/vnode.h:138: error: syntax error before "nputpage" /usr/include/sys/vnode.h:139: error: syntax error before "nmap" /usr/include/sys/vnode.h:140: error: syntax error before "naddmap" /usr/include/sys/vnode.h:141: error: syntax error before "ndelmap" /usr/inclu
[HACKERS] 9.5 release notes may need ON CONFLICT DO NOTHING compatibility notice for FDW authors
postgres_fdw supports ON CONFLICT DO NOTHING, provided no inference specification is provided. Foreign tables do not have associated unique indexes (or exclusion constraints) as far as the optimizer is concerned, and so Postgres does not accept an inference specification for foreign tables -- the optimizer will simply complain that a unique index that satisfies the user's inference specification is unavailable. There is no support for ON CONFLICT DO UPDATE with postgres_fdw, but that's really only because an inference specification (or explicitly named constraint) is always required for DO UPDATE. The deparsing support actually added will have deparsing add "ON CONFLICT DO NOTHING" for the SQL generated for execution on foreign servers if the original statement had that exact, unadorned ON CONFLICT clause. As things stand, every other possible ON CONFLICT clause will throw an error in some way before the FDW is consulted at all, so FDW authors need not concern themselves with those other cases (unless perhaps we allow ON CONFLICT DO UPDATE to not require an inference specification in a last minute behavioral tweak, as suggested by Simon Riggs, making ON CONFLICT DO UPDATE support by foreign data wrappers a possibility that must be considered). postgres_fdw handles the one simple ON CONFLICT DO NOTHING case (the only case that can actually reach it), but no other FDW we ship pay any attention. Do we need to make existing contrib FDWs, like file_fdw, explicitly reject unadorned ON CONFLICT DO NOTHING clauses as wrong-headed? Is it okay to just let them not pay attention at all on the theory that it's the same as performing INSERT ... ON CONFLICT DO NOTHING on a table that has no unique indexes or exclusion constraints? In any case, third party foreign data wrappers that target other database system will totally ignore ON CONFLICT DO NOTHING when built against the master branch (unless they consider these questions). They should perhaps make a point of rejecting DO NOTHING outright where it makes sense that support could exist, but it just doesn't. Or they could just add support (I imagine that this would be very easy for mysql_fdw, for example -- MySQL has INSERT IGNORE). I feel a compatibility item in the release notes is in order so the question is considered, but there seems to be no place to do that on the Wiki, and the original commit message does not have a note like this. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] jsonb concatenate operator's semantics seem questionable
On 05/24/2015 05:38 PM, Tom Lane wrote: Andres Freund writes: On 2015-05-24 12:17:35 -0700, Peter Geoghegan wrote: Having gone to the trouble of making the parser support this stuff (in a way that makes us not follow the SQL standard in a couple of places), we ought to have a similar capability for jsonb. I haven't looked into it, but it seems like a good project for 9.6. I'm not volunteering to undertake the project, though. I'm not convinced. The array stuff requires ugly contortions in a bunch of places, and it's likely going to be worse for jsonb. FWIW, I've got some interest myself in the idea of allowing subscripting syntax to be applied to things other than plain arrays, which I think is what Peter is proposing here. You could imagine applying it to hstore, for example, and ending up with something that acts like a Perl hash (and even performs similarly, once you'd invented an expanded-object representation for hstore). Coming up with a non-ugly API for datatypes would be the hard part. Interesting, you do cast a wide net these days. I imagine we'd have each type register a function along the lines of foo_set(target foo, newval element_of_foo, path variadic "any") returns boolean And then we'd turn set myfoo[bar][baz][blurfl] = someval into foo_set(myfoo, someval, bar, baz, blurfl) In the catalog I guess we'd need to store the oid of the function, and possibly oid of the element type (e.g. for jsonb it would just be jsonb), and some dependency information. But I'm sure there a a great many wrinkles I haven't thought of. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] jsonb concatenate operator's semantics seem questionable
Andres Freund writes: > On 2015-05-24 12:17:35 -0700, Peter Geoghegan wrote: >> Having gone to the trouble of making the parser support this stuff (in >> a way that makes us not follow the SQL standard in a couple of >> places), we ought to have a similar capability for jsonb. I haven't >> looked into it, but it seems like a good project for 9.6. I'm not >> volunteering to undertake the project, though. > I'm not convinced. The array stuff requires ugly contortions in a bunch > of places, and it's likely going to be worse for jsonb. FWIW, I've got some interest myself in the idea of allowing subscripting syntax to be applied to things other than plain arrays, which I think is what Peter is proposing here. You could imagine applying it to hstore, for example, and ending up with something that acts like a Perl hash (and even performs similarly, once you'd invented an expanded-object representation for hstore). Coming up with a non-ugly API for datatypes would be the hard part. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] jsonb concatenate operator's semantics seem questionable
On 2015-05-24 12:17:35 -0700, Peter Geoghegan wrote: > Having gone to the trouble of making the parser support this stuff (in > a way that makes us not follow the SQL standard in a couple of > places), we ought to have a similar capability for jsonb. I haven't > looked into it, but it seems like a good project for 9.6. I'm not > volunteering to undertake the project, though. I'm not convinced. The array stuff requires ugly contortions in a bunch of places, and it's likely going to be worse for jsonb. Greetings, Andres Freund -- 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] jsonb concatenate operator's semantics seem questionable
On 05/24/2015 03:17 PM, Peter Geoghegan wrote: On Thu, May 21, 2015 at 2:25 PM, Andrew Dunstan wrote: This change really makes this set of jsonb features quite a bit more compelling. I'm glad I thought of it - wish I had done so earlier. So notwithstanding the controversy upthread, I think this is a good result. I think that we should look into making jsonb support array-style subscripting within updates (to update "nested subdatums" directly). This would make the new concatenate operator a lot more compelling. Also, UPDATE targetlists don't accept a table qualification in their targetlist (for the assign-to column) because the parser similarly needs to support updating composite type's "nested subdatums" directly. Having gone to the trouble of making the parser support this stuff (in a way that makes us not follow the SQL standard in a couple of places), we ought to have a similar capability for jsonb. I haven't looked into it, but it seems like a good project for 9.6. I'm not volunteering to undertake the project, though. Yes, sounds like it would be good. I too am not volunteering. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] jsonb concatenate operator's semantics seem questionable
On Thu, May 21, 2015 at 2:25 PM, Andrew Dunstan wrote: > This change really makes this set of jsonb features quite a bit more > compelling. I'm glad I thought of it - wish I had done so earlier. So > notwithstanding the controversy upthread, I think this is a good result. I think that we should look into making jsonb support array-style subscripting within updates (to update "nested subdatums" directly). This would make the new concatenate operator a lot more compelling. Also, UPDATE targetlists don't accept a table qualification in their targetlist (for the assign-to column) because the parser similarly needs to support updating composite type's "nested subdatums" directly. Having gone to the trouble of making the parser support this stuff (in a way that makes us not follow the SQL standard in a couple of places), we ought to have a similar capability for jsonb. I haven't looked into it, but it seems like a good project for 9.6. I'm not volunteering to undertake the project, though. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fsync-pgdata-on-recovery tries to write to more files than previously
Christoph Berg writes: > Re: To Andres Freund 2015-05-24 <20150524075244.gb27...@msg.df7cb.de> >> Re: Andres Freund 2015-05-24 <20150524005245.gd32...@alap3.anarazel.de> >>> How about, to avoid masking actual problems, we have a more >>> differentiated logic for the toplevel data directory? > pg_log/ is also admin domain. What about only recursing into > well-known directories + postgresql.auto.conf? The idea that this code would know exactly what's what under $PGDATA scares me. I can positively guarantee that it would diverge from reality over time, and nobody would notice until it ate their data, failed to start, or otherwise behaved undesirably. pg_log/ is a perfect example, because that is not a hard-wired directory name; somebody could point the syslogger at a different place very easily. Wiring in special behavior for that name is just wrong. I would *much* rather have a uniform rule for how to treat each file the scan comes across. It might take some tweaking to get to one that works well; but once we did, we could have some confidence that it wouldn't break later. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Run pgindent now?
Andrew Dunstan writes: > On 05/23/2015 11:37 PM, Tom Lane wrote: >> No, pgindent has *always* been wonky about lines that contain a typedef >> name but are not variable declarations. > Well, that sounds like something we should try to patch, doesn't it? > (No, I'm not volunteering.) In the past, the main argument against changing pgindent has been that it would cause reformatting of settled code. For example, when Bruce twiddled its right margin limit for comments around 8.1, that caused literally years worth of back-patching pain. If we can get to an agreement on re-indenting back branches at the same time as master, then this problem would go away, and I for one would get a lot more enthusiastic about trying to improve pgindent rather than just working around its oddities. Not that I'm volunteering either right now. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Run pgindent now?
On 05/23/2015 11:37 PM, Tom Lane wrote: Bruce Momjian writes: On Sun, May 24, 2015 at 04:16:07AM +0200, Andres Freund wrote: - if (IsA(node, Aggref) || IsA(node, GroupingFunc)) + if (IsA(node, Aggref) ||IsA(node, GroupingFunc)) There's a bunch of changes like this. Looks rather odd to me? I don't recall seing much code looking like that? Wow, that is quite odd. No, pgindent has *always* been wonky about lines that contain a typedef name but are not variable declarations. I've gotten in the habit of breaking IsA tests like these into two lines: if (IsA(node, Aggref) || IsA(node, GroupingFunc)) just so that it doesn't look weird when pgindent gets done with it. You can see similar weirdness around sizeof usages, if you look. Well, that sounds like something we should try to patch, doesn't it? (No, I'm not volunteering.) cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Run pgindent now?
On Sat, May 23, 2015 at 11:37:30PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > On Sun, May 24, 2015 at 04:16:07AM +0200, Andres Freund wrote: > >> - if (IsA(node, Aggref) || IsA(node, GroupingFunc)) > >> + if (IsA(node, Aggref) ||IsA(node, GroupingFunc)) > >> > >> There's a bunch of changes like this. Looks rather odd to me? I don't > >> recall seing much code looking like that? > > > Wow, that is quite odd. > > No, pgindent has *always* been wonky about lines that contain a typedef > name but are not variable declarations. I've gotten in the habit of > breaking IsA tests like these into two lines: > > if (IsA(node, Aggref) || > IsA(node, GroupingFunc)) > > just so that it doesn't look weird when pgindent gets done with it. > You can see similar weirdness around sizeof usages, if you look. Oh, I checked if IsA was a typedef, but it isn't. I see now the problem is the Aggref typedef on the line. > >> Also, does somebody have an idea to keep pgindent from butchering inline > >> asm like: > > > Ah, we have a file /pgtop/src/tools/pgindent/exclude_file_patterns which > > has excluded files, and s_lock.h is one of them. I think > > /include/port/atomics/arch-x86.h needs to be added, and the pgindent > > commit on the file reverted. > > Yeah, probably :-( OK, will do. I am going to revert and exclude the entire include/port/atomics directory. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] xid wrap / optimize frozen tables?
Hi Jeff and all, On 23/05/15 22:13, Jeff Janes wrote: > Are you sure it is the read IO that causes the problem? Yes. Trouble is here that we are talking about a 361 GB table List of relations Schema |Name | Type | Owner |Size| Description +-+--+--++- public | *redacted*_y2015m04 | table| postgres | 361 GB | and while we have shared_buffers = 325GB huge_pages = on this is not the only table of this size (total db size ist 1.8tb) and more current data got written to *redacted*_y2015m05 (the manually-partitioned table for may), so most of the m04 data would have got evicted from the cache when this issue surfaced initially. There is one application pushing data (bulk inserts) and we have transaction rates for this app in a log. The moment the vacuum started, these rates dropped. Unfortunately I cannot present helpful log excerpts here as the autovacuum never finished so far (because the admin killed the db), so we have zero logging about past autovac events. At the moment, the application is shut down and the machine is only running the vacs: query_start | 2015-05-22 19:33:52.44334+02 waiting | f query| autovacuum: VACUUM public.*redacted*_y2015m04 (to prevent wraparound) query_start | 2015-05-22 19:34:02.46004+02 waiting | f query| autovacuum: VACUUM ANALYZE public.*redacted*_y2015m05 (to prevent wraparound) so we know that any io must be caused by the vacs: shell# uptime 13:33:33 up 1 day, 18:01, 2 users, load average: 5.75, 12.71, 8.43 shell# zpool iostat capacity operationsbandwidth pool alloc free read write read write --- - - - - - - tank1 358G 6.90T872 55 15.1M 3.08M Again, we know IO capacity is insufficient, the pool is on 2 magnetic disks only atm, so an avg read rate of 872 IOPS averaged over 42 hours is not even bad. > I don't know happened to that, but there is another patch waiting for review > and > testing: > > https://commitfest.postgresql.org/5/221/ This is really interesting, thank you very much for the pointer. Cheers, Nils -- 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] fsync-pgdata-on-recovery tries to write to more files than previously
Re: To Andres Freund 2015-05-24 <20150524075244.gb27...@msg.df7cb.de> > Re: Andres Freund 2015-05-24 <20150524005245.gd32...@alap3.anarazel.de> > > How about, to avoid masking actual problems, we have a more > > differentiated logic for the toplevel data directory? I think we could > > just skip all non-directory files in there data_directory itself. None > > of the files in the toplevel directory, with the exception of > > postgresql.auto.conf, will ever get written to by PG itself. And if > > there's readonly files somewhere in a subdirectory, I won't feel > > particularly bad. pg_log/ is also admin domain. What about only recursing into well-known directories + postgresql.auto.conf? (I've also been wondering if pg_basebackup shouldn't skip pg_log, but that's a different topic...) Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- 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] fsync-pgdata-on-recovery tries to write to more files than previously
Re: Andres Freund 2015-05-24 <20150524005245.gd32...@alap3.anarazel.de> > How about, to avoid masking actual problems, we have a more > differentiated logic for the toplevel data directory? I think we could > just skip all non-directory files in there data_directory itself. None > of the files in the toplevel directory, with the exception of > postgresql.auto.conf, will ever get written to by PG itself. And if > there's readonly files somewhere in a subdirectory, I won't feel > particularly bad. I like that idea. Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
