Re: [HACKERS] crash-safe visibility map, take five

2011-06-24 Thread Jeff Davis
On Thu, 2011-06-23 at 22:02 -0400, Robert Haas wrote: 1. INSERT to a new page, marking it with LSN X 2. WAL flushed to LSN Y (Y X) 2. Some persistent snapshot (that doesn't see the INSERT) is released, and generates WAL recording that fact with LSN Z (Z Y) 3. Lazy VACUUM marks the

Re: [HACKERS] crash-safe visibility map, take five

2011-06-24 Thread Robert Haas
On Fri, Jun 24, 2011 at 1:43 PM, Jeff Davis pg...@j-davis.com wrote: And anything that is WAL-logged must obey the WAL-before-data rule. We have a system already that ensures that when synchronous_commit=off, CLOG pages can't be flushed before the corresponding WAL record makes it to disk.

Re: [HACKERS] crash-safe visibility map, take five

2011-06-23 Thread Robert Haas
On Wed, Jun 22, 2011 at 10:40 PM, Jeff Davis pg...@j-davis.com wrote: 1. Torn pages -- not a problem because it's a single bit and idempotent. Right. 2. PD_ALL_VISIBLE bit makes it to disk before a WAL record representing an action that makes the page all-visible. Depending on what action

Re: [HACKERS] crash-safe visibility map, take five

2011-06-23 Thread Jeff Davis
On Thu, 2011-06-23 at 18:18 -0400, Robert Haas wrote: Lazy VACUUM is the only thing that makes a page all visible. I don't understand the part about snapshots. Lazy VACUUM is the only thing that _marks_ a page with PD_ALL_VISIBLE. After an INSERT to a new page, and after all snapshots are

Re: [HACKERS] crash-safe visibility map, take five

2011-06-23 Thread Robert Haas
On Thu, Jun 23, 2011 at 6:40 PM, Jeff Davis pg...@j-davis.com wrote: On Thu, 2011-06-23 at 18:18 -0400, Robert Haas wrote: Lazy VACUUM is the only thing that makes a page all visible.  I don't understand the part about snapshots. Lazy VACUUM is the only thing that _marks_ a page with

Re: [HACKERS] crash-safe visibility map, take five

2011-06-22 Thread Jeff Davis
On Thu, 2011-06-16 at 23:17 -0400, Noah Misch wrote: 2. In the words of a comment added by the patch: * The critical integrity requirement here is that we must never end up with * a situation where the visibility map bit is set, and the page-level * PD_ALL_VISIBLE bit is clear. If that

Re: [HACKERS] crash-safe visibility map, take five

2011-06-22 Thread Robert Haas
On Wed, Jun 22, 2011 at 8:53 PM, Jeff Davis pg...@j-davis.com wrote: On Thu, 2011-06-16 at 23:17 -0400, Noah Misch wrote: 2. In the words of a comment added by the patch:  * The critical integrity requirement here is that we must never end up with  * a situation where the visibility map bit is

Re: [HACKERS] crash-safe visibility map, take five

2011-06-22 Thread Jeff Davis
On Wed, 2011-06-22 at 21:53 -0400, Robert Haas wrote: On Wed, Jun 22, 2011 at 8:53 PM, Jeff Davis pg...@j-davis.com wrote: On Thu, 2011-06-16 at 23:17 -0400, Noah Misch wrote: 2. In the words of a comment added by the patch: * The critical integrity requirement here is that we must never

Re: [HACKERS] crash-safe visibility map, take five

2011-06-18 Thread Noah Misch
On Fri, Jun 17, 2011 at 01:21:17PM -0400, Robert Haas wrote: On Thu, Jun 16, 2011 at 11:17 PM, Noah Misch n...@leadboat.com wrote: I suggest revisiting the suggestion in http://archives.postgresql.org/message-id/27743.1291135...@sss.pgh.pa.us and including a latestRemovedXid in

Re: [HACKERS] crash-safe visibility map, take five

2011-06-18 Thread Robert Haas
On Sat, Jun 18, 2011 at 9:37 AM, Noah Misch n...@leadboat.com wrote: Indeed.  If we conflicted on the xmin of the most-recent all-visible tuple when setting the bit, all-visible on the standby would become a superset of all-visible on the master, and we could cease to ignore the bits.  This is

Re: [HACKERS] crash-safe visibility map, take five

2011-06-18 Thread Noah Misch
On Sat, Jun 18, 2011 at 10:04:52PM -0400, Robert Haas wrote: On Sat, Jun 18, 2011 at 9:37 AM, Noah Misch n...@leadboat.com wrote: Indeed. ?If we conflicted on the xmin of the most-recent all-visible tuple when setting the bit, all-visible on the standby would become a superset of

Re: [HACKERS] crash-safe visibility map, take five

2011-06-18 Thread Robert Haas
On Sat, Jun 18, 2011 at 10:41 PM, Noah Misch n...@leadboat.com wrote: This version of the patch has some bogus hunks in int.c, int8.c, and heap.c reverting a few of your other recent commits. Woops, good catch. Corrected version attached, in case anyone else wants to take a look at it. Looks

Re: [HACKERS] crash-safe visibility map, take five

2011-06-17 Thread Robert Haas
On Thu, Jun 16, 2011 at 11:17 PM, Noah Misch n...@leadboat.com wrote: I took a look at this patch. No kidding! Thanks for the very detailed review. +1 for Buffer over Buffer * when the value isn't mutated for the caller. I changed this. I suggest revisiting the suggestion in

Re: [HACKERS] crash-safe visibility map, take five

2011-06-16 Thread Noah Misch
Robert, I took a look at this patch. To summarize its purpose as I understand it, it does two independent but synergistic things: 1. Move the INSERT/UPDATE/DELETE clearing of visibility map bits from after the main operation to before it. This has no affect on crash recovery. It closes a race

Re: [HACKERS] crash-safe visibility map, take five

2011-05-23 Thread Robert Haas
On Wed, May 11, 2011 at 8:46 AM, Pavan Deolasee pavan.deola...@gmail.com wrote: Why do the set and clear functions need pass-by-reference (Buffer *) argument ? I don't see them modifying the argument at all. This patch adds the clear function, but the existing set function also suffers from

Re: [HACKERS] crash-safe visibility map, take five

2011-05-11 Thread Pavan Deolasee
On Tue, May 10, 2011 at 7:38 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, May 10, 2011 at 9:45 AM, Merlin Moncure mmonc...@gmail.com wrote: I see: here's a comment that was throwing me off: + /* +* If we didn't get the lock and it turns out we need it, we'll have to

Re: [HACKERS] crash-safe visibility map, take five

2011-05-10 Thread Robert Haas
On Mon, May 9, 2011 at 10:25 PM, Merlin Moncure mmonc...@gmail.com wrote: On Fri, May 6, 2011 at 5:47 PM, Robert Haas robertmh...@gmail.com wrote: On Wed, Mar 30, 2011 at 8:52 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Another question: To address the problem in

Re: [HACKERS] crash-safe visibility map, take five

2011-05-10 Thread Merlin Moncure
On Tue, May 10, 2011 at 7:48 AM, Robert Haas robertmh...@gmail.com wrote: I thought I'd explained it fairly thoroughly in the comments, but evidently not.  Suggestions for improvement are welcome. ok. that clears it up nicely. Here goes in more detail: Every time we insert, update, or delete

Re: [HACKERS] crash-safe visibility map, take five

2011-05-10 Thread Robert Haas
On Tue, May 10, 2011 at 9:45 AM, Merlin Moncure mmonc...@gmail.com wrote: I see: here's a comment that was throwing me off: +       /* +        * If we didn't get the lock and it turns out we need it, we'll have to +        * unlock and re-lock, to avoid holding the buffer lock across an

Re: [HACKERS] crash-safe visibility map, take five

2011-05-10 Thread Jesper Krogh
On 2011-05-10 14:48, Robert Haas wrote: We could avoid all of this complexity - and the possibility of pinning the visibility map page needlessly - by locking the heap buffer first and then pinning the visibility map page if the heap page is all-visible. However, that would involve holding the

Re: [HACKERS] crash-safe visibility map, take five

2011-05-10 Thread Robert Haas
On Tue, May 10, 2011 at 11:34 AM, Jesper Krogh jes...@krogh.cc wrote: On 2011-05-10 14:48, Robert Haas wrote: We could avoid all of this complexity - and the possibility of pinning the visibility map page needlessly - by locking the heap buffer first and then pinning the visibility map page

Re: [HACKERS] crash-safe visibility map, take five

2011-05-10 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Tue, May 10, 2011 at 11:34 AM, Jesper Krogh jes...@krogh.cc wrote: Or what is the downside for keeping it across IO? Will it block other processes trying to read it? Heikki might be in a better position to comment on that than I am, since he wrote

Re: [HACKERS] crash-safe visibility map, take five

2011-05-10 Thread Robert Haas
On Tue, May 10, 2011 at 1:00 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, May 10, 2011 at 11:34 AM, Jesper Krogh jes...@krogh.cc wrote: Or what is the downside for keeping it across IO? Will it block other processes trying to read it? Heikki might

Re: [HACKERS] crash-safe visibility map, take five

2011-05-09 Thread Rob Wultsch
On Fri, May 6, 2011 at 2:47 PM, Robert Haas robertmh...@gmail.com wrote: Comments? At my day job there is saying: Silence is consent. I am surprised there has not been more discussion of this change, considering the magnitude of the possibilities it unlocks. -- Rob Wultsch wult...@gmail.com

Re: [HACKERS] crash-safe visibility map, take five

2011-05-09 Thread Merlin Moncure
On Fri, May 6, 2011 at 5:47 PM, Robert Haas robertmh...@gmail.com wrote: On Wed, Mar 30, 2011 at 8:52 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Another question: To address the problem in http://archives.postgresql.org/pgsql-hackers/2010-02/msg02097.php , should we

[HACKERS] crash-safe visibility map, take five

2011-05-06 Thread Robert Haas
On Wed, Mar 30, 2011 at 8:52 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Another question: To address the problem in http://archives.postgresql.org/pgsql-hackers/2010-02/msg02097.php , should we just clear the vm before the log of insert/update/delete? This may reduce