Re: [HACKERS] Reviewing freeze map code

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, CONNECTION_LIMIT, and IS_TEMPLATE. >

Re: [HACKERS] Reviewing freeze map code

On Tue, May 17, 2016 at 4:34 PM, Joshua D. Drake wrote: > On 05/17/2016 12:32 PM, Alvaro Herrera wrote: > >> Syntaxes are; >> VACUUM (SCAN_ALL) table_name; >> VACUUM (SCAN_ALL); -- for all tables on database >> >> Is SCAN_ALL really the best we can do here? The

Re: [HACKERS] Reviewing freeze map code

On 05/17/2016 12:32 PM, Alvaro Herrera wrote: Syntaxes are; VACUUM (SCAN_ALL) table_name; VACUUM (SCAN_ALL); -- for all tables on database 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

Re: [HACKERS] Reviewing freeze map code

On Tue, May 17, 2016 at 3:32 PM, Alvaro Herrera wrote: > Masahiko Sawada wrote: >> On Mon, May 16, 2016 at 10:49 AM, Robert Haas wrote: > >> > We should support scan_all only with the new-style options syntax for >> > VACUUM; that is, vacuum

Re: [HACKERS] Reviewing freeze map code

Masahiko Sawada wrote: > On Mon, May 16, 2016 at 10:49 AM, Robert Haas wrote: > > We should support scan_all only with the new-style options syntax for > > VACUUM; that is, vacuum (scan_all) rename. That doesn't require > > making scan_all a keyword, which is good: this

Re: [HACKERS] Reviewing freeze map code

On Mon, May 16, 2016 at 10:49 AM, Robert Haas wrote: > On Tue, May 10, 2016 at 10:40 PM, Masahiko Sawada > wrote: >> Or second way I came up with is having tool to remove particular _vm >> file safely, which is executed via SQL or client tool like

Re: [HACKERS] Reviewing freeze map code

On Tue, May 10, 2016 at 10:40 PM, Masahiko Sawada wrote: > Or second way I came up with is having tool to remove particular _vm > file safely, which is executed via SQL or client tool like > pg_resetxlog. > > Attached updated VACUUM SCAN_ALL patch. > Please find it. We

Re: [HACKERS] Reviewing freeze map code

