[Twisted-web] /freeform_post!!random causes exceptions

[Andrea Arcangeli]

On Thu, Jan 13, 2005 at 01:32:51PM -0600, J Turner wrote:
> On Mon, Jan 10, 2005 at 06:03:40PM +0100, Andrea Arcangeli wrote:
> > I
> > don't know about the postgres and the other bits you mention (so far I
> > tried pgasync but it's not working correctly and its interface is
> > inefficient since it requires duplicating lots of code so at the very
> > least that needs fixing, while psycopg2 rocks).
> 
> What does that mean?  How is the interface inefficient?  How do you have
> to duplicate lots of code?  I'm not disagreeing, I'm trying to draw out
> more concrete criticism.

Well the code, I checked out svn a few days ago was like this, quite
different from the below more recent one ;).

		def finish(dc,d,cursor):
			cursor.release()
			request = ctx.locate(inevow.IRequest)
			request.setComponent(iformless.IRedirectAfterPost,"/%s" % IPageTitle(ctx))
			d.callback(None)
			^^^^^^^^^^^^^^^^


		d = Deferred()
		^^^^^^^^^^^^^^
		connection = pgasync.connect(**WIKI_DB_ARGS)
		^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

You must have changed quite some bits in the meantime, thanks.

> Before you answer, you should know that I've released quite a few versions
> recently enchancing the api/workflow area.  In the latest version, full
> queuing happens.  So this is ok:
> 
> --- 
> conn = pgasync.connect(**DBARGS)
> cursor = conn.cursor()
> cursor.execute("insert into ...." ,{"blah"  : "toast"})
> conn.commit()
> cursor.release()
> ---
> 
> No callbacks required.  Everything queued, including commands issued
> before the *real* connection is made.  Only add a callback when you care
> about the outcome.  (or to add an errBack, etc).

So it sounds like you fixed it meanwhile.

Still I'm not convinced creating a new connection (even if you
internally do pooling) is the right API.  I'm also not sure if you've a
maximum number of open connections. The thread limited the number of
connections. I don't dislike having a max number too (but of course it's
a very minor feature).

The part I like less in the API is the pgasync.connect(**DBARGS) with
DBARGS being a global (especially that a gloabl with username and pwd
inside doesn't look very nice IMHO ;). But I can fix this on my side by
hiding it in my own sql_class, but personally I prefer the idea of
passing the username and password to a "pooling class" during startup,
and then to do the operations at runtime on the pooling class.

> But feel free to offer suggestions, I'm all about improving this.

My suggestion is to make it backwards compatible with adbapi.  Isn't
that technically possible or am I missing something? I'd just like to
replace adbapi with pgasync and be done with it.

An API already exists, it's fine if you extend it to provide more
features and more finegrined access to the data, but I don't see why you
can't provide a backwards compatible API, which is also a nicer API IMHO.

If you can make it backwards compatible you've a chance to increase the
userbase quickly IMHO.

What about the type covnversions, does it gets everything right
automatically, like 'false' -> False, numeric -> decimal integer -> int
etc..?

> When all is said and done, pgasync has, in every test I've tried, shown
> to be *much* faster than adbapi/threads/synclib in an async environment
> under heavy load. 

Most probably this is the removal of the futex locks (at least in
linux), I've run an strace on the main twisted thread, when it floods
data, and it's doing nothing but futex locking. Perhaps that could be
optimized a bit on the threaded version too. But I agree for a 2-way
pgasync is probably nicer in performance terms.