Re: multi-threaded pgbench

Lists: pgsql-hackers
From: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
To: pgsql-hackers(at)postgresql(dot)org
Subject: multi-threaded pgbench
Date: 2009-07-08 05:25:17
Message-ID: 20090708134406.AEB4.52131E4D@oss.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Pgbench is a famous tool to measure postgres performance, but nowadays
it does not work well because it cannot use multiple CPUs. On the other
hand, postgres server can use CPUs very well, so the bottle-neck of
workload is *in pgbench*.

Multi-threading would be a solution. The attached patch adds -j
(number of jobs) option to pgbench. If the value N is greater than 1,
pgbench runs with N threads. Connections are equally-divided into
them (ex. -c64 -j4 => 4 threads with 16 connections each). It can
run on POSIX platforms with pthread and on Windows with win32 threads.

Here are results of multi-threaded pgbench runs on Fedora 11 with intel
core i7 (8 logical cores = 4 physical cores * HT). -j8 (8 threads) was
the best and the tps is 4.5 times of -j1, that is a traditional result.

$ pgbench -i -s10
$ pgbench -n -S -c64 -j1 => tps = 11600.158593
$ pgbench -n -S -c64 -j2 => tps = 17947.100954
$ pgbench -n -S -c64 -j4 => tps = 26571.124001
$ pgbench -n -S -c64 -j8 => tps = 52725.470403
$ pgbench -n -S -c64 -j16 => tps = 38976.675319
$ pgbench -n -S -c64 -j32 => tps = 28998.499601
$ pgbench -n -S -c64 -j64 => tps = 26701.877815

Is it acceptable to use pthread in contrib module?
If ok, I will add the patch to the next commitfest.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center

Attachment Content-Type Size
pgbench-mt.patch application/octet-stream 25.6 KB

From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-08 15:23:32
Message-ID: 20090708152332.GB5053@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Itagaki Takahiro wrote:

> Is it acceptable to use pthread in contrib module?

We don't have a precedent it seems. I think the requirement would be
that it should compile if pthread support is not present.

> If ok, I will add the patch to the next commitfest.

Add it anyway -- discussion should happen during commitfest if it
doesn't spark right away.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-08 15:41:19
Message-ID: 4A54BE1F.1070401@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Alvaro Herrera wrote:
> Itagaki Takahiro wrote:
>
>> Is it acceptable to use pthread in contrib module?
>
> We don't have a precedent it seems. I think the requirement would be
> that it should compile if pthread support is not present.

My thoughts as well. But I wonder, would it be harder or easier to use
fork() instead?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-08 15:46:05
Message-ID: 22112.1247067965@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Itagaki Takahiro wrote:
>> Is it acceptable to use pthread in contrib module?

> We don't have a precedent it seems. I think the requirement would be
> that it should compile if pthread support is not present.

Right. Breaking it for non-pthread environments is not acceptable.

The real question here is whether it will be a problem if pgbench
delivers significantly different results when built with or without
threading support. I can see arguents either way on that ...

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-08 15:56:04
Message-ID: 4A54C194.9000406@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Heikki Linnakangas wrote:
> Alvaro Herrera wrote:
>
>> Itagaki Takahiro wrote:
>>
>>
>>> Is it acceptable to use pthread in contrib module?
>>>
>> We don't have a precedent it seems. I think the requirement would be
>> that it should compile if pthread support is not present.
>>
>
> My thoughts as well. But I wonder, would it be harder or easier to use
> fork() instead?
>
>

I have just been down this road to some extent with parallel pg_restore,
which uses threads on Windows. That might be useful as a bit of a
template. Extending it to use pthreads would probably be fairly trivial.
The thread/fork specific stuff ended up being fairly isolated for
pg_restore. see src/bin/pg_dump/pg_backup_archiver.c:spawn_restore()

I think you should have it use pthreads if available, or Windows threads
there, or fork() elsewhere.

cheers

andrew


From: Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)cc>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-08 16:03:09
Message-ID: 4A54C33D.1060503@kaltenbrunner.cc
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
>> Itagaki Takahiro wrote:
>>> Is it acceptable to use pthread in contrib module?
>
>> We don't have a precedent it seems. I think the requirement would be
>> that it should compile if pthread support is not present.
>
> Right. Breaking it for non-pthread environments is not acceptable.
>
> The real question here is whether it will be a problem if pgbench
> delivers significantly different results when built with or without
> threading support. I can see arguents either way on that ...

