Re: increasing the default WAL segment size

On Wed, Sep 6, 2017 at 8:24 PM, Beena Emerson <memissemerson@gmail.com> wrote:
> Hello,
>
> On Wed, Sep 6, 2017 at 7:37 AM, Andres Freund <andres@anarazel.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