Re: [HACKERS] Reviewing freeze map code

2016-06-14 Thread Thomas Munro
On Tue, Jun 14, 2016 at 4:02 AM, Robert Haas wrote: > On Sat, Jun 11, 2016 at 5:00 PM, Robert Haas wrote: >>> How about changing the return tuple of heap_prepare_freeze_tuple to >>> a bitmap? Two flags: "Freeze [not] done" and "[No] more freezing

Re: [HACKERS] Reviewing freeze map code

2016-06-13 Thread Andres Freund
On June 13, 2016 11:02:42 AM CDT, Robert Haas wrote: >(Official status update: I'm hoping that senior hackers will carefully >review these patches for defects. If they do not, I plan to commit >the patches anyway neither less than 48 nor more than 60 hours from >now

Re: [HACKERS] Reviewing freeze map code

2016-06-13 Thread Robert Haas
On Sat, Jun 11, 2016 at 5:00 PM, Robert Haas wrote: >> How about changing the return tuple of heap_prepare_freeze_tuple to >> a bitmap? Two flags: "Freeze [not] done" and "[No] more freezing >> needed" > > Yes, I think something like that sounds about right. Here's a

Re: [HACKERS] Reviewing freeze map code

2016-06-12 Thread Amit Kapila
On Sat, Jun 11, 2016 at 1:24 AM, Robert Haas wrote: > > 3. vacuumlazy.c includes this code: > > if (heap_prepare_freeze_tuple(tuple.t_data, FreezeLimit, > MultiXactCutoff, [nfrozen])) >

Re: [HACKERS] Reviewing freeze map code

2016-06-11 Thread Robert Haas
On Fri, Jun 10, 2016 at 4:55 PM, Alvaro Herrera wrote: > Robert Haas wrote: >> 3. vacuumlazy.c includes this code: >> >> if (heap_prepare_freeze_tuple(tuple.t_data, FreezeLimit, >> MultiXactCutoff, [nfrozen])) >>

Re: [HACKERS] Reviewing freeze map code

2016-06-10 Thread Alvaro Herrera
Robert Haas wrote: > 3. vacuumlazy.c includes this code: > > if (heap_prepare_freeze_tuple(tuple.t_data, FreezeLimit, > MultiXactCutoff, [nfrozen])) > frozen[nfrozen++].offset = offnum; > else if

Re: [HACKERS] Reviewing freeze map code

2016-06-10 Thread Robert Haas
On Fri, Jun 10, 2016 at 1:59 PM, Andres Freund wrote: >> Master, but with an existing standby. So it could be related to >> hot_standby_feedback or such. > > I just managed to trigger it again. > > > #1 0x7fa1a73778da in __GI_abort () at abort.c:89 > #2

Re: [HACKERS] Reviewing freeze map code

2016-06-10 Thread Andres Freund
On 2016-06-09 23:39:24 -0700, Andres Freund wrote: > On 2016-06-10 11:58:26 +0530, Amit Kapila wrote: > > I have tried in multiple ways by running pgbench with read-write tests, but > > could not see any such behaviour. > > It took over an hour of pgbench on a fast laptop till I saw it. > > > >

Re: [HACKERS] Reviewing freeze map code

2016-06-10 Thread Amit Kapila
On Fri, Jun 10, 2016 at 12:09 PM, Andres Freund wrote: > > On 2016-06-10 11:58:26 +0530, Amit Kapila wrote: > > > > While looking at code in this area, I observed that during replay of > > records (heap_xlog_delete), we first clear the vm, then update the page. > > So we don't

Re: [HACKERS] Reviewing freeze map code

2016-06-10 Thread Masahiko Sawada
On Fri, Jun 10, 2016 at 1:11 AM, Robert Haas wrote: > On Thu, Jun 9, 2016 at 5:48 AM, Amit Kapila wrote: >> Attached patch implements the above 2 functions. I have addressed the >> comments by Sawada San and you in latest patch and updated the

Re: [HACKERS] Reviewing freeze map code

2016-06-10 Thread Amit Kapila
On Thu, Jun 9, 2016 at 9:41 PM, Robert Haas wrote: > > > > 2. The all-visible checks seemed to me to be incorrect and incomplete. > I made the check match the logic in lazy_scan_heap. > Okay, I thought we just want to check for dead-tuples. If we want the logic similar to

Re: [HACKERS] Reviewing freeze map code

2016-06-10 Thread Andres Freund
On 2016-06-10 11:58:26 +0530, Amit Kapila wrote: > I have tried in multiple ways by running pgbench with read-write tests, but > could not see any such behaviour. It took over an hour of pgbench on a fast laptop till I saw it. > I have tried by even crashing and > restarting the server and then

Re: [HACKERS] Reviewing freeze map code

2016-06-10 Thread Amit Kapila
On Fri, Jun 10, 2016 at 8:27 AM, Andres Freund wrote: > > > On June 9, 2016 7:46:06 PM PDT, Amit Kapila > wrote: > >On Fri, Jun 10, 2016 at 8:08 AM, Andres Freund > >wrote: > > > >> On 2016-06-09 19:33:52 -0700, Andres Freund

Re: [HACKERS] Reviewing freeze map code

2016-06-09 Thread Andres Freund
On June 9, 2016 7:46:06 PM PDT, Amit Kapila wrote: >On Fri, Jun 10, 2016 at 8:08 AM, Andres Freund >wrote: > >> On 2016-06-09 19:33:52 -0700, Andres Freund wrote: >> > I played with it for a while, and besides >> > finding intentionally caused

Re: [HACKERS] Reviewing freeze map code

2016-06-09 Thread Amit Kapila
On Fri, Jun 10, 2016 at 8:08 AM, Andres Freund wrote: > On 2016-06-09 19:33:52 -0700, Andres Freund wrote: > > I played with it for a while, and besides > > finding intentionally caused corruption, it didn't flag anything > > (besides crashing on a standby, as in 2)). > >

Re: [HACKERS] Reviewing freeze map code

2016-06-09 Thread Andres Freund
On 2016-06-09 19:33:52 -0700, Andres Freund wrote: > I played with it for a while, and besides > finding intentionally caused corruption, it didn't flag anything > (besides crashing on a standby, as in 2)). Ugh. Just sends after I sent that email: oid|t_ctid

Re: [HACKERS] Reviewing freeze map code

2016-06-09 Thread Andres Freund
Hi, I found two, relatively minor, issues. 1) I think we should perform a relkind check in collect_corrupt_items(). Atm we'll "gladly" run against an index. If we actually entered the main portion of the loop in collect_corrupt_items(), that could end up corrupting the table (via