well pgbench as it is now is now is more ore less unusable on modern
hardware for SELECT type queries(way too slow to scale to what the
backend can do thses days and the number of cores in a recent box).
It is only somewhat usable on the default update heavy test as well
because even there it is hitting scalability limits (ie I can easily
improve on its numbers with a perl script that forks and issues the same
queries).
I would even go as far as issuing a WARNING if pgbench is invoked and
not compiled with threads if we accept this patch...

Stefan


From: Greg Smith <gsmith(at)gregsmith(dot)com>
To: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-08 16:38:05
Message-ID: alpine.GSO.2.01.0907081227570.18239@westnet.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, 8 Jul 2009, Itagaki Takahiro wrote:

> Multi-threading would be a solution. The attached patch adds -j
> (number of jobs) option to pgbench.

Should probably name this -w "numbers of workers" to stay consistent with
terminology used on the server side.

> Is it acceptable to use pthread in contrib module?
> If ok, I will add the patch to the next commitfest.

pgbench is basically broken right now, as demonstrated by the lack of
scaling show in your results and similar ones I've collected. This looks
like it fixes the primary problem there. While it would be nice if a
multi-process based solution were written instead, unless someone is
willing to step up and volunteer to write one I'd much rather see your
patch go in than doing nothing at all. It shouldn't even impact old
results if you don't toggle the option on.

I have 3 new server systems I was going to run pgbench on anyway in the
next month as part of my standard performance testing on new hardware.
I'll be happy to mix in results using the multi-threaded pgbench to check
the patch's performance, along with the rest of the initial review here.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-08 16:40:37
Message-ID: 23103.1247071237@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> I think you should have it use pthreads if available, or Windows threads
> there, or fork() elsewhere.

Hmm, but how will you communicate stats back from the sub-processes?
pg_restore doesn't need anything more than a success/failure result
from its child processes, but I think pgbench will want more.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-08 16:57:43
Message-ID: 4A54D007.10709@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> I think you should have it use pthreads if available, or Windows threads
>> there, or fork() elsewhere.
>>
>
> Hmm, but how will you communicate stats back from the sub-processes?
> pg_restore doesn't need anything more than a success/failure result
> from its child processes, but I think pgbench will want more.
>
>

My first reaction is to say "use a pipe."

cheers

andtrew


From: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-09 02:09:21
Message-ID: 20090709110455.98B9.52131E4D@oss.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:

> I think you should have it use pthreads if available, or Windows threads
> there, or fork() elsewhere.

Just a question - which platform does not support any threading?
I think threading is very common in modern applications. If there
are such OSes, they seem to be just abandoned and not maintained...

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center


