Re: patch to allow disable of WAL recycling

On 05.07.18 17:37, Jerry Jelinek wrote:
> Your patch describes this feature as a performance feature.  We would
> need to see more measurements about what this would do on other
> platforms and file systems than your particular one.  Also, we need to
> be careful with user options that trade off reliability for performance
> and describe them in much more detail.
>
> I don't think this change really impacts the reliability of PG, since PG
> doesn't actually preallocate all of the WAL files. I think PG will
> allocate WAL files as it runs, up to the  wal_keep_segments limit, at
> which point it would start recycling. If the filesystem fills up before
> that limit is reached, PG would have to handle the filesystem being full
> when attempting to allocate a new WAL file (as it would with my change
> if WAL recycling is disabled). Of course once all of the WAL files have
> finally been allocated, then PG won't need additional space on a non-COW
> filesystem. I'd be happy to add more details to the man page change
> describing this new option and the implications if the underlying
> filesystem fills up.

The point is, the WAL recycling has a purpose, perhaps several. If it
didn't have one, we wouldn't do it. So if we add an option to turn it
off to get performance gains, we have to do some homework.

> If the problem is specifically the file system caching behavior, then we
> could also consider using the dreaded posix_fadvise().
>
> I'm not sure that solves the problem for non-cached files, which is
> where we've observed the performance impact of recycling, where what
> should be a write intensive workload turns into a read-modify-write
> workload because we're now reading an old WAL file that is many hours,
> or even days, old and has thus fallen out of the memory-cached data for
> the filesystem. The disk reads still have to happen.

But they could happen ahead of time.

> Then again, I can understand that turning off WAL recycling is sensible
> on ZFS, since there is no point in preallocating space that will never
> be used.  But then we should also turn off all other preallocation of
> WAL files, including the creation of new (non-recycled) ones.
>
> I don't think we'd see any benefit from that (since the newly allocated
> file is certainly cached), and the change would be much more intrusive,
> so I opted for the trivial change in the patch I proposed.

The change would be more invasive, but I think it would ultimately make
the code more clear and maintainable and the user interfaces more
understandable in the long run. I think that would be better than a
slightly ad hoc knob that fixed one particular workload once upon a time.

But we're probably not there yet. We should start with a more detailed
performance analysis of the originally proposed patch.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services