Re: [HACKERS] Reviewing freeze map code

2016-06-09 Thread Andres Freund
Hi Robert, Amit, thanks for working on this. On 2016-06-09 12:11:15 -0400, Robert Haas wrote: > 4. The tests as written were not safe under concurrency; they could > return spurious results if the page changed between the time you > checked the visibility map and the time you actually examined

Re: [HACKERS] Reviewing freeze map code

2016-06-09 Thread Robert Haas
On Thu, Jun 9, 2016 at 5:48 AM, Amit Kapila wrote: > Attached patch implements the above 2 functions. I have addressed the > comments by Sawada San and you in latest patch and updated the documentation > as well. I made a number of changes to this patch. Here is the

Re: [HACKERS] Reviewing freeze map code

2016-06-09 Thread Amit Kapila
On Thu, Jun 9, 2016 at 8:48 AM, Amit Kapila wrote: > > On Wed, Jun 8, 2016 at 6:31 PM, Robert Haas wrote: > > > > > > Here's my proposal: > > > > 1. You already implemented a function to find non-frozen tuples on > > supposedly all-frozen pages.

Re: [HACKERS] Reviewing freeze map code

2016-06-08 Thread Amit Kapila
On Wed, Jun 8, 2016 at 6:31 PM, Robert Haas wrote: > > > Here's my proposal: > > 1. You already implemented a function to find non-frozen tuples on > supposedly all-frozen pages. Great. > > 2. Let's implement a second function to find dead tuples on supposedly >

Re: [HACKERS] Reviewing freeze map code

2016-06-08 Thread Amit Kapila
On Wed, Jun 8, 2016 at 6:31 PM, Robert Haas wrote: > > On Wed, Jun 8, 2016 at 4:01 AM, Amit Kapila wrote: > > If we want to address both page level and tuple level inconsistencies, I > > could see below possibility. > > > > 1. An API that returns

Re: [HACKERS] Reviewing freeze map code

2016-06-08 Thread Robert Haas
On Wed, Jun 8, 2016 at 4:01 AM, Amit Kapila wrote: > If we want to address both page level and tuple level inconsistencies, I > could see below possibility. > > 1. An API that returns setof records containing a block that have > inconsistent vm bit, a block where visible

