On Sat, Dec 31, 2016 at 9:01 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> Agreed and changed accordingly.
>
>> 2. It seems that we have missed one unlock in case of absorbed
>> wakeups. You have initialised extraWaits with -1 and if there is one
>> extra wake up then extraWaits will become 0 (it means we have made one
>> extra call to PGSemaphoreLock and it's our responsibility to fix it as
>> the leader will Unlock only once). But it appear in such case we will
>> not make any call to PGSemaphoreUnlock.
>>
>
> Good catch! I have fixed it by initialising extraWaits to 0. This
> same issue exists from Group clear xid for which I will send a patch
> separately.
>
> Apart from above, the patch needs to be adjusted for commit be7b2848
> which has changed the definition of PGSemaphore.
I have reviewed the latest patch and I don't have any more comments.
So if there is no objection from other reviewers I can move it to
"Ready For Committer"?
I have performed one more test, with 3000 scale factor because
previously I tested only up to 1000 scale factor. The purpose of this
test is to check whether there is any regression at higher scale
factor.
Machine: Intel 8 socket machine.
Scale Factor: 3000
Shared Buffer: 8GB
Test: Pgbench RW test.
Run: 30 mins median of 3
Other modified GUC:
-N 300 -c min_wal_size=15GB -c max_wal_size=20GB -c
checkpoint_timeout=900 -c maintenance_work_mem=1GB -c
checkpoint_completion_target=0.9
Summary:
- Did not observed any regression.
- The performance gain is in sync with what we have observed with
other tests at lower scale factors.
Sync_Commit_Off:
client Head Patch
8 10065 10009
16 18487 18826
32 28167 28057
64 26655 28712
128 20152 24917
256 16740 22891
Sync_Commit_On:
Client Head Patch
8 5102 5110
16 8087 8282
32 12523 12548
64 14701 15112
128 14656 15238
256 13421 16424
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com