Received: from localhost (unknown [200.46.208.211]) by mail.postgresql.org (Postfix) with ESMTP id 1FC9C63343C for ; Tue, 17 Mar 2009 18:46:03 -0300 (ADT) Received: from mail.postgresql.org ([200.46.204.86]) by localhost (mx1.hub.org [200.46.208.211]) (amavisd-maia, port 10024) with ESMTP id 40761-01-3 for ; Tue, 17 Mar 2009 18:45:54 -0300 (ADT) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from brmea-mail-2.sun.com (brmea-mail-2.Sun.COM [192.18.98.43]) by mail.postgresql.org (Postfix) with ESMTP id C2371634528 for ; Tue, 17 Mar 2009 18:45:32 -0300 (ADT) Received: from fe-amer-10.sun.com ([192.18.109.80]) by brmea-mail-2.sun.com (8.13.6+Sun/8.12.9) with ESMTP id n2HLjVuD025234 for ; Tue, 17 Mar 2009 21:45:31 GMT MIME-version: 1.0 Content-type: multipart/mixed; boundary="Boundary_(ID_6QKC0SKo8JxNVl0OlNAYPA)" Received: from conversion-daemon.mail-amer.sun.com by mail-amer.sun.com (Sun Java(tm) System Messaging Server 7.0-5.01 64bit (built Feb 19 2009)) id <0KGO00A006Q3HK00@mail-amer.sun.com> for pgsql-performance@postgresql.org; Tue, 17 Mar 2009 15:45:31 -0600 (MDT) Received: from [129.148.169.151] ([unknown] [129.148.169.151]) by mail-amer.sun.com (Sun Java(tm) System Messaging Server 7.0-5.01 64bit (built Feb 19 2009)) with ESMTPSA id <0KGO00KZU73G9Q00@mail-amer.sun.com>; Tue, 17 Mar 2009 15:45:17 -0600 (MDT) Date: Tue, 17 Mar 2009 17:41:20 -0400 From: "Jignesh K. Shah" Subject: Re: Proposal of tunable fix for scalability of 8.4 In-reply-to: <1237225198.3963.60.camel@ebony.2ndQuadrant> To: Simon Riggs Cc: Tom Lane , Kevin Grittner , pgsql-performance@postgresql.org Message-id: <49C01900.5030406@sun.com> Organization: Sun Microsystems References: <49B824DD.7090302@sun.com> <49B7F470.EE98.0025.0@wicourts.gov> <29080.1236821556@sss.pgh.pa.us> <49B87161.10201@sun.com> <1237225198.3963.60.camel@ebony.2ndQuadrant> User-Agent: Thunderbird 2.0.0.17 (X11/20081023) X-Virus-Scanned: Maia Mailguard 1.0.1 X-Archive-Number: 200903/234 X-Sequence-Number: 33156 This is a multi-part message in MIME format. --Boundary_(ID_6QKC0SKo8JxNVl0OlNAYPA) Content-type: multipart/alternative; boundary="Boundary_(ID_r8I3WB4Fah2j82bNiuo/jw)" --Boundary_(ID_r8I3WB4Fah2j82bNiuo/jw) Content-type: text/plain; format=flowed; charset=ISO-8859-1 Content-transfer-encoding: 7BIT On 03/16/09 13:39, Simon Riggs wrote: > On Wed, 2009-03-11 at 22:20 -0400, Jignesh K. Shah wrote: > >> A tunable does not impact existing behavior > > Why not put the tunable parameter into the patch and then show the test > results with it in? If there is no overhead, we should then be able to > see that. > I did a patch where I define lock_wakeup_algorithm with default value of 0, and range is 0 to 32 It basically handles three types of algorithms and 32 different permutations, such that: When lock_wakeup_algorithm is set to 0 => default logic of wakeup (only 1 exclusive or all sequential shared) 1 => wake up all sequential exclusives or all sequential shared 32>= n >=2 => wake up first n waiters irrespective of exclusive or sequential I did a quick test with patch. Unfortunately it improves my number even with default setting 0 (not sure whether I should be pleased or sad - Definitely no overhead infact seems to help performance a bit. NOTE: Logic is same, implementation is slightly different for default set) my Prepatch numbers typically peaked around 136,000 tpm With the patch and settings: lock_wakeup_algorithm=0 PEAK: 962: 512: Medium Throughput: 161121.000 Avg Medium Resp: 0.051 When lock_wakeup_algorithm=1 Then my PEAK increases to PEAK 1560: 832: Medium Throughput: 176577.000 Avg Medium Resp: 0.086 (Couldn't recreate the 184K+ result.. need to check that) I still havent tested for the rest 2-32 values but you get the point, the patch is quite flexible with various types of permutations and no overhead. Do give it a try on your own setup and play with values and compare it with your original builds. Regards, Jignesh --Boundary_(ID_r8I3WB4Fah2j82bNiuo/jw) Content-type: text/html; charset=ISO-8859-1 Content-transfer-encoding: 7BIT

On 03/16/09 13:39, Simon Riggs wrote:
On Wed, 2009-03-11 at 22:20 -0400, Jignesh K. Shah wrote:
A tunable does not impact existing behavior

Why not put the tunable parameter into the patch and then show the test
results with it in? If there is no overhead, we should then be able to
see that.



I did a patch where I define lock_wakeup_algorithm with default value of 0, and range is 0 to 32
It basically handles three types of algorithms and 32 different permutations, such that:
When lock_wakeup_algorithm is set to
0               => default logic of wakeup (only 1 exclusive or all sequential shared)
1               => wake up all sequential exclusives or all sequential shared
32>= n >=2      => wake up first n waiters irrespective of exclusive or sequential



I did a quick test with patch. Unfortunately it improves my number even with default setting 0 (not sure whether I should be pleased or sad - Definitely no overhead infact seems to help performance a bit. NOTE: Logic is same, implementation is slightly different for default set)

my Prepatch numbers typically peaked around 136,000 tpm
With the patch and settings:

lock_wakeup_algorithm=0
PEAK: 962: 512: Medium Throughput: 161121.000 Avg Medium Resp: 0.051


When lock_wakeup_algorithm=1
Then my PEAK increases to
PEAK 1560: 832: Medium Throughput: 176577.000 Avg Medium Resp: 0.086
(Couldn't recreate the 184K+ result.. need to check that)

I still havent tested for the rest 2-32 values but you get the point, the patch is quite flexible with various types of permutations and no overhead.

Do give it a try on your own setup and play with values and compare it with your original builds.

Regards,
Jignesh

 --Boundary_(ID_r8I3WB4Fah2j82bNiuo/jw)-- --Boundary_(ID_6QKC0SKo8JxNVl0OlNAYPA) Content-type: text/plain; name=lwlock.c.patch Content-transfer-encoding: 7BIT Content-disposition: inline; filename=lwlock.c.patch *** lwlock.c Tue Mar 17 12:27:49 2009 --- lwlock.c.orig Wed Mar 11 12:48:27 2009 *************** *** 87,93 **** static int lock_addin_request = 0; static bool lock_addin_request_allowed = true; - int LWLockWakeupAlgorithm; #ifdef LWLOCK_STATS static int counts_for_pid = 0; --- 87,92 ---- *************** *** 564,570 **** PGPROC *head; PGPROC *proc; int i; - int runq; PRINT_LWDEBUG("LWLockRelease", lockid, lock); --- 563,568 ---- *************** *** 612,631 **** * as many waiters as want shared access. */ proc = head; ! if (LWLockWakeupAlgorithm || !proc->lwExclusive) ! { ! if (LWLockWakeupAlgorithm <= 1) ! { while (proc->lwWaitLink != NULL && ! (proc->lwExclusive == proc->lwWaitLink->lwExclusive)) proc = proc->lwWaitLink; - } - else - { - runq= LWLockWakeupAlgorithm; - while (proc->lwWaitLink != NULL && --runq) - proc = proc->lwWaitLink; - } } /* proc is now the last PGPROC to be released */ lock->head = proc->lwWaitLink; --- 610,620 ---- * as many waiters as want shared access. */ proc = head; ! if (!proc->lwExclusive) ! { while (proc->lwWaitLink != NULL && ! !proc->lwWaitLink->lwExclusive) proc = proc->lwWaitLink; } /* proc is now the last PGPROC to be released */ lock->head = proc->lwWaitLink; --Boundary_(ID_6QKC0SKo8JxNVl0OlNAYPA) Content-type: text/plain; name=lwlock.h.patch Content-transfer-encoding: 7BIT Content-disposition: inline; filename=lwlock.h.patch *** lwlock.h.orig Tue Mar 17 14:27:10 2009 --- lwlock.h Tue Mar 17 08:24:40 2009 *************** *** 103,106 **** --- 103,107 ---- extern void RequestAddinLWLocks(int n); + extern int LWLockWakeupAlgorithm; #endif /* LWLOCK_H */ --Boundary_(ID_6QKC0SKo8JxNVl0OlNAYPA) Content-type: text/plain; name=guc.c.patch Content-transfer-encoding: 7BIT Content-disposition: inline; filename=guc.c.patch *** guc.c.orig Tue Mar 17 07:30:26 2009 --- guc.c Tue Mar 17 07:47:10 2009 *************** *** 57,62 **** --- 57,63 ---- #include "postmaster/walwriter.h" #include "regex/regex.h" #include "storage/bufmgr.h" + #include "storage/lwlock.h" #include "storage/fd.h" #include "tcop/tcopprot.h" #include "tsearch/ts_cache.h" *************** *** 167,172 **** --- 168,174 ---- static bool assign_maxconnections(int newval, bool doit, GucSource source); static bool assign_autovacuum_max_workers(int newval, bool doit, GucSource source); static bool assign_effective_io_concurrency(int newval, bool doit, GucSource source); + static bool assign_lock_wakeup_algorithm(int newval, bool doit, GucSource source); static const char *assign_pgstat_temp_directory(const char *newval, bool doit, GucSource source); static char *config_enum_get_options(struct config_enum *record, *************** *** 416,421 **** --- 418,424 ---- static int wal_segment_size; static bool integer_datetimes; static int effective_io_concurrency; + static int lock_wakeup_algorithm; /* should be static, but commands/variable.c needs to get at these */ char *role_string; *************** *** 1727,1732 **** --- 1730,1745 ---- }, { + {"lock_wakeup_algorithm", PGC_USERSET, RESOURCES, + gettext_noop("Select algorithm for lock waiters wakeup. 0 is default"), + gettext_noop("Select 1 for SMP systems, and higher for manual conrol of number of waiters to wake up.") + }, + &lock_wakeup_algorithm, + 0, 0, 32, + assign_lock_wakeup_algorithm, NULL + }, + + { {"log_rotation_age", PGC_SIGHUP, LOGGING_WHERE, gettext_noop("Automatic log file rotation will occur after N minutes."), NULL, *************** *** 7668,7673 **** --- 7681,7698 ---- #endif /* USE_PREFETCH */ } + static bool + assign_lock_wakeup_algorithm(int newval, bool doit, GucSource source) + { + if ( newval < 0 && newval > 32) + return false; + + if (doit) + LWLockWakeupAlgorithm= newval ; + + return true; + } + static const char * assign_pgstat_temp_directory(const char *newval, bool doit, GucSource source) { --Boundary_(ID_6QKC0SKo8JxNVl0OlNAYPA)--