Re: [HACKERS] Reviewing freeze map code

2016-06-08 Thread Amit Kapila
On Wed, Jun 8, 2016 at 11:39 AM, Andres Freund wrote: > > On 2016-06-08 10:04:56 +0530, Amit Kapila wrote: > > On Tue, Jun 7, 2016 at 11:01 PM, Andres Freund wrote:> > > > I think if we go with the pg_check_visibility approach, we should also > > > copy

Re: [HACKERS] Reviewing freeze map code

2016-06-08 Thread Masahiko Sawada
On Wed, Jun 8, 2016 at 12:19 PM, Amit Kapila wrote: > On Tue, Jun 7, 2016 at 10:10 PM, Masahiko Sawada > wrote: >> >> Thank you for implementing the patch. >> >> I've not test it deeply but here are some comments. >> This check tool only checks if

Re: [HACKERS] Reviewing freeze map code

2016-06-08 Thread Andres Freund
On 2016-06-08 10:04:56 +0530, Amit Kapila wrote: > On Tue, Jun 7, 2016 at 11:01 PM, Andres Freund wrote:> > > I think if we go with the pg_check_visibility approach, we should also > > copy the other consistency checks from vacuumlazy.c, given they can't > > easily be

Re: [HACKERS] Reviewing freeze map code

2016-06-07 Thread Amit Kapila
On Tue, Jun 7, 2016 at 11:01 PM, Andres Freund wrote:> > I think if we go with the pg_check_visibility approach, we should also > copy the other consistency checks from vacuumlazy.c, given they can't > easily be triggered. Are you referring to checks that are done in

Re: [HACKERS] Reviewing freeze map code

2016-06-07 Thread Amit Kapila
On Wed, Jun 8, 2016 at 8:37 AM, Robert Haas wrote: > > On Tue, Jun 7, 2016 at 10:19 AM, Amit Kapila wrote: > > I have implemented the above function in attached patch. Currently, it > > returns SETOF tupleids, but if we want some variant of same,

Re: [HACKERS] Reviewing freeze map code

2016-06-07 Thread Amit Kapila
On Tue, Jun 7, 2016 at 10:10 PM, Masahiko Sawada wrote: > > Thank you for implementing the patch. > > I've not test it deeply but here are some comments. > This check tool only checks if the frozen page has live-unfrozen tuple. > That is, it doesn't care in case where the

Re: [HACKERS] Reviewing freeze map code

2016-06-07 Thread Robert Haas
On Tue, Jun 7, 2016 at 10:19 AM, Amit Kapila wrote: > I have implemented the above function in attached patch. Currently, it > returns SETOF tupleids, but if we want some variant of same, that should > also be possible. I think we'd want to bump the pg_visibility

Re: [HACKERS] Reviewing freeze map code

2016-06-07 Thread Jim Nasby
On 6/6/16 3:57 PM, Peter Geoghegan wrote: On Mon, Jun 6, 2016 at 11:35 AM, Andres Freund wrote: We need a read-only utility which checks that the system is in a correct and valid state. There are a few of those which have been built for different pieces, I believe, and we

Re: [HACKERS] Reviewing freeze map code

2016-06-07 Thread Andres Freund
On 2016-06-07 19:49:59 +0530, Amit Kapila wrote: > On Tue, Jun 7, 2016 at 2:52 AM, Robert Haas wrote: > > > > On Mon, Jun 6, 2016 at 5:06 PM, Andres Freund wrote: > > > > > > > I'd also be ok with adding & documenting (beta release notes) > > > CREATE

Re: [HACKERS] Reviewing freeze map code

2016-06-07 Thread Masahiko Sawada
On Tue, Jun 7, 2016 at 11:19 PM, Amit Kapila wrote: > On Tue, Jun 7, 2016 at 2:52 AM, Robert Haas wrote: >> >> On Mon, Jun 6, 2016 at 5:06 PM, Andres Freund wrote: >> >> >> > I'd also be ok with adding & documenting (beta

Re: [HACKERS] Reviewing freeze map code

2016-06-07 Thread Amit Kapila
On Tue, Jun 7, 2016 at 2:52 AM, Robert Haas wrote: > > On Mon, Jun 6, 2016 at 5:06 PM, Andres Freund wrote: > > > > I'd also be ok with adding & documenting (beta release notes) > > CREATE EXTENSION pg_visibility; > > SELECT relname FROM pg_class WHERE

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Andres Freund
Hi, On 2016-06-06 17:22:38 -0400, Robert Haas wrote: > > I'd also be ok with adding & documenting (beta release notes) > > CREATE EXTENSION pg_visibility; > > SELECT relname FROM pg_class WHERE relkind IN ('r', 'm') AND NOT > > pg_check_visibility(oid); > > or something olong those lines. > >

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Andres Freund
Hi, On 2016-06-06 17:00:19 -0400, Robert Haas wrote: > 1. I think it is pretty misleading to say that those checks aren't > reachable any more. It's not like we freeze every page when we mark > it all-visible. True. What I mean is that you can't force the checks (and some that I think should

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Mon, Jun 6, 2016 at 5:06 PM, Andres Freund wrote: > On 2016-06-06 16:41:19 -0400, Robert Haas wrote: >> I really don't understand how you can not weigh in on the original >> thread leading up to my mid-March commits and say "hey, this needs a >> better testing tool", and

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Andres Freund
On 2016-06-06 16:41:19 -0400, Robert Haas wrote: > I really don't understand how you can not weigh in on the original > thread leading up to my mid-March commits and say "hey, this needs a > better testing tool", and then when you finally get around to > reviewing it in May, I'm supposed to drop

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Mon, Jun 6, 2016 at 4:27 PM, Andres Freund wrote: > On 2016-06-06 16:18:19 -0400, Stephen Frost wrote: >> That could be as simple as a query with the right things installed, or >> it might be an independent tool, but not having any way to check isn't >> good. That said,

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Peter Geoghegan
On Mon, Jun 6, 2016 at 11:35 AM, Andres Freund wrote: >> We need a read-only utility which checks that the system is in a correct >> and valid state. There are a few of those which have been built for >> different pieces, I believe, and we really should have one for the >>

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Mon, Jun 6, 2016 at 2:35 PM, Andres Freund wrote: >> > Why would they have to write the complex query? Wouldn't they just >> > need to run that we wrote for them? > > Then write that query. Verify that that query performs halfway > reasonably fast. Document that it should

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Mon, Jun 6, 2016 at 4:06 PM, Andres Freund wrote: >> I do appreciate you reviewing this code, very much, and genuinely, and >> it would be great if more people wanted to review it. > >> But this kind of reads like you think that I'm being a jerk, which I'm >> trying pretty

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Andres Freund
On 2016-06-06 16:18:19 -0400, Stephen Frost wrote: > That could be as simple as a query with the right things installed, or > it might be an independent tool, but not having any way to check isn't > good. That said, trying to make VACUUM do that doesn't make sense to me > either. The point is

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Stephen Frost
Andres, all, * Andres Freund (and...@anarazel.de) wrote: > On 2016-06-06 15:16:10 -0400, Robert Haas wrote: > > On Mon, Jun 6, 2016 at 2:35 PM, Andres Freund wrote: > > and like you have the right to tell assign me arbitrary work, which I > > think you don't. > > It's not

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Andres Freund
Hi, On 2016-06-06 15:16:10 -0400, Robert Haas wrote: > On Mon, Jun 6, 2016 at 2:35 PM, Andres Freund wrote: > >> > Why would they have to write the complex query? Wouldn't they just > >> > need to run that we wrote for them? > > > > Then write that query. Verify that that

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Mon, Jun 6, 2016 at 2:35 PM, Andres Freund wrote: >> > Why would they have to write the complex query? Wouldn't they just >> > need to run that we wrote for them? > > Then write that query. Verify that that query performs halfway > reasonably fast. Document that it should

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Andres Freund
On 2016-06-06 14:24:14 -0400, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: > > On Mon, Jun 6, 2016 at 11:44 AM, Andres Freund wrote: > > >> In terms of diagnostic tools, you can get the VM bits and > > >> page-level bits using the pg_visibility

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: > On Mon, Jun 6, 2016 at 11:44 AM, Andres Freund wrote: > >> In terms of diagnostic tools, you can get the VM bits and > >> page-level bits using the pg_visibility extension; I wrote it > >> precisely because of concerns like the

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Mon, Jun 6, 2016 at 11:44 AM, Andres Freund wrote: >> In terms of diagnostic tools, you can get the VM bits and >> page-level bits using the pg_visibility extension; I wrote it >> precisely because of concerns like the ones you raise here. If you >> want to cross-check the

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Andres Freund
On 2016-06-06 05:34:32 -0400, Robert Haas wrote: > On Mon, Jun 6, 2016 at 5:11 AM, Michael Paquier > wrote: > >> Attached is a sample patch that controls full page vacuum by new GUC > >> parameter. > > > > Don't we want a reloption for that? Just wondering... > > Why?

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Tom Lane
Robert Haas writes: > On Mon, Jun 6, 2016 at 11:28 AM, Andres Freund wrote: >> Except that we right now don't have any realistic way to figure out >> whether this new feature actually does the right thing. > I just don't see how running VACUUM on the

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Mon, Jun 6, 2016 at 11:28 AM, Andres Freund wrote: > On 2016-06-06 05:34:32 -0400, Robert Haas wrote: >> On Mon, Jun 6, 2016 at 5:11 AM, Michael Paquier >> wrote: >> >> Attached is a sample patch that controls full page vacuum by new GUC >> >>

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Andres Freund
On 2016-06-06 11:37:25 -0400, Robert Haas wrote: > On Mon, Jun 6, 2016 at 11:28 AM, Andres Freund wrote: > > Except that we right now don't have any realistic way to figure out > > whether this new feature actually does the right thing. Which makes > > testing this

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Tom Lane
Robert Haas writes: > I'm intuitively sympathetic to the idea that we should have an option > for this, but I can't figure out in what case we'd actually tell > anyone to use it. It would be useful for the kinds of bugs listed > above to have VACUUM (rebuild_vm) to blow

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Mon, Jun 6, 2016 at 10:18 AM, Alvaro Herrera wrote: > Robert Haas wrote: >> My gut feeling on this is to either do nothing or add a VACUUM option >> (not a GUC, not a reloption) called even_frozen_pages, default false. >> What is your opinion? > > +1 for that approach

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Fri, Jun 3, 2016 at 11:41 PM, Robert Haas wrote: > (Status update for Noah: I expect Masahiko Sawada will respond > quickly, but if not I'll give some kind of update by Monday COB > anyhow.) I believe this open item is now closed, unless Andres has more comments or

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Alvaro Herrera
Robert Haas wrote: > My gut feeling on this is to either do nothing or add a VACUUM option > (not a GUC, not a reloption) called even_frozen_pages, default false. > What is your opinion? +1 for that approach -- I thought that was already agreed weeks ago and the only question was what to name

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Tom Lane
Robert Haas writes: > On Mon, Jun 6, 2016 at 9:53 AM, Tom Lane wrote: >> Taking out GUCs is not >> easier than taking out statement parameters; you risk breaking >> applications either way. > Agreed, but that doesn't really answer the question of which

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Mon, Jun 6, 2016 at 9:53 AM, Tom Lane wrote: > Masahiko Sawada writes: >> On Sat, Jun 4, 2016 at 1:46 PM, Masahiko Sawada >> wrote: >>> So other idea is to have GUC parameter, vacuum_even_frozen_page for example. >>> If this

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Mon, Jun 6, 2016 at 7:46 AM, Robert Haas wrote: > On Sat, Jun 4, 2016 at 12:18 AM, Masahiko Sawada > wrote: >> Attached updated patch. > > The error-checking enhancements here look good to me, except that you > forgot to initialize

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Tom Lane
Masahiko Sawada writes: > On Sat, Jun 4, 2016 at 1:46 PM, Masahiko Sawada wrote: >> So other idea is to have GUC parameter, vacuum_even_frozen_page for example. >> If this parameter is set true (false by default), we do vacuum whole >> table forcibly

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Sat, Jun 4, 2016 at 12:18 AM, Masahiko Sawada wrote: > Attached updated patch. The error-checking enhancements here look good to me, except that you forgot to initialize totalBytesRead. I've committed those changes with a fix for that problem and will look at the rest

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Masahiko Sawada
On Mon, Jun 6, 2016 at 6:34 PM, Robert Haas wrote: > On Mon, Jun 6, 2016 at 5:11 AM, Michael Paquier > wrote: >>> Attached is a sample patch that controls full page vacuum by new GUC >>> parameter. >> >> Don't we want a reloption for that? Just

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Robert Haas
On Mon, Jun 6, 2016 at 5:11 AM, Michael Paquier wrote: >> Attached is a sample patch that controls full page vacuum by new GUC >> parameter. > > Don't we want a reloption for that? Just wondering... Why? Just for consistency? I think the bigger question here is

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Michael Paquier
On Mon, Jun 6, 2016 at 5:44 PM, Masahiko Sawada wrote: > On Sat, Jun 4, 2016 at 1:46 PM, Masahiko Sawada wrote: >> On Sat, Jun 4, 2016 at 12:59 AM, Robert Haas wrote: >>> On Fri, Jun 3, 2016 at 11:21 AM, Masahiko Sawada

Re: [HACKERS] Reviewing freeze map code

2016-06-06 Thread Masahiko Sawada
On Sat, Jun 4, 2016 at 1:46 PM, Masahiko Sawada wrote: > On Sat, Jun 4, 2016 at 12:59 AM, Robert Haas wrote: >> On Fri, Jun 3, 2016 at 11:21 AM, Masahiko Sawada >> wrote: Can you submit that part as a separate patch? >>>

Re: [HACKERS] Reviewing freeze map code

2016-06-03 Thread Masahiko Sawada
On Sat, Jun 4, 2016 at 12:59 AM, Robert Haas wrote: > On Fri, Jun 3, 2016 at 11:21 AM, Masahiko Sawada > wrote: >>> Can you submit that part as a separate patch? >> >> Attached. > > Thanks, committed. > I'm address the review comment of 7087166

Re: [HACKERS] Reviewing freeze map code

2016-06-03 Thread Masahiko Sawada
On Sat, Jun 4, 2016 at 12:41 PM, Robert Haas wrote: > On Fri, Jun 3, 2016 at 10:25 PM, Masahiko Sawada > wrote: + charnew_vmbuf[BLCKSZ]; + char *new_cur = new_vmbuf; +

Re: [HACKERS] Reviewing freeze map code

2016-06-03 Thread Robert Haas
On Fri, Jun 3, 2016 at 10:25 PM, Masahiko Sawada wrote: >>> + charnew_vmbuf[BLCKSZ]; >>> + char *new_cur = new_vmbuf; >>> + boolempty = true; >>> + bool

Re: [HACKERS] Reviewing freeze map code

2016-06-03 Thread Masahiko Sawada
On Sat, May 7, 2016 at 5:42 AM, Robert Haas wrote: > On Thu, May 5, 2016 at 2:20 PM, Andres Freund wrote: >> On 2016-05-02 14:48:18 -0700, Andres Freund wrote: >>> 7087166 pg_upgrade: Convert old visibility map format to new format. >> >> +const char *

Re: [HACKERS] Reviewing freeze map code

2016-06-03 Thread Robert Haas
On Fri, Jun 3, 2016 at 11:21 AM, Masahiko Sawada wrote: >> Can you submit that part as a separate patch? > > Attached. Thanks, committed. >>> I'm address the review comment of 7087166 commit, and will post the patch. >> >> When? > > On Saturday. Great. Will that address

Re: [HACKERS] Reviewing freeze map code

2016-06-03 Thread Masahiko Sawada
On Sat, Jun 4, 2016 at 12:08 AM, Robert Haas wrote: > On Fri, Jun 3, 2016 at 10:49 AM, Masahiko Sawada > wrote: >> That patch also incorporates the following review comment. >> We can push at least this fix. > > Can you submit that part as a

Re: [HACKERS] Reviewing freeze map code

2016-06-03 Thread Robert Haas
On Fri, Jun 3, 2016 at 10:49 AM, Masahiko Sawada wrote: > That patch also incorporates the following review comment. > We can push at least this fix. Can you submit that part as a separate patch? > I'm address the review comment of 7087166 commit, and will post the patch.

Re: [HACKERS] Reviewing freeze map code

2016-06-03 Thread Masahiko Sawada
On Fri, Jun 3, 2016 at 11:03 PM, Robert Haas wrote: > On Thu, Jun 2, 2016 at 11:24 AM, Masahiko Sawada > wrote: >> Attached patch optimises skipping pages logic so that blkno can jump to >> next_unskippable_block directly while counting the number

Re: [HACKERS] Reviewing freeze map code

2016-06-03 Thread Robert Haas
On Wed, Jun 1, 2016 at 3:50 AM, Masahiko Sawada wrote: > Attached patch fixes only above comments, other are being addressed now. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing

Re: [HACKERS] Reviewing freeze map code

2016-06-03 Thread Robert Haas
On Thu, Jun 2, 2016 at 11:24 AM, Masahiko Sawada wrote: > Attached patch optimises skipping pages logic so that blkno can jump to > next_unskippable_block directly while counting the number of all_visible > and all_frozen pages. So we can avoid double checking visibility

Re: [HACKERS] Reviewing freeze map code

2016-06-02 Thread Masahiko Sawada
On Sat, May 7, 2016 at 5:40 AM, Robert Haas wrote: > On Wed, May 4, 2016 at 8:08 PM, Andres Freund wrote: >> On 2016-05-02 14:48:18 -0700, Andres Freund wrote: >>> 77a1d1e Department of second thoughts: remove PD_ALL_FROZEN. >> >> Nothing to say here.

Re: [HACKERS] Reviewing freeze map code

2016-06-01 Thread Masahiko Sawada
On Sat, May 7, 2016 at 5:34 AM, Robert Haas wrote: > On Mon, May 2, 2016 at 8:25 PM, Andres Freund wrote: >> + * heap_tuple_needs_eventual_freeze >> + * >> + * Check to see whether any of the XID fields of a tuple (xmin, xmax, xvac) >> + * will

Re: [HACKERS] Reviewing freeze map code

2016-05-31 Thread Robert Haas
On Sun, May 29, 2016 at 1:44 AM, Noah Misch wrote: > On Fri, May 06, 2016 at 04:42:48PM -0400, Robert Haas wrote: >> On Thu, May 5, 2016 at 2:20 PM, Andres Freund wrote: >> > On 2016-05-02 14:48:18 -0700, Andres Freund wrote: >> > +

Re: [HACKERS] Reviewing freeze map code

2016-05-30 Thread Michael Paquier
On Tue, May 31, 2016 at 4:40 AM, Jeff Janes wrote: > On Wed, May 18, 2016 at 3:57 PM, Alvaro Herrera > wrote: >> Andres Freund wrote: >> >>> >>> If we had a checking module for all this it'd possibly be sufficient, >>> but we don't. >> >> Here's an

Re: [HACKERS] Reviewing freeze map code

2016-05-30 Thread Jeff Janes
On Wed, May 18, 2016 at 3:57 PM, Alvaro Herrera wrote: > Andres Freund wrote: > >> >> If we had a checking module for all this it'd possibly be sufficient, >> but we don't. > > Here's an idea. We need core-blessed extensions (src/extensions/, you > know I've proposed

Re: [HACKERS] Reviewing freeze map code

2016-05-29 Thread Masahiko Sawada
On Sun, May 29, 2016 at 2:44 PM, Noah Misch wrote: > On Fri, May 06, 2016 at 04:42:48PM -0400, Robert Haas wrote: >> On Thu, May 5, 2016 at 2:20 PM, Andres Freund wrote: >> > On 2016-05-02 14:48:18 -0700, Andres Freund wrote: >> > +

Re: [HACKERS] Reviewing freeze map code

2016-05-28 Thread Noah Misch
On Fri, May 06, 2016 at 04:42:48PM -0400, Robert Haas wrote: > On Thu, May 5, 2016 at 2:20 PM, Andres Freund wrote: > > On 2016-05-02 14:48:18 -0700, Andres Freund wrote: > > + charnew_vmbuf[BLCKSZ]; > > + char

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Peter Geoghegan
On Wed, May 18, 2016 at 3:57 PM, Alvaro Herrera wrote: > Since we were considering a new VACUUM option, surely this is serious > enough to warrant more than just contrib. I would like to see us consider the long-term best place for amcheck's functionality at the same

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Alvaro Herrera
Andres Freund wrote: > > (AFAIK, "select count(*) from table" would offer a similar amount of > > sanity checking as a full-table VACUUM scan does, so it's not like > > we've removed functionality with no near-term replacement.) > > I don't think that'd do anything comparable to >

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Andres Freund
On 2016-05-18 18:42:16 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-05-18 18:25:39 -0400, Tom Lane wrote: > >> Yes, I've been wondering that too. VACUUM is not meant as a corruption > >> checker, and should not be made into one, so what is the point of this > >>

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Tom Lane
Andres Freund writes: > On 2016-05-18 18:25:39 -0400, Tom Lane wrote: >> Yes, I've been wondering that too. VACUUM is not meant as a corruption >> checker, and should not be made into one, so what is the point of this >> flag exactly? > Well, so far a VACUUM FREEZE (or just

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Andres Freund
On 2016-05-18 18:25:39 -0400, Tom Lane wrote: > Josh berkus writes: > > Maybe this is the wrong perspective. I mean, is there a reason we even > > need this option, other than a lack of any other way to do a full table > > scan to check for corruption, etc.? If we're only

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Tom Lane
Josh berkus writes: > Maybe this is the wrong perspective. I mean, is there a reason we even > need this option, other than a lack of any other way to do a full table > scan to check for corruption, etc.? If we're only doing this for > integrity checking, then maybe it's

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Josh berkus
On 05/18/2016 03:51 PM, Peter Geoghegan wrote: > On Wed, May 18, 2016 at 8:52 AM, Jeff Janes wrote: >> How about going with something that says more about why we are doing >> it, rather than trying to describe in one or two words what it is >> doing? >> >> VACUUM (FORENSIC)

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Peter Geoghegan
On Wed, May 18, 2016 at 8:52 AM, Jeff Janes wrote: > How about going with something that says more about why we are doing > it, rather than trying to describe in one or two words what it is > doing? > > VACUUM (FORENSIC) > > VACUUM (DEBUG) > > VACUUM (LINT) +1 -- Peter

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Jeff Janes
On Wed, May 18, 2016 at 7:09 AM, Joe Conway wrote: > On 05/18/2016 09:55 AM, Victor Yegorov wrote: >> 2016-05-18 16:45 GMT+03:00 Robert Haas > >: >> >> No, that's what the existing FREEZE option does. This new option

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Joe Conway
On 05/18/2016 09:55 AM, Victor Yegorov wrote: > 2016-05-18 16:45 GMT+03:00 Robert Haas >: > > No, that's what the existing FREEZE option does. This new option is > about unnecessarily vacuuming pages that don't need it. The >

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Victor Yegorov
2016-05-18 16:45 GMT+03:00 Robert Haas : > No, that's what the existing FREEZE option does. This new option is > about unnecessarily vacuuming pages that don't need it. The > expectation is that vacuuming all-frozen pages will be a no-op. > VACUUM (INCLUDING ALL) ? --

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Robert Haas
On Wed, May 18, 2016 at 9:42 AM, Joshua D. Drake wrote: >> It's not a bad thought, but I do think it might be a bit confusing. >> My main priority for this new option is that people aren't tempted to >> use it very often, and I think a name like "even_frozen_pages" is more

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Joshua D. Drake
On 05/18/2016 05:51 AM, Robert Haas wrote: On Wed, May 18, 2016 at 8:41 AM, David Steele wrote: I think we should give this a name that hints more strongly at this being an exceptional thing, like vacuum (even_frozen_pages). How about just FROZEN? Perhaps it's too

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Robert Haas
On Wed, May 18, 2016 at 8:41 AM, David Steele wrote: >> I think we should give this a name that hints more strongly at this >> being an exceptional thing, like vacuum (even_frozen_pages). > > How about just FROZEN? Perhaps it's too confusing to have that and FREEZE, > but I

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread David Steele
On 5/18/16 6:37 AM, Robert Haas wrote: On Tue, May 17, 2016 at 5:47 PM, Gavin Flower wrote: On 18/05/16 09:34, Vik Fearing wrote: On 17/05/16 21:32, Alvaro Herrera wrote: Is SCAN_ALL really the best we can do here? The business of having an underscore in an

Re: [HACKERS] Reviewing freeze map code

2016-05-18 Thread Robert Haas
On Tue, May 17, 2016 at 5:47 PM, Gavin Flower wrote: > On 18/05/16 09:34, Vik Fearing wrote: >> On 17/05/16 21:32, Alvaro Herrera wrote: >>> >>> Is SCAN_ALL really the best we can do here? The business of having an >>> underscore in an option name has no precedent

Re: [HACKERS] Reviewing freeze map code

2016-05-17 Thread Gavin Flower
On 18/05/16 09:34, Vik Fearing wrote: On 17/05/16 21:32, Alvaro Herrera wrote: Is SCAN_ALL really the best we can do here? The business of having an underscore in an option name has no precedent (other than CURRENT_DATABASE and the like). ALTER DATABASE has options for ALLOW_CONNECTIONS,

<    1   2   3   >