Hi all,
I took some time after getting bored being stuck at home sick, to poke the
boundary benchmark a little more. I was disappointed with Go's
representation in the test as well as the numbers displayed overall. In my
experience both platforms should perform better, but in Go's case, there
were "first 5 minutes" indicators of restricting factors. It turns out that
Go's apparent poor performance was due to an innocent hardcoded value in
database/sql. The most minimal patch conceived simply changes the value
from 2 to 8 or 10, and most of the performance that can be attained then
will be. In fact, against tip, I was able to take it much higher than that
(with a tuned postgresql.conf), to hundreds of connections. Be thankful
it's using a pure Go database driver.
Apologies for the slightly inflammatory subject line. It's just for
entertainment.
The real reason for this mail, prior to submitting the patch, is that the
specific approach in the patch could come under some discussion. The
attached diff is probably the "minimal sane" patch that could be applied,
but I have a few items worth discussing. If little discussion comes of it,
I will submit this patch for now, and consider sending additional work if I
can make the time for it. As I am new to this community, I don't yet know
what style of patches are desired. If incremental, sane but incidental
changes such as the attached patch are acceptable, I will start pushing
them through.
Here are my own review thoughts on the patch and surrounding issues:
- The current code defaults to 2 "idle connections". In reality this
equates to 2 "active connections" for various reasons. This default value
is continued only such that users would not be subject to unexpected
semantic changes.
- There is no maximum limit implemented. While not the subject of this
problem or patch, this should be implemented.
- The immediate close behavior (something I am tempted to consider a
bug) has not been altered. This point may become more of a philosophical
discussion. In my experience, it's better to handle bursts with grace where
possible. If this discussion becomes lit, it may be best to split it to
another thread.
- Related to the immediate close behavior, with or without changing that
aspect, I feel that the pool could be rewritten with channels and
goroutines. An almost example lives here
(https://github.com/eventmachine/eventmachine/blob/master/lib/em/pool.rb)
(for those of you with very high attention to detail: yes, that contains
some races that are easier to eradicate in a Go style environment).
- If a channel approach is not taken, it may yet still make sense to at
least move the mutex in database/sql to a RW variant with a persistent
store of Conn objects and a free / use list. This should allow
simplification of the Close anti-race logic. "defer" helps with this stuff,
but, it's not showing itself to be a magic bullet.
- The second comment on Open suggests that drivers will implement an
Open that returns a *DB. This is no longer the case, and this comment
should be removed (if I submit this patch, should I include that or add it
to a second patch - that's the common approach in this community?).
- Given the aforementioned comment on sql.Open, it's not entirely clear
if DB instances should always be created via sql.Open directly. If they
are, then the patch is correct setting maxIdleConns during construction of
the struct in sql.Open, if not, then more work is required to handle the
default in MaxIdleConns() where a value of 0 will be treated as 2, if
bullet 1 is to remain. This latter approach would be unfortunate, because
there are some real world use cases for 0 being a meaningful value.
- It might be useful separately from a maximum value, although related,
to make the implementation act more like a pool (at least optionally).
Right now driver.Conn.Open() is left with the only recourse of panic when a
database server is for example, run out of socket resources. It seems that
Go has all the ability to create this scenario with ease.
On to the Good News:
*
*
The attached patch, as imperfect as it may be, does take the Go
implementation of Collins Go-And-Java test[1] far beyond the Java
implementation. You can find my fork on github[2], and some specific
results in the README.md[3]
16kr/s for Go, and 10kr/s for Java. I'm sure the Java version could be
optimized at least somewhat. Both applications pretty much hose the CPU of
my now quite warm Macbook Pro. The remote side (executing wrk(1)) was a mac
mini with the machines attached between a Gigabit network running MTU 9000.
There are a few more details in the README, but I'm sure I missed relevant
items. The next bottleneck appears to be context switching - I was sharing
the application machine with the Postgresql forks, and the test was
generating more than 50% sys load under tension.
Anyone should be happy with these results (including the Java/Dropwizard
results). They're very good, and clearly demonstrate that for any generally
normal real world application, the infrastructure and libraries in use will
certainly not be the cause of any statistically significant latency.
The last round of tests, on which this numbers are based, came from
"f4a92d7eeb8d" with the attached patch applied.
Thanks,
James
[1]
http://boundary.com/blog/2012/09/17/comparing-go-and-java-part-2-performance/
[2] https://github.com/raggi/go-and-java
[3] https://github.com/raggi/go-and-java/blob/master/README.md#example-results
--
