Re: increasing the default WAL segment size

On Wed, Sep 6, 2017 at 8:24 PM, Beena Emerson <memissemerson(at)gmail(dot)com> wrote:
> Hello,
>
> On Wed, Sep 6, 2017 at 7:37 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>> Hi,
>
>> Unresolved:
>> - this needs some new performance tests, the number of added instructions
>> isn't trivial. Don't think there's anything, but ...
>
> I will give out the results soon.

Performance tests:

The following results are the median of 3 runs for 32 and 56
clients/threads on a pgbench database of 300 scale with each run of
900s (15 min) for various wal segment sizes and shared buffers 8GB.

Following is the % difference of the performance of patched code
(initdb wal-segsize) over the original code (configure wal-segsize)

size | c_32 | c_56
------------+-------------------+--------------
4MB | 1.11 | -0.18
8MB | 0 | -1.56
16MB | 0.79 | 0.23
64MB | 0.89 | 0.28
1024MB | -1.29 | -0.09

Median values:
size | 32_original | 32_patched |
56_original | 56_patched
------------+-----------------------+-----------------------+-----------------------+--------------------
4MB | 83999.06142 | 84933.78919 |
95667.13483 | 95492.21335
8MB | 84949.08195 | 84947.35953 |
96584.13828 | 95081.37257
16MB | 84155.40321 | 84820.98328 |
95697.53134 | 95914.98814
64MB | 84496.2927 | 85245.70758 |
96307.95222 | 96581.1183
1024 | 76230.39323 | 75247.03348 |
92495.18142 | 92410.59222

We can conclude that there is not much difference.

[1] Previous performance results:
https://www.postgresql.org/message-id/CAOG9ApESjqYm2VQWxNrZAKySzVo-vDw2JWhDqYQStzD%2BgwRUiA%40mail.gmail.com

>
>> - read through it again, check long lines
> I have broken the long lines where necessary and applied pgindent as well.
>
>> - pg_standby's RetrieveWALSegSize() does too much for it's name. It
>> seems quite weird that a function named that way has the section below
>> "/* check if clean up is necessary */"
>
> we set 2 cleanup related variables once WalSegSize is set, namely
> need_cleanup and exclusiveCleanupFileName. Does
> SetWALSegSizeAndCleanupValues look good?
>
>> - the way you redid the ReadControlFile() invocation doesn't quite seem
>> right. Consider what happens if XLOGbuffers isn't -1 - then we
>> wouldn't read the control file, but you unconditionally copy it in
>> XLOGShmemInit(). I think we instead should introduce something like
>> XLOGPreShmemInit() that reads the control file unless in bootstrap
>> mode. Then get rid of the second ReadControlFile() already present.
>
> I did not think it was necessary to create a new function, I have
> simply added the check and
> function call within the XLOGShmemInit().
>
>> - In pg_resetwal.c:ReadControlFile() we ignore the file contents if
>> there's an invalid segment size, but accept the contents as guessed if
>> there's a crc failure - that seems a bit weird?
>
> I have changed the behaviour to treat it as guessed and also modified
> the error message.
>
>>- verify EXEC_BACKEND does the right thing

Ashutosh Sharma has verified this and confirms that there are no issues.

>> - not this commit/patch, but XLogReadDetermineTimeline() could really
>> use some simplifying of repetitive expressions
>
> I will check this.
>
>> - XLOGShmemInit shouldn't memcpy to temp_cfile and such, why not just
>> save previous pointer in a local variable?
> done.
>
>> - could you fill in the Reviewed-By: line in the commit message?
> I have added the names in alphabetical order.
>
> Kindly check the attached v2 patch.

PFA the rebased patch.

--

Beena Emerson

EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company