From: Greg Smith <gsmith(at)gregsmith(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-09 04:08:17
Message-ID: alpine.GSO.2.01.0907090006350.11767@westnet.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, 8 Jul 2009, Tom Lane wrote:

> pg_restore doesn't need anything more than a success/failure result
> from its child processes, but I think pgbench will want more.

The biggest chunk of returned state to consider is how each client
transaction generates a line of latency information that goes into the log
file.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD


From: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>, pgsql-hackers(at)postgresql(dot)org
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Subject: Re: multi-threaded pgbench
Date: 2009-07-09 08:51:18
Message-ID: 20090709172254.98BC.52131E4D@oss.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Here is an updated version of multi-threaded pgbench patch.

Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:

> > Hmm, but how will you communicate stats back from the sub-processes?
> My first reaction is to say "use a pipe."

I added partial implementation of pthread using fork and pipe for platform
without ENABLE_THREAD_SAFETY. Pthread version is not necessarily needed
if we have the fork version, but I still left it as-is.

The name of new option is still -j, that is borrowed from pg_restore
and gmake. They use -j for multi-worker-processing.

-j NUM number of threads (default: 1)

I needed to modify the meaning of tps (excluding connections establishing)
a little because connections are executed in parallel. I subtract average
of connection times from total execution time.

total_time := last_thread_finish_time - first_thread_start_time
tps (including connection) := num_transaction / total_time
tps (excluding connection) := num_transaction /
(total_time - (total_connection_time / num_threads))

I notice that I also fixed a few parts of pgbench:
* Use instr_time instead of struct timeval.
Macros in portability/instr_time.h makes codes cleaner.
* Accept "\sleep 1ms" format (no spaces between "1" and "ms") for sleep
meta command. The old version of pgbench interprets "1ms" as just "1",
that means "1 s". It was confusable.

I'll add the patch to the commitfest page.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center

Attachment Content-Type Size
pgbench-mt_20090709.patch application/octet-stream 38.7 KB

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, pgsql-hackers(at)postgresql(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Subject: Re: multi-threaded pgbench
Date: 2009-07-18 19:25:03
Message-ID: 603c8f070907181225v6c4af0e3l1423205994e094b6@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Jul 9, 2009 at 4:51 AM, Itagaki
Takahiro<itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> wrote:
> Here is an updated version of multi-threaded pgbench patch.

Greg (Smith), do you have time to review this version? If not, I will
assign a round-robin reviewer when one becomes available.

...Robert


From: Greg Stark <gsstark(at)mit(dot)edu>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, Andrew Dunstan <andrew(at)dunslane(dot)net>, pgsql-hackers(at)postgresql(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Subject: Re: multi-threaded pgbench
Date: 2009-07-18 19:40:04
Message-ID: 407d949e0907181240w31f08441kea2e3ca6fcfdb5c0@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sat, Jul 18, 2009 at 8:25 PM, Robert Haas<robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Jul 9, 2009 at 4:51 AM, Itagaki
> Takahiro<itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> wrote:
>> Here is an updated version of multi-threaded pgbench patch.
>
> Greg (Smith), do you have time to review this version?  If not, I will
> assign a round-robin reviewer when one becomes available.

Incidentally you could assign me something if you want.

I gave feedback on Simon/Your join removal and the Append min/max
patch. I don't think either has really reached any conclusive
"finished" state though. I suppose I should mark your patch as
"returned with feedback" even if it's mostly just "good work, keep
going"? And the other patch isn't actually in this commitfest but I
think we're still discussing what it should do.

--
greg
http://mit.edu/~gsstark/resume.pdf


From: Josh Berkus <josh(at)agliodbs(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, Andrew Dunstan <andrew(at)dunslane(dot)net>, pgsql-hackers(at)postgresql(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Subject: Re: multi-threaded pgbench
Date: 2009-07-19 04:50:13
Message-ID: 4A62A605.4090209@agliodbs.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> Greg (Smith), do you have time to review this version? If not, I will
> assign a round-robin reviewer when one becomes available.

I can do a concurrency test of this next week.

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Greg Stark <gsstark(at)mit(dot)edu>
Cc: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, Andrew Dunstan <andrew(at)dunslane(dot)net>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Subject: Re: multi-threaded pgbench
Date: 2009-07-19 10:29:37
Message-ID: A2089D3C-0EB5-41FF-B299-1E254A33CE67@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Jul 18, 2009, at 3:40 PM, Greg Stark <gsstark(at)mit(dot)edu> wrote:

> On Sat, Jul 18, 2009 at 8:25 PM, Robert Haas<robertmhaas(at)gmail(dot)com>
> wrote:
>> On Thu, Jul 9, 2009 at 4:51 AM, Itagaki
>> Takahiro<itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> wrote:
>>> Here is an updated version of multi-threaded pgbench patch.
>>
>> Greg (Smith), do you have time to review this version? If not, I
>> will
>> assign a round-robin reviewer when one becomes available.
>
> Incidentally you could assign me something if you want.

OK.

> I gave feedback on Simon/Your join removal and the Append min/max
> patch. I don't think either has really reached any conclusive
> "finished" state though. I suppose I should mark your patch as
> "returned with feedback" even if it's mostly just "good work, keep
> going"? And the other patch isn't actually in this commitfest but I
> think we're still discussing what it should do.

Well, I think we really need Tom to look at join removal. If he
doesn't have any better ideas for how to structure the code it's not
clear to me that we shouldn't just commit what I already did and then
start future work from there. But this seems like an issue for that
thread rather than this one.

Wrt append min/max I think we should postpone further discussion until
end of commitfest, since it was submitted mid-CommitFest.

...Robert


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Josh Berkus <josh(at)agliodbs(dot)com>
Cc: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, Andrew Dunstan <andrew(at)dunslane(dot)net>, pgsql-hackers(at)postgresql(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Subject: Re: multi-threaded pgbench
Date: 2009-07-19 10:49:08
Message-ID: 603c8f070907190349i3dabf3f3nf6c12cbfb080bb61@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sun, Jul 19, 2009 at 12:50 AM, Josh Berkus<josh(at)agliodbs(dot)com> wrote:
>> Greg (Smith), do you have time to review this version?  If not, I will
>> assign a round-robin reviewer when one becomes available.
>
> I can do a concurrency test of this next week.

Sounds good.

...Robert


From: Greg Smith <gsmith(at)gregsmith(dot)com>
To: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-23 02:23:38
Message-ID: alpine.GSO.2.01.0907222158050.16713@westnet.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

I just took multi-threaded pgbench for an initial spin, looks good overall
with only a couple of small rough edges.

The latest code works differently depending on whether you compiled with
--enable-thread-safety or not, it defines some structures based on fork if
it's not enabled:

#elif defined(ENABLE_THREAD_SAFETY)
#include <pthread.h>
#else
#include <sys/wait.h>
typedef struct fork_pthread *pthread_t;
typedef int pthread_attr_t;
static int pthread_create(pthread_t *thread, pthread_attr_t *attr, void
* (*start_routine)(void *), void * arg);
static int pthread_join(pthread_t th, void **thread_return);
#endif

That second code path, when --enable-thread-safety is turned off, crashes
and burns on my Linux system:

gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv
-I../../src/interfaces/libpq -I. -I../../src/include -D_GNU_SOURCE -c -o
pgbench.o pgbench.c -MMD -MP -MF .deps/pgbench.Po
pgbench.c:72: error: conflicting types for pthread_t
/usr/include/bits/pthreadtypes.h:50: error: previous declaration of
pthread_t was here
pgbench.c:73: error: conflicting types for pthread_attr_t
/usr/include/bits/pthreadtypes.h:57: error: previous declaration of
pthread_attr_t was here

So that's the first problem to sort out, I was planning to test that path
as well as the regular threaded one. Since I'd expect there to be Linux
packages built both with and without thread safety enabled, they both
should work, even though people should always be turning safety on
nowadays.

We should try to get a Windows based tester here too at some point,
there's a completely different set of thread wrapper code for that OS that
could use a look by somebody more familiar than me with that platform.

The second thing that concerns me is that there's a limitation in the code
where the number of clients must be a multiple of the number of workers.
When I tried to gradually step up the client volume the tests wouldn't
run:

$ ./pgbench -j 16 -S -c 24 -t 10000 pgbench
number of clients (24) must be a multiple number of threads (16)

Once the larger issues are worked out, I would be much friendlier if it
were possible to pass new threads a client count so that the last in the
pool could service a smaller number. The logic for that is kind of a
pain--in this case you'd want 8 threads running 2 clients each while 8 ran
1 client--but it would really be much friendlier and flexible that way.

Onto performance. My test system has a 16 cores of Xeon X5550 @ 2.67GHz.
I created a little pgbench database (-s 10) and used the default
postgresql.conf parameters for everything but max_connections for a rough
initial test.

Performance on this box drops off pretty fast once you get past 16
clients; using the original, unpatched pgbench:

c tps
16 86887
24 70685
32 63787
64 64712
128 60602

A quick test of the new version suggest that there's no glaring
performance regression running it with a single client thread:

$ ./pgbench.orig -S -c 64 -t 10000 pgbench
tps = 64712.451737 (including connections establishing)

$ ./pgbench -S -c 64 -t 10000 pgbench
tps = 63806.494046 (including connections establishing)

So I moved onto to testing with a worker thread per CPU:

./pgbench -j 16 -S -c 16 -t 100000 pgbench
./pgbench -j 16 -S -c 32 -t 50000 pgbench
./pgbench -j 16 -S -c 64 -t 10000 pgbench
./pgbench -j 16 -S -c 128 -t 10000 pgbench

And got considerably better results:

c tps
16 96223
32 89014
64 82487
128 74217

That's as much as a 40% speedup @ 32 clients, and even a decent win at
lower counts.

The patch looks like it accomplishes its performance goals quite well
here. I'll be glad to run some more extensive performance tests, but I'd
like to at least see the version without --enable-thread-safety fixed
first so that I can queue up and compare both versions when I go through
that.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD


From: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
To: Greg Smith <gsmith(at)gregsmith(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-23 03:25:44
Message-ID: 20090723120538.9820.52131E4D@oss.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Greg Smith <gsmith(at)gregsmith(dot)com> wrote:

> That second code path, when --enable-thread-safety is turned off, crashes
> and burns on my Linux system:

It comes from confliction of identifiers.
Renaming identifiers with #define can solve the errors:

#define pthread_t pg_pthread_t
#define pthread_attr_t pg_pthread_attr_t
#define pthread_create pg_pthread_create
#define pthread_join pg_pthread_join
typedef struct fork_pthread *pthread_t;
...

Another idea is that we don't use pthread and add 'pg_thread' wrapper
module on the top of pthread.

We can choose either of implementations... Which is better?

> $ ./pgbench -j 16 -S -c 24 -t 10000 pgbench
> number of clients (24) must be a multiple number of threads (16)

It's hard on forking-thread platforms because multiple threads need
to access the job queue. We need to put the queue on inter-process
shared memory, but it introduces additional complexities.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center


From: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
To: Greg Smith <gsmith(at)gregsmith(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-24 01:17:42
Message-ID: 20090724101248.93DD.52131E4D@oss.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> wrote:

> Greg Smith <gsmith(at)gregsmith(dot)com> wrote:
> > That second code path, when --enable-thread-safety is turned off, crashes
> > and burns on my Linux system:
>
> It comes from confliction of identifiers.
> Renaming identifiers with #define can solve the errors:
> #define pthread_t pg_pthread_t

Here is a patch to fix compile errors by identifier-renaming
when thread-safety is disabled on linux.

Also I fixed file descriptor leaks at the end of benchmark.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center

Attachment Content-Type Size
pgbench-mt_20090724.patch application/octet-stream 39.2 KB

From: Josh Williams <joshwilliams(at)ij(dot)net>
To: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, Greg Smith <gsmith(at)gregsmith(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-28 04:47:02
Message-ID: 1248756422.6270.148.camel@lapdragon
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, 2009-07-22 at 22:23 -0400, Greg Smith wrote:
> Onto performance. My test system has a 16 cores of Xeon X5550 @
> 2.67GHz.
> I created a little pgbench database (-s 10) and used the default
> postgresql.conf parameters for everything but max_connections for a
> rough
> initial test.

To test on Windows, I set up a similar database on an 8-core 2.0GHz
E5335 (closest match I have.) It's compiled against a fresh CVS pull
from this morning, patched with the "20090724" updated version. I tried
to mirror the tests as much as possible, including the concurrent thread
counts despite having half the number of available cores. Doing that
didn't have much impact on the results, but more on that later.

Comparing the unpatched version to the new version running a single
client thread, there's no significant performance difference:

C:\pgsql85>bin\pgbenchorig.exe -S -c 8 -t 100000 pgbench
...
tps = 19061.234215 (including connections establishing)

C:\pgsql85>bin\pgbench.exe -S -c 8 -t 100000 pgbench
tps = 18852.928562 (including connections establishing)

As a basis of comparison the original pgbench was run with increasing
client counts, which shows the same drop off in throughput past the
16-client sweet spot:

con tps
8 18871
16 19161
24 18804
32 18670
64 17598
128 16664

However I was surprised to see these results for the patched version,
running 16 worker threads (apart from the 8 client run of course.)

C:\pgsql85>bin\pgbench.exe -S -j 16 -c 128 -t 100000 pgbench ...
con tps
8 18435 (-j 8)
16 18866
24 -----
32 17937
64 17016
128 15930

In all cases the patched version resulted in a lower performing output
than the unpatched version. It's clearly working, at least in that it's
launching the requested number of worker threads when looking at the
process. Adjusting the worker thread count down to match the number of
cores yielded identical results in the couple of test cases I ran.
Maybe pgbench itself is less of a bottleneck in this environment,
relatively speaking?

- Josh Williams


From: Greg Smith <gsmith(at)gregsmith(dot)com>
To: Josh Williams <joshwilliams(at)ij(dot)net>
Cc: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-28 16:10:57
Message-ID: alpine.GSO.2.01.0907281204310.29621@westnet.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, 28 Jul 2009, Josh Williams wrote:

> Maybe pgbench itself is less of a bottleneck in this environment,
> relatively speaking?

On UNIXish systems, you know you've reached the conditions under which the
threaded pgbench would be helpful if the pgbench client program itself is
taking up a large percentage of a CPY just by itself. If your test system
is still setup, it might be interesting to try the 64 and 128 client cases
with Task Manager open, to see what percentage of the CPU the pgbench
driver program is using. If the pgbench client isn't already pegged at a
full CPU, I wouldn't necessarily threading it to help--it would just add
overhead that doesn't buy you anything, which seems to be what you're
measuring.

All the Linux tests suggest that limit tends up show up at over 20,000 TPS
nowawadys, so maybe your Window system is bottlenecking somewhere
completely different before it reaches saturation on the client.

In any case, Josh's review is exactly what I wanted to see here--the code
does compile and run successfully for someone besides its author under
Windows. Making it *effective* on that platform might end up being
outside the scope of what we want to chew on right now. I'll have updated
performance results to submit later this week against the updated patch.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD


From: Josh Williams <joshwilliams(at)ij(dot)net>
To: Greg Smith <gsmith(at)gregsmith(dot)com>
Cc: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-29 03:38:36
Message-ID: 1248838716.6953.38.camel@lapdragon
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, 2009-07-28 at 12:10 -0400, Greg Smith wrote:
> If your test system
> is still setup, it might be interesting to try the 64 and 128 client cases
> with Task Manager open, to see what percentage of the CPU the pgbench
> driver program is using. If the pgbench client isn't already pegged at a
> full CPU, I wouldn't necessarily threading it to help--it would just add
> overhead that doesn't buy you anything, which seems to be what you're
> measuring.

That's a really good point, I do recall seeing pgbench taking only a
fraction of the CPU... Running it again, it hovers around 6 or 7
percent in both cases, so it's only using up around half a core.

Huh, running the patched version on a single thread with 128 clients
just got it to crash. Actually consistently, three times now. Will try
the same thing on the development box tomorrow morning to get some
better debugging information.

> All the Linux tests suggest that limit tends up show up at over 20,000 TPS
> nowawadys, so maybe your Window system is bottlenecking somewhere
> completely different before it reaches saturation on the client.

I figured it was just indicating a limitation of the environment, where
Windows has some kind of inefficiency either in the PG port or just
something inherent in how the OS works. It does make me wonder where
exactly all that CPU time is going, though. OProfile, how I miss thee.
But that's a different discussion entirely.

- Josh Williams


From: Josh Williams <joshwilliams(at)ij(dot)net>
To: Greg Smith <gsmith(at)gregsmith(dot)com>
Cc: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-29 21:31:53
Message-ID: 1248903114.6348.195.camel@lapdragon
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, 2009-07-28 at 23:38 -0400, Josh Williams wrote:
> Huh, running the patched version on a single thread with 128 clients
> just got it to crash. Actually consistently, three times now. Will try
> the same thing on the development box tomorrow morning to get some
> better debugging information.

So yeah, buffer overrun.

In pgbench.c FD_SETSIZE is redefined to get around the Windows default
of 64. But this is done after bringing in winsock2.h (a couple levels
in as a result of first including postgres_fe.h). So any fd_set is
built with an array of 64 descriptors, while pgbench thinks it has 1024
available to work with.

This was introduced a while back; the multi-threaded patch just makes it
visible by giving it an important pointer to write over. Previously it
would just run over into the loop counter (and probably a couple other
things) and thus it'd continue on happily with the [sub]set it has.

In either case this seems to be a simple fix, to move that #define
earlier (see pgbench_win32.patch.)

- Josh Williams

Attachment Content-Type Size
pgbench_win32.patch text/x-patch 626 bytes

From: Greg Smith <gsmith(at)gregsmith(dot)com>
To: Josh Williams <joshwilliams(at)ij(dot)net>
Cc: Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-30 00:35:36
Message-ID: alpine.GSO.2.01.0907291918380.19638@westnet.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

This patch is wrapping up nicely. I re-tested against the updated
pgbench-mt_20090724 and now I get similar results whether or not
--enable-thread-safety is enabled on Linux, so that problem is gone.
Josh's successful Windows tests along with finding the bug he attached a
patch to is also encouraging.

I re-ran my performance tests with the same basic setup (16 core system,
database scale=10, read-only tests) but this time increased shared_buffers
to 256MB just to see if results popped up significantly (they didn't).

Here's a comparison of the original pgbench select-only TPS against the
new version using 1 thread:

clients
threads 16 32 64 128
none 91763 69707 68465 63730
1 90797 70117 66324 63626

I ran these a few times and those are basically the same result. If
there's a regression using 1 threads instead of 1 process, which I thought
I was seeing at one point with j=1/c=128, under closer investigation it
would have to be much smaller than the run to run variation of pgbench
because it vanished when I collected many runs of data.

Running the new pgbench with thread safety turned on:

clients
threads 16 32 64 128
1 89503 67849 67120 63499
2 97883 91888 87556 84430
4 95319 96409 90445 83569
8 96002 95411 88988 82383
16 103798 95056 87701 82253
32 X 95869 88253 82253

Running it without thread safety turned on so it uses processes instead
(this is the case I couldn't report on before):

clients
threads 16 32 64 128
1 89706 68702 64545 62770
2 99224 91677 88812 82442
4 96124 96552 90245 83311
8 97066 96000 89149 83266
16 103276 96088 88276 82652
32 X 97405 90082 83672

Those two tables are also identical relative to the run to run pgbench
noise.

This looks ready for a committer review to me, I'm happy that the patch
performs as expected and it seems to work across two platforms.

To step back for a second, I'm testing a fairly optimistic situation--the
standard RHEL 2.6.18 kernel which doesn't have any major issues here--and
I see a decent sized speedup (>30%) in the worst case. I've reported
before that running pgbench on newer Linux kernels (>=2.6.23) is horribly
slow, and sure enough the original results kicking off this thread showed
the same thing: only 11600 TPS on a modern 8 core system. That's less
than 1/4 what that server is capable of, and this patch allows working
around that issue nicely. pgbench not scaling up really a much worse
problem than my test results suggest.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD


From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Josh Williams <joshwilliams(at)ij(dot)net>
Cc: Greg Smith <gsmith(at)gregsmith(dot)com>, Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: multi-threaded pgbench
Date: 2009-07-30 09:28:03
Message-ID: 9837222c0907300228v53a98596g199902cacd3eae5e@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Jul 29, 2009 at 23:31, Josh Williams<joshwilliams(at)ij(dot)net> wrote:
> On Tue, 2009-07-28 at 23:38 -0400, Josh Williams wrote:
>> Huh, running the patched version on a single thread with 128 clients
>> just got it to crash.  Actually consistently, three times now.  Will try
>> the same thing on the development box tomorrow morning to get some
>> better debugging information.
>
> So yeah, buffer overrun.
>
> In pgbench.c FD_SETSIZE is redefined to get around the Windows default
> of 64.  But this is done after bringing in winsock2.h (a couple levels
> in as a result of first including postgres_fe.h).  So any fd_set is
> built with an array of 64 descriptors, while pgbench thinks it has 1024
> available to work with.
>
> This was introduced a while back; the multi-threaded patch just makes it
> visible by giving it an important pointer to write over.  Previously it
> would just run over into the loop counter (and probably a couple other
> things) and thus it'd continue on happily with the [sub]set it has.

Yikes.
Yeah, this is fallout from the hacking we did with moving the winsock
includes around a while back. At the time the #defines were added,
winsock came in through the win32.h file :S

> In either case this seems to be a simple fix, to move that #define
> earlier (see pgbench_win32.patch.)

Yes, and it seems to be entirely unrelated to the multithreaded patch.
Thus, applied as a separate patch.

--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/