Re: [HACKERS] Last Commitfest patches waiting review
On Mon, Oct 6, 2014 at 11:53 AM, Peter Geoghegan p...@heroku.com wrote: On Mon, Oct 6, 2014 at 11:27 AM, Robert Haas robertmh...@gmail.com wrote: Well, really, I was just suggesting that I can spend more time on the patch, but not immediately. We haven't really talked about the idea of the HyperLogLog-based abort mechanism - the actual cost model - even though I thought we'd have discussed that extensively by now. My concern is that if we only get it committed in the last commitfest, we may run out of time to make sortsupport work for B-Tree index builds. That's where the sortsupport for text stuff will be really useful. -- 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] Last Commitfest patches waiting review
On 10/09/2014 09:59 PM, Peter Geoghegan wrote: My concern is that if we only get it committed in the last commitfest, we may run out of time to make sortsupport work for B-Tree index builds. That's where the sortsupport for text stuff will be really useful. B-tree index build uses tuplesort.c. What's missing? - Heikki -- 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] Last Commitfest patches waiting review
On Thu, Oct 9, 2014 at 12:14 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: B-tree index build uses tuplesort.c. What's missing? I don't think that all that much is missing. Tuplesort expects to work with an index scankey when sorting B-Tree tuples. There needs to be something like a reverse lookup of the sortsupport function. It looks like a historical oversight, that would take time to fix, but wouldn't be particularly challenging. You'd need to pick out the operators from the scankey, so you'd have something like what tuplesort_begin_heap() starts off with with tuplesort_begin_index_btree(). copytup_index() would then later need to be modifed to make abbreviation occur there too, but that's no big deal. -- 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] Last Commitfest patches waiting review
On Thu, Oct 9, 2014 at 12:51 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Oh, I didn't realize we don't do that already! I'm surprised, I would've expected index build to have been the first thing we'd use the SortSupport stuff in. The thing is that the most compelling numbers for sortsupport (plus the related improvements to tuplesort itself) were from the onlyKey optimization - the numbers are only so-so when you look at multi-attribute sorts, because we specialize qsort() for both of those two cases (i.e. one specialization, qsort_ssup(), only looks at datum1, while qsort_tuple() looks at everything else, through which comparetup_heap() and comparetup_datum() are called). But with the B-Tree comparator, we're only ever going to be able to use qsort_tuple() which is roughly equivalent to the so-so multi-attribute case for heap tuples (because we need to detect duplicate violations, and a few other things - no choice there). It kind of makes sense that we didn't push ourselves to get B-Tree support until now. But with sortsupport for text accelerating sorts by perhaps as much as 10 times in sympathetic (though realistic) cases, it would be crazy to have that without B-Tree support (abbreviated keys always force us to use the qsort_tuple() specialization, because the comparator tie-breaker logic must live in places like comparetup_heap()). Yeah, that seems worth doing, independently of the this patch. As I mentioned, that might be less true than you'd think. Can you write a separate patch to use SortSupport for B-tree index builds, please? Eliminating the FunctionCallInfoData overhead should shave off some some cycles from every index build. I'll look into it. Hopefully an effort to actually implement it will show that I was right, and there isn't much to it. -- 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] Last Commitfest patches waiting review
On Thu, Oct 9, 2014 at 1:13 PM, Peter Geoghegan p...@heroku.com wrote: Can you write a separate patch to use SortSupport for B-tree index builds, please? Eliminating the FunctionCallInfoData overhead should shave off some some cycles from every index build. I'll look into it. Hopefully an effort to actually implement it will show that I was right, and there isn't much to it. I was able to get this working pretty quickly. All regression tests pass. I'm not going to post a patch just yet, because I still need to make the cluster case work, and I'll probably want to refactor my approach to performing catalog lookups a bit, but the important point is that my original suspicion that this isn't very difficult or invasive has been confirmed. I should have something to post before too long. -- 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] Last Commitfest patches waiting review
On Fri, Oct 3, 2014 at 12:14 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Sort support for text with strxfrm() poor man's keys Robert? What do you think of Peter's latest patch? I haven't had time to look at it yet. Can we move it to the next CommitFest? I spent a lot of time on this one, but I can't keep doing that forever, because, you know, other work. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Last Commitfest patches waiting review
On Mon, Oct 6, 2014 at 7:57 AM, Robert Haas robertmh...@gmail.com wrote: I haven't had time to look at it yet. Can we move it to the next CommitFest? I spent a lot of time on this one, but I can't keep doing that forever, because, you know, other work. Are you suggesting that it would be useful to have input from another person? Because if you are, then I agree that ideally that would be possible. -- 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] Last Commitfest patches waiting review
On Mon, Oct 6, 2014 at 11:27 AM, Robert Haas robertmh...@gmail.com wrote: Well, really, I was just suggesting that I can spend more time on the patch, but not immediately. We haven't really talked about the idea of the HyperLogLog-based abort mechanism - the actual cost model - even though I thought we'd have discussed that extensively by now. Maybe I'm reading too much into that, but my guess is that that's because it's a particularly hard thing to have an opinion on. I think that It might not be a bad idea to have another opinion on that in particular. We've historically resisted this kind of special case optimization, and yet the optimization is likely to be very effective in many representative cases. Plus, some aspects of the cost model are a bit fuzzy, in a way that things in the executor ordinarily are not, and so I can understand how this would present any reviewer with difficulty. By the way, the original description of this approach to accelerating sorts I saw was here, in this 2001 paper: http://lgis.informatik.uni-kl.de/archiv/wwwdvs.informatik.uni-kl.de/courses/DBSREAL/SS2001/Vorlesungsunterlagen/Implementing_Sorting.pdf (Under 2.4 Cache-optimized techniques) -- 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] Last Commitfest patches waiting review
2014-10-03 23:14 GMT+07:00 Heikki Linnakangas hlinnakan...@vmware.com: Fix xpath() to return namespace definitions (fixes the issue with nested or repeated xpath()) Peter, you've done some XML stuff before; could you have a look at this too? I am the author of the patch. I've sent Peter off-the-list review request email as you had suggested before, but he didn't respond. How can i help to ease the review? The last patch is re-implementation, as per Tom Lane's findings about xmlNodeCopy's behavior if there's not enough memory. It turns out that the reimplementation is not as simple as before (because reimplement some of xmlNodeCopy code must be reimplemented here). Reviewing the patch myself, i've found some code formatting problems. Will fix and post in the patch's thread. Regards, -- Ali Akbar
Re: [HACKERS] Last Commitfest patches waiting review
Thanks to everyone's who's reviewed a patch so far. One last crunch, and we'll be through. We have 7 patches left in Needs Review state: pg_receivexlog: addition of --create/--drop to create/drop repslots Waiting for Magnus. Amit promised to take a look if Magnus continues to be busy. Sort support for text with strxfrm() poor man's keys Robert? What do you think of Peter's latest patch? add latency limit to pgbench throttling (--rate) I'm waiting for Greg Smith to have a look at the latest patch. Escaping from blocked send() by pg_terminate_backend(). Andres posted a patch series using a completely different approach. I guess this should be just marked as returned with feedback, if we're going to pursue the different approach instead. Fix local_preload_libraries to work as expected. pg_dump refactor to remove global variables Peter: Ping? Will you have the time to review these? Fix xpath() to return namespace definitions (fixes the issue with nested or repeated xpath()) Peter, you've done some XML stuff before; could you have a look at this too? - Heikki -- 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] Last Commitfest patches waiting review
On Fri, Oct 3, 2014 at 6:14 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Thanks to everyone's who's reviewed a patch so far. One last crunch, and we'll be through. We have 7 patches left in Needs Review state: pg_receivexlog: addition of --create/--drop to create/drop repslots Waiting for Magnus. Amit promised to take a look if Magnus continues to be busy. Andres did, not Amit. And thanks Andres! :) And sorry aobut that one - no way I'll get to it until at the earliest next week but almost certainly not until the week after :( So I really appreciate the pickup. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Last Commitfest patches waiting review
On 2014-10-03 19:14:14 +0300, Heikki Linnakangas wrote: Thanks to everyone's who's reviewed a patch so far. One last crunch, and we'll be through. We have 7 patches left in Needs Review state: pg_receivexlog: addition of --create/--drop to create/drop repslots Waiting for Magnus. Amit promised to take a look if Magnus continues to be busy. I've committed half of it, and will commit the second half soon. I guess you meant Andres instead of Amit? Escaping from blocked send() by pg_terminate_backend(). Andres posted a patch series using a completely different approach. I guess this should be just marked as returned with feedback, if we're going to pursue the different approach instead. Sounds fair to me. I'll comment with details on the corresponding thread. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Last Commitfest patches waiting review
On 09/27/2014 05:12 PM, Tom Lane wrote: Heikki Linnakangas hlinnakan...@vmware.com writes: [ unreviewed patches ] Grouping Sets There has been a lot of discussion, but no decisions. See http://www.postgresql.org/message-id/87vbodhtvb@news-spur.riddles.org.uk. Would a committer be interested to take responsibility of this? If not, this will just linger indefinitely. I should and will take this, but not in this commitfest: I've been just horribly busy lately with both work and personal issues. If we can punt it to the next fest, I will promise to work on it then. Ok, thanks! I moved this to next commitfest and put your name on it as reviewer. INNER JOIN removals The latest patch is significantly different from what was originally submitted for the commitfest, so I wouldn't feel bad just bumping this to the next one. I'll do that unless someone picks this up soon. Tom: I know you're busy with the more urgent jsonb patch.. Do you think you would find the time to review this anytime soon? Anyone else? Same deal here. I marked this as Returned with feedback, as the trigger-queue issue seems like a show-stopper. Selectivity estimation for inet operators I think there's a bug in the estimation of semi-joins, see http://www.postgresql.org/message-id/5423dc8d.3080...@vmware.com. But I think we could split this patch into two, and commit the non-join selectivity estimator first, as that seems OK. If no-one else steps up to the plate, I can do that.. And I'd like to look this one over too ... Ok, punted this to next commitfest too. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Last Commitfest patches waiting review
We are down to 13 patch in Needs Review state. Many of these are stragglers that no-one's really even looked at yet. If we want to finish the commitfest, we'll soon have to decide what to do to patches that no-one cares enough about to review them. pg_receivexlog: addition of --create/--drop to create/drop repslots --- Magnus has promised to review this, but hasn't had the time for weeks. Does anyone else want to pick this up? I'm OK to wait for Magnus to handle this - I expect it to be a quick review and commit - but we should not hold up the commitfest for this. Grouping Sets --- There has been a lot of discussion, but no decisions. See http://www.postgresql.org/message-id/87vbodhtvb@news-spur.riddles.org.uk. Would a committer be interested to take responsibility of this? If not, this will just linger indefinitely. Sort support for text with strxfrm() poor man's keys --- Peter: Are you waiting for Robert to review this? Robert, could you review the latest patch, please? Peter: Could you try to get rid of the extra SortSupport object that Robert didn't like? (http://www.postgresql.org/message-id/ca+tgmobde+ydfnhts0gwpt54-er8bpt3vx8rpshd+98ctdo...@mail.gmail.com). I think it would speed up the process if you did that, instead of waiting for Robert to find the time. Compression of Full Page Writes --- This has been a ridiculously long thread, with diversions into different compression and CRC algorithms. Given that, the latest patch is surprisingly small. I don't think this is going to get any better with further discussion, and the patch looks OK at a quick glance, so I've now marked this as Ready for Committer. hash join - dynamic bucket count --- Kevin: Could you review the latest patch, please? INNER JOIN removals --- The latest patch is significantly different from what was originally submitted for the commitfest, so I wouldn't feel bad just bumping this to the next one. I'll do that unless someone picks this up soon. Tom: I know you're busy with the more urgent jsonb patch.. Do you think you would find the time to review this anytime soon? Anyone else? event triggers: more DROP info --- Adam: Are you planning to do more review on this? Alvaro: do you feel comfortable proceeding with the review you got so far? Selectivity estimation for inet operators --- I think there's a bug in the estimation of semi-joins, see http://www.postgresql.org/message-id/5423dc8d.3080...@vmware.com. But I think we could split this patch into two, and commit the non-join selectivity estimator first, as that seems OK. If no-one else steps up to the plate, I can do that.. add latency limit to pgbench throttling (--rate) --- Rukh: Are you planning to review the latest patch version? Escaping from blocked send() by pg_terminate_backend(). --- I've had a look, but I'd like to have a second opinion on this. Fix local_preload_libraries to work as expected. --- Peter: Could you move this forward, please? pg_dump refactor to remove global variables --- Peter: Can you review the latest patch, please? Fix xpath() to return namespace definitions (fixes the issue with nested or repeated xpath()) --- No-one's volunteered to review this. I don't understand XML enough to review this, and Abhijit who was earlier signed up as reviewer said the same thing. Peter: Could you take a look at this too? You've dabbled into XML stuff before.. - Heikki -- 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] Last Commitfest patches waiting review
On 2014-09-27 11:18:26 +0300, Heikki Linnakangas wrote: pg_receivexlog: addition of --create/--drop to create/drop repslots --- Magnus has promised to review this, but hasn't had the time for weeks. Does anyone else want to pick this up? I'm OK to wait for Magnus to handle this - I expect it to be a quick review and commit - but we should not hold up the commitfest for this. I'll take that one, sometime next week, if Magnus hasn't gotten to it by then. Compression of Full Page Writes --- This has been a ridiculously long thread, with diversions into different compression and CRC algorithms. Given that, the latest patch is surprisingly small. I don't think this is going to get any better with further discussion, and the patch looks OK at a quick glance, so I've now marked this as Ready for Committer. Did you like the patch? On a quick pass I wasn't very enthusiastic about it in its current state. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Last Commitfest patches waiting review
On 09/27/2014 11:31 AM, Andres Freund wrote: On 2014-09-27 11:18:26 +0300, Heikki Linnakangas wrote: pg_receivexlog: addition of --create/--drop to create/drop repslots --- Magnus has promised to review this, but hasn't had the time for weeks. Does anyone else want to pick this up? I'm OK to wait for Magnus to handle this - I expect it to be a quick review and commit - but we should not hold up the commitfest for this. I'll take that one, sometime next week, if Magnus hasn't gotten to it by then. Thanks! Compression of Full Page Writes --- This has been a ridiculously long thread, with diversions into different compression and CRC algorithms. Given that, the latest patch is surprisingly small. I don't think this is going to get any better with further discussion, and the patch looks OK at a quick glance, so I've now marked this as Ready for Committer. Did you like the patch? On a quick pass I wasn't very enthusiastic about it in its current state. Now that I look at it closer, there's some ugly things there like putting the xl_compress field to XLogRecord. I guess I should post to the thread with more detailed comments. - Heikki -- 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] Last Commitfest patches waiting review
Heikki Linnakangas hlinnakan...@vmware.com writes: [ unreviewed patches ] Grouping Sets There has been a lot of discussion, but no decisions. See http://www.postgresql.org/message-id/87vbodhtvb@news-spur.riddles.org.uk. Would a committer be interested to take responsibility of this? If not, this will just linger indefinitely. I should and will take this, but not in this commitfest: I've been just horribly busy lately with both work and personal issues. If we can punt it to the next fest, I will promise to work on it then. INNER JOIN removals The latest patch is significantly different from what was originally submitted for the commitfest, so I wouldn't feel bad just bumping this to the next one. I'll do that unless someone picks this up soon. Tom: I know you're busy with the more urgent jsonb patch.. Do you think you would find the time to review this anytime soon? Anyone else? Same deal here. Selectivity estimation for inet operators I think there's a bug in the estimation of semi-joins, see http://www.postgresql.org/message-id/5423dc8d.3080...@vmware.com. But I think we could split this patch into two, and commit the non-join selectivity estimator first, as that seems OK. If no-one else steps up to the plate, I can do that.. And I'd like to look this one over too ... Escaping from blocked send() by pg_terminate_backend(). I've had a look, but I'd like to have a second opinion on this. I concur with your opinion that this is scary as heck. We need multiple pairs of eyeballs if we're going to do anything in this area. In the long run though, I think pushing functionality into signal handlers is exactly backwards; we ought to be trying to go the other way. 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] Last Commitfest patches waiting review
On 2014-09-27 10:12:03 -0400, Tom Lane wrote: Escaping from blocked send() by pg_terminate_backend(). I've had a look, but I'd like to have a second opinion on this. I concur with your opinion that this is scary as heck. We need multiple pairs of eyeballs if we're going to do anything in this area. In the long run though, I think pushing functionality into signal handlers is exactly backwards; we ought to be trying to go the other way. I very much agree with that concern. The interactions are just complicated. I'm now refreshing my work to do all waiting in client communication via latches. While far from trivial, I'm pretty sure that's the better course in the long run. I guess I'll post it in the other thread. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Last Commitfest patches waiting review
On Sat, Sep 27, 2014 at 1:18 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Sort support for text with strxfrm() poor man's keys --- Peter: Are you waiting for Robert to review this? Robert, could you review the latest patch, please? Peter: Could you try to get rid of the extra SortSupport object that Robert didn't like? (http://www.postgresql.org/message-id/ca+tgmobde+ydfnhts0gwpt54-er8bpt3vx8rpshd+98ctdo...@mail.gmail.com). I think it would speed up the process if you did that, instead of waiting for Robert to find the time. I am not waiting on Robert to spend the time, FWIW. The question that resolving if we should not have an extra sortsupport object is blocking on is the need to have a consistent sorttuple.datum1 representation for the benefit of having comparetup_heap() know that it's either always abbreviated keys or always pointers to text. My view is that it's not worth going back to fix up datum1 to always be a pointer to text when we abort abbreviation - I think we should just forget about datum1 on the rare occasion that happens (due to the costs involved, as well as the complexity implied). I think that it will be necessary for me to rigorously prove that view, as with the memcmp() == 0 thing. So I'm looking at that. -- 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] Last Commitfest patches waiting review
On 9/27/14, 4:18 AM, Heikki Linnakangas wrote: add latency limit to pgbench throttling (--rate) --- Rukh: Are you planning to review the latest patch version? Rukh is free to jump onto this ASAP, but if it's still there next week I already had my eye on picking that up and taking it out for a spin. I already updated my pgbench-tools set to incorporate the rate limit for 9.4, and I use that part all the time now. I think I understand Fabien's entire game plan for this now, and I want the remainder of the set to land in 9.5. -- Greg Smith greg.sm...@crunchydatasolutions.com Chief PostgreSQL Evangelist - http://crunchydatasolutions.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
