Thread

1. fix use of posix_fadvise in xlog.c

   Mark Wong <markwkm@gmail.com> — 2010-06-10T03:47:26Z

   Hi all,
   
   I wanted to propose a fix for to xlog.c regarding the use of
   posix_fadvise() for 9.1 (unless someone feels it's ok for 9.0).
   Currently posix_fadvise() is used right before a log file is closed so
   it's effectively not doing anything, when posix_fadvise is to be
   called.  This patch moves the posix_fadvise() call into 3 other
   locations within XLogFileInit() where a file handle is returned.  The
   first case is where an existing open file handle is returned.  The
   next case is when a file is to be zeroed out.  The third case is
   returning a file handle, which may be the file that was just zeroed
   out.
   
   Does this look ok?
   
   Regards,
   Mark
   

2. Re: fix use of posix_fadvise in xlog.c

   Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2010-06-10T06:25:08Z

   On 10/06/10 06:47, Mark Wong wrote:
   > I wanted to propose a fix for to xlog.c regarding the use of
   > posix_fadvise() for 9.1 (unless someone feels it's ok for 9.0).
   > Currently posix_fadvise() is used right before a log file is closed so
   > it's effectively not doing anything, when posix_fadvise is to be
   > called.  This patch moves the posix_fadvise() call into 3 other
   > locations within XLogFileInit() where a file handle is returned.  The
   > first case is where an existing open file handle is returned.  The
   > next case is when a file is to be zeroed out.  The third case is
   > returning a file handle, which may be the file that was just zeroed
   > out.
   
   I don't think POSIX_FADV_DONTNEED does what you think it does. It tells 
   the kernel that "you don't need to keep these pages in the cache 
   anymore, I won't be accessing them anymore". If you call it when you 
   open the file, before reading/writing, there is nothing in the cache and 
   the call will do nothing.
   
   -- 
      Heikki Linnakangas
      EnterpriseDB   http://www.enterprisedb.com
   
   

3. Re: fix use of posix_fadvise in xlog.c

   Mark Wong <markwkm@gmail.com> — 2010-06-10T15:17:19Z

   
   On Jun 9, 2010, at 11:25 PM, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com 
    > wrote:
   
   > On 10/06/10 06:47, Mark Wong wrote:
   >> I wanted to propose a fix for to xlog.c regarding the use of
   >> posix_fadvise() for 9.1 (unless someone feels it's ok for 9.0).
   >> Currently posix_fadvise() is used right before a log file is closed  
   >> so
   >> it's effectively not doing anything, when posix_fadvise is to be
   >> called.  This patch moves the posix_fadvise() call into 3 other
   >> locations within XLogFileInit() where a file handle is returned.  The
   >> first case is where an existing open file handle is returned.  The
   >> next case is when a file is to be zeroed out.  The third case is
   >> returning a file handle, which may be the file that was just zeroed
   >> out.
   >
   > I don't think POSIX_FADV_DONTNEED does what you think it does. It  
   > tells the kernel that "you don't need to keep these pages in the  
   > cache anymore, I won't be accessing them anymore". If you call it  
   > when you open the file, before reading/writing, there is nothing in  
   > the cache and the call will do nothing.
   
   Oops, my bad.  I think I was confused by the short description in the  
   man page.  I didn't read the longer descriptoon. :( Then would it be  
   worth making the this call after the file is zeroed out?
   
   Regards,
   Mark
   
   

4. Re: fix use of posix_fadvise in xlog.c

   Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2010-06-10T15:37:49Z

   On 10/06/10 18:17, Mark Wong wrote:
   > On Jun 9, 2010, at 11:25 PM, Heikki Linnakangas
   > <heikki.linnakangas@enterprisedb.com> wrote:
   >> I don't think POSIX_FADV_DONTNEED does what you think it does. It
   >> tells the kernel that "you don't need to keep these pages in the cache
   >> anymore, I won't be accessing them anymore". If you call it when you
   >> open the file, before reading/writing, there is nothing in the cache
   >> and the call will do nothing.
   >
   > Oops, my bad. I think I was confused by the short description in the man
   > page. I didn't read the longer descriptoon. :( Then would it be worth
   > making the this call after the file is zeroed out?
   
   Not sure. If you're churning through WAL files at a reasonable speed, 
   the zeroed-out file will soon be written to again. OTOH, we always write 
   whole pages, so maybe the OS is smart enough to not read the page back 
   to memory just to overwrite it.
   
   In a steady-state situation new WAL files are not created very often 
   because we recycle old ones, so it probably doesn't make much difference.
   
   -- 
      Heikki Linnakangas
      EnterpriseDB   http://www.enterprisedb.com
   
   

5. Re: fix use of posix_fadvise in xlog.c

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-06-10T15:44:02Z

   Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
   > In a steady-state situation new WAL files are not created very often 
   > because we recycle old ones, so it probably doesn't make much difference.
   
   Yeah.  We really don't worry too much about the performance of the
   new-WAL-file-creation code path because of this.
   
   			regards, tom lane
   
   

6. Re: fix use of posix_fadvise in xlog.c

   Greg Smith <greg@2ndquadrant.com> — 2010-06-10T21:34:29Z

   Tom Lane wrote:
   > Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
   >   
   >> In a steady-state situation new WAL files are not created very often 
   >> because we recycle old ones, so it probably doesn't make much difference.
   >>     
   >
   > Yeah.  We really don't worry too much about the performance of the
   > new-WAL-file-creation code path because of this.
   >   
   
   The only situation where the WAL zeroing path turns ugly is if you 
   launch a bunch of activity against a fresh server that doesn't have any 
   segments to recycle yet.  The last time we talked about improving that, 
   the best idea I thought came out was to be better about preallocating 
   segments than the code already is, rather than trying to speed up how 
   the kernel deals with the situation.  See the links for "Be more 
   aggressive about creating WAL files" at http://wiki.postgresql.org/wiki/Todo
   
   I'm also not very optimistic about adding more posix_fadvise calls 
   really helping just because the implementations of those are so 
   unpredictable across operating systems.  I'm sure that Mark could figure 
   out the right magic to speed up this specific case on Linux, but have my 
   doubts that work would translate very well to many other operating 
   systems.  Whereas a more generic preallocation improvement would help 
   everywhere.
   
   -- 
   Greg Smith  2ndQuadrant US  Baltimore, MD
   PostgreSQL Training, Services and Support
   greg@2ndQuadrant.com   www.2ndQuadrant.us