On 5/10/16 11:42 PM, Jim Nasby wrote: On 5/6/16 4:55 PM, Peter Geoghegan wrote: On Fri, May 6, 2016 at 2:49 PM, Andres Freund wrote: Jeff Janes has done astounding work in these matters. (I don't think we credit him enough for that.) +many. Agreed. I'm a huge fan of

Re: [HACKERS] Reviewing freeze map code

On 5/6/16 4:08 PM, Joshua D. Drake wrote: VACUUM THEWHOLEDAMNTHING +100 (hahahaha) You know what? Why not? Seriously? We aren't product. This is supposed to be a bit fun. Let's have some fun with it? It would be so easy to turn that into a positive advocacy opportunity. Honestly, for an

Re: [HACKERS] Reviewing freeze map code

On 5/6/16 4:55 PM, Peter Geoghegan wrote: On Fri, May 6, 2016 at 2:49 PM, Andres Freund wrote: Jeff Janes has done astounding work in these matters. (I don't think we credit him enough for that.) +many. Agreed. I'm a huge fan of what Jeff has been able to do in this

Re: [HACKERS] Reviewing freeze map code

On 5/6/16 4:20 PM, Andres Freund wrote: On 2016-05-06 14:15:47 -0700, Josh berkus wrote: For the serious testing, does anyone have a good technique for creating loads which would stress-test vacuum freezing? It's hard for me to come up with anything which wouldn't be very time-and-resource

Re: [HACKERS] Reviewing freeze map code

On Tue, May 10, 2016 at 11:30 PM, Robert Haas wrote: > On Mon, May 9, 2016 at 7:40 PM, Ants Aasma wrote: >> On Mon, May 9, 2016 at 10:53 PM, Robert Haas wrote: >>> On Sun, May 8, 2016 at 10:42 PM, Masahiko Sawada

Re: [HACKERS] Reviewing freeze map code

On Mon, May 9, 2016 at 7:40 PM, Ants Aasma wrote: > On Mon, May 9, 2016 at 10:53 PM, Robert Haas wrote: >> On Sun, May 8, 2016 at 10:42 PM, Masahiko Sawada >> wrote: >>> Attached draft patch adds SCANALL option to VACUUM in

Re: [HACKERS] Reviewing freeze map code

On Mon, May 9, 2016 at 10:53 PM, Robert Haas wrote: > On Sun, May 8, 2016 at 10:42 PM, Masahiko Sawada > wrote: >> Attached draft patch adds SCANALL option to VACUUM in order to scan >> all pages forcibly while ignoring visibility map information.

Re: [HACKERS] Reviewing freeze map code

On Sun, May 8, 2016 at 10:42 PM, Masahiko Sawada wrote: > Attached draft patch adds SCANALL option to VACUUM in order to scan > all pages forcibly while ignoring visibility map information. > The option name is SCANALL for now but we could change it after got consensus. If

Re: [HACKERS] Reviewing freeze map code

On Tue, May 3, 2016 at 6:48 AM, Andres Freund wrote: > fd31cd2 Don't vacuum all-frozen pages. - appendStringInfo(, _("pages: %u removed, %u remain, %u skipped due to pins\n"), + appendStringInfo(, _("pages: %u removed, %u remain, %u

Re: [HACKERS] Reviewing freeze map code

On Sun, May 8, 2016 at 3:18 PM, Masahiko Sawada wrote: > On Sat, May 7, 2016 at 11:08 PM, Masahiko Sawada > wrote: >> On Sat, May 7, 2016 at 6:00 AM, Joshua D. Drake >> wrote: >>> On 05/06/2016 01:58 PM, Stephen Frost

Re: [HACKERS] Reviewing freeze map code

On Sat, May 7, 2016 at 11:08 PM, Masahiko Sawada wrote: > On Sat, May 7, 2016 at 6:00 AM, Joshua D. Drake > wrote: >> On 05/06/2016 01:58 PM, Stephen Frost wrote: >>> >>> * Joshua D. Drake (j...@commandprompt.com) wrote: Yeah I thought

Re: [HACKERS] Reviewing freeze map code

On Sat, May 7, 2016 at 6:00 AM, Joshua D. Drake wrote: > On 05/06/2016 01:58 PM, Stephen Frost wrote: >> >> * Joshua D. Drake (j...@commandprompt.com) wrote: >>> >>> Yeah I thought about that, it is the word "FORCE" that bothers me. >>> When you use FORCE there is an

Re: [HACKERS] Reviewing freeze map code

Alvaro Herrera wrote: > We touched this question in connection with multixact freezing and > wraparound. Testers seem to want to be given a script that they can > install and run, then go for a beer and get back to a bunch of errors to > report. Here I spent some time trying to explain what to

Re: [HACKERS] Reviewing freeze map code

On 2016-05-07 10:00:27 +1200, Thomas Munro wrote: > On Sat, May 7, 2016 at 8:34 AM, Robert Haas wrote: > >> Did somebody verify the new contents are correct? > > > > I admit that I didn't. It seemed like an unlikely place for a goof, > > but I guess we should verify. > >

Re: [HACKERS] Reviewing freeze map code

On Sat, May 7, 2016 at 8:34 AM, Robert Haas wrote: > On Mon, May 2, 2016 at 8:25 PM, Andres Freund wrote: >> +static const uint8 number_of_ones_for_visible[256] = { >> ... >> +}; >> +static const uint8 number_of_ones_for_frozen[256] = { >> ... >> }; >>

Re: [HACKERS] Reviewing freeze map code

On Fri, May 6, 2016 at 2:49 PM, Andres Freund wrote: >> Jeff Janes has done astounding work in these matters. (I don't think >> we credit him enough for that.) > > +many. Agreed. I'm a huge fan of what Jeff has been able to do in this area. I often say so. It would be even

Re: [HACKERS] Reviewing freeze map code

On 2016-05-06 18:31:03 -0300, Alvaro Herrera wrote: > I don't know what happens when the freeze_table_age threshold is > reached. We scan all non-frozen pages, whereas we earlier had to scan all pages. That's really both the significant benefit, and the danger. Because if we screw up the

Re: [HACKERS] Reviewing freeze map code

On 05/06/2016 02:48 PM, Andres Freund wrote: On 2016-05-06 14:39:57 -0700, Joshua D. Drake wrote: Yes, this is true but with a proper testing framework, I don't need a 15 minute break. I need 1 hour to configure, the rest just "happens" and reports back. That only works if somebody writes

Re: [HACKERS] Reviewing freeze map code

On 2016-05-06 18:36:52 -0300, Alvaro Herrera wrote: > Andres Freund wrote: > > > On 2016-05-06 14:17:13 -0700, Joshua D. Drake wrote: > > > How do I test? > > > > > > Is there a script I can run? > > > > Unfortunately there's few interesting things to test with pre-made > > scripts. There's no

Re: [HACKERS] Reviewing freeze map code

On 2016-05-06 14:39:57 -0700, Joshua D. Drake wrote: > > > What are we looking for exactly? > > > > Data corruption, efficiency problems. > > > > I am really not trying to be difficult here but Data Corruption is an easy > one... what is the metric we accept as an efficiency problem? That's

Re: [HACKERS] Reviewing freeze map code

On 05/06/2016 02:29 PM, Andres Freund wrote: Hi, On 2016-05-06 14:17:13 -0700, Joshua D. Drake wrote: How do I test? Is there a script I can run? Unfortunately there's few interesting things to test with pre-made scripts. There's no relevant OS dependency here, so each already existing

Re: [HACKERS] Reviewing freeze map code

Andres Freund wrote: > On 2016-05-06 14:17:13 -0700, Joshua D. Drake wrote: > > How do I test? > > > > Is there a script I can run? > > Unfortunately there's few interesting things to test with pre-made > scripts. There's no relevant OS dependency here, so each already > existing test doesn't

Re: [HACKERS] Reviewing freeze map code

Joshua D. Drake wrote: > On 05/06/2016 01:40 PM, 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

Hi, On 2016-05-06 14:17:13 -0700, Joshua D. Drake wrote: > How do I test? > > Is there a script I can run? Unfortunately there's few interesting things to test with pre-made scripts. There's no relevant OS dependency here, so each already existing test doesn't really lead to significantly

Re: [HACKERS] Reviewing freeze map code

On 2016-05-06 14:15:47 -0700, Josh berkus wrote: > For the serious testing, does anyone have a good technique for creating > loads which would stress-test vacuum freezing? It's hard for me to come > up with anything which wouldn't be very time-and-resource intensive > (like running at 10,000 TPS

Re: [HACKERS] Reviewing freeze map code

On 05/06/2016 02:08 PM, Andres Freund wrote: VACUUM THEWHOLEDAMNTHING I know that would never fly but damn if that wouldn't be an awesome keyword for VACUUM. It bothers me more than it probably should: Nobdy tests, reviews, whatever a complex patch with significant data-loss potential. But

Re: [HACKERS] Reviewing freeze map code

On 05/06/2016 02:12 PM, Andres Freund wrote: > On 2016-05-06 14:10:04 -0700, Josh berkus wrote: >> On 05/06/2016 02:08 PM, Andres Freund wrote: >> >>> It bothers me more than it probably should: Nobdy tests, reviews, >>> whatever a complex patch with significant data-loss potential. But as >>>

Re: [HACKERS] Reviewing freeze map code

On 2016-05-06 14:10:04 -0700, Josh berkus wrote: > On 05/06/2016 02:08 PM, Andres Freund wrote: > > > It bothers me more than it probably should: Nobdy tests, reviews, > > whatever a complex patch with significant data-loss potential. But as > > soon somebody dares to mention an option name... >

Re: [HACKERS] Reviewing freeze map code

On 05/06/2016 02:08 PM, Andres Freund wrote: > It bothers me more than it probably should: Nobdy tests, reviews, > whatever a complex patch with significant data-loss potential. But as > soon somebody dares to mention an option name... Definitely more than it should, because it's gonna happen

Re: [HACKERS] Reviewing freeze map code

On 05/06/2016 02:03 PM, Stephen Frost wrote: VACUUM THEWHOLEDAMNTHING +100 (hahahaha) You know what? Why not? Seriously? We aren't product. This is supposed to be a bit fun. Let's have some fun with it? It would be so easy to turn that into a positive advocacy opportunity. JD --

Re: [HACKERS] Reviewing freeze map code

On 2016-05-06 14:03:11 -0700, Joshua D. Drake wrote: > On 05/06/2016 02:01 PM, Josh berkus wrote: > > On 05/06/2016 01:58 PM, Andres Freund wrote: > > > On 2016-05-06 13:54:09 -0700, Joshua D. Drake wrote: > > > > On 05/06/2016 01:50 PM, Andres Freund wrote: > > > > > > > There already is FREEZE

Re: [HACKERS] Reviewing freeze map code

* Josh berkus (j...@agliodbs.com) wrote: > On 05/06/2016 01:58 PM, Andres Freund wrote: > > On 2016-05-06 13:54:09 -0700, Joshua D. Drake wrote: > >> On 05/06/2016 01:50 PM, Andres Freund wrote: > > >>> There already is FREEZE - meaning something different - so I doubt it. > >> > >> Yeah I

Re: [HACKERS] Reviewing freeze map code

On 05/06/2016 02:01 PM, Josh berkus wrote: On 05/06/2016 01:58 PM, Andres Freund wrote: On 2016-05-06 13:54:09 -0700, Joshua D. Drake wrote: On 05/06/2016 01:50 PM, Andres Freund wrote: There already is FREEZE - meaning something different - so I doubt it. Yeah I thought about that, it is

Re: [HACKERS] Reviewing freeze map code

On 05/06/2016 01:58 PM, Andres Freund wrote: > On 2016-05-06 13:54:09 -0700, Joshua D. Drake wrote: >> On 05/06/2016 01:50 PM, Andres Freund wrote: >>> There already is FREEZE - meaning something different - so I doubt it. >> >> Yeah I thought about that, it is the word "FORCE" that bothers me.

Re: [HACKERS] Reviewing freeze map code

On 05/06/2016 01:58 PM, Stephen Frost wrote: * Joshua D. Drake (j...@commandprompt.com) wrote: Yeah I thought about that, it is the word "FORCE" that bothers me. When you use FORCE there is an assumption that no matter what, it plows through (think rm -f). So if we don't use FROZEN, that's cool

Re: [HACKERS] Reviewing freeze map code

* Joshua D. Drake (j...@commandprompt.com) wrote: > Yeah I thought about that, it is the word "FORCE" that bothers me. > When you use FORCE there is an assumption that no matter what, it > plows through (think rm -f). So if we don't use FROZEN, that's cool > but FORCE doesn't work either. Isn't

Re: [HACKERS] Reviewing freeze map code

On 2016-05-06 13:54:09 -0700, Joshua D. Drake wrote: > On 05/06/2016 01:50 PM, Andres Freund wrote: > > > > > Let's add VACUUM (FORCE) or something like that. > > > > Yes, that makes sense. > > > > > > > This is actually inverted. Vacuum by default should vacuum the entire > > > relation > >

Re: [HACKERS] Reviewing freeze map code

On 05/06/2016 01:50 PM, Andres Freund wrote: Let's add VACUUM (FORCE) or something like that. Yes, that makes sense. This is actually inverted. Vacuum by default should vacuum the entire relation What? Why on earth would that be a good idea? Not to speak of hte fact that that's not been

Re: [HACKERS] Reviewing freeze map code

On 2016-05-06 13:48:09 -0700, Joshua D. Drake wrote: > On 05/06/2016 01:40 PM, 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

Re: [HACKERS] Reviewing freeze map code

On 05/06/2016 01:40 PM, 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. fd31cd2 Don't vacuum all-frozen pages.

Re: [HACKERS] Reviewing freeze map code

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 * > +rewriteVisibilityMap(const char *fromfile, const char *tofile, bool force) >

Re: [HACKERS] Reviewing freeze map code

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. > >> fd31cd2 Don't vacuum all-frozen pages. > > Hm. I do wonder if it's going to

Re: [HACKERS] Reviewing freeze map code

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 eventually require freezing. Similar to heap_tuple_needs_freeze, > + * but there's no

Re: [HACKERS] Reviewing freeze map code

Hi, On 2016-05-02 14:48:18 -0700, Andres Freund wrote: > 7087166 pg_upgrade: Convert old visibility map format to new format. +const char * +rewriteVisibilityMap(const char *fromfile, const char *tofile, bool force) ... + while ((bytesRead = read(src_fd, buffer, BLCKSZ)) == BLCKSZ) +

Re: [HACKERS] Reviewing freeze map code

On 2016-05-02 14:48:18 -0700, Andres Freund wrote: > 77a1d1e Department of second thoughts: remove PD_ALL_FROZEN. Nothing to say here. > fd31cd2 Don't vacuum all-frozen pages. Hm. I do wonder if it's going to bite us that we don't have a way to actually force vacuuming of the whole table

Re: [HACKERS] Reviewing freeze map code

On Tue, May 3, 2016 at 6:48 AM, Andres Freund wrote: > Hi, > > The freeze map changes, besides being very important, seem to be one of > the patches with a high risk profile in 9.6. Robert had asked whether > I'd take a look. I thought it'd be a good idea to review that

Re: [HACKERS] Reviewing freeze map code

Hi, some of the review items here are mere matters of style/preference. Feel entirely free to discard them, but I thought if I'm going through the change anyway... On 2016-05-02 14:48:18 -0700, Andres Freund wrote: > a892234 Change the format of the VM fork to add a second bit per page. TL;DR:

[HACKERS] Reviewing freeze map code

Hi, The freeze map changes, besides being very important, seem to be one of the patches with a high risk profile in 9.6. Robert had asked whether I'd take a look. I thought it'd be a good idea to review that while running tests for

<    1   2   3