|
From: Cyan O. <cya...@gm...> - 2020-02-11 19:48:07
|
On Tue, Feb 11, 2020 at 6:04 PM Kevin Kenny <kev...@gm...> wrote:
> On Tue, Feb 11, 2020 at 9:32 AM Cyan Ogilvie <cya...@gm...>
> wrote:
> > I've put together a thread-aware connection pooling mechanism for TDBC
> and would like to know if there is interest in merging the changes into
> TDBC itself, and if so, what the process for that would be.
>
> This sounds like a terrific idea. At this point, the process is quite
> informal - we publish 'Informational' TIP's for major API changes, but
> that's about it.
>
> I'm presuming that the 'thread-aware'-ness is at the driver (and
> connection attachment) level? At the higher levels, there are quite a
> lot of Tcl_Obj's about, and the logic is mostly designed as
> 'multi-interp-safe and thread-oblivious'.
>
Yes, mainly - it's the driver's responsibility to ensure that the detach
method and the relevant portions of the constructor's -attach handling are
thread safe, and that the handle returned by detach can use attached from
another thread, but that's about it. The core connection pooling machinery
that's currently in tdbc.tcl is very heavily involved in all the thread
details, but exclusively at a Tcl level using the Thread package, and tsv
variables to hold the detached handles for the pools. To avoid the issue
of moving Tcl_Objs between threads the current design allows moving only
the minimum state (the native database connection handle), although it
would be up to each driver to implement detach and -attach in any way that
makes sense it it. My pure-Tcl postgres driver for instance would just
thread::detach the socket chan and hand the chan name back.
> > The design looks like this:
> >
> > To retrieve a handle from the pool:
> >
> > tdbc::${driver}::connection create db -host localhost -db foo
> >
> > becomes:
> >
> > tdbc::pool $driver create db -host localhost -db foo
> >
> > When the thread is finished with the connection, it instead of
> >
> > db destroy (or db close)
> >
> > it would do:
> >
> > db release
> >
> > which destroys db and any statements and resultsets associated with it,
> rolls back the transaction if one is open, and parks the database handle
> for the connection in a pool of detached connections. The next call to
> tdbc::pool that matches the driver and connection arguments will retrieve
> the most recently released connection instead of creating a new connection
> (making it more of a stack than a pool). This is done to keep the busy set
> of handles to a minimum and allow the idle ones to time out (2 minutes) and
> be closed. This way the pool will grow as needed with a load spike and
> then shrink back down once the spike has passed and idle detached
> connections time out.
>
> Nice, clean, simple. I like it a lot.
>
> I wonder if we could actually override the object destruction in some
> way, or else go through a wrapper object, so that the user of the
> connection would use 'db destroy' or 'rename db {}' in either case. I
> can easily see myself forgetting which pathway to use when finalizing
> a connection!
Very tempting, and I've put comments in the code in the relevant places to
explore this, but I shied away from it for now because it would need a
really-really-destroy method to make the system work - otherwise
connections will never really die, just return to the pool. The background
checks on the pool need to be able to destroy objects to finalise the
underlying database connection (in the case of an idle timeout or a
connection error).
It isn't an error to call close or destroy (or rename to {}) on a handle
retrieved from the pool, it just explicitly closes that connection, which
may be deliberate in situations such as getting an exception on a query
that indicates that the connection is in a bad state (Aurora Serverless
MySQL from Amazon will kill long-running transactions if it wants to scale
the cluster and couldn't find a safe scaling point, and is configured to do
this. In this case the application gets a mysql 1080 exception and that
connection is dead). There isn't any accounting related to a handle that
is issued from the pool to clean up, the "release" method (added by the
pool machinery when it served up the connection) contains the logic to put
the connection back into the pool it came from, but that's all there is. I
could also imagine a higher-level connection pool manager that enforces
things like a maximum number of times to reuse a handle, which would need
to be able to explicitly destroy a connection rather than releasing it).
So both are valid, and currently under the application's control, but I
agree that it would be nice to have the default behaviour for a connection
served from the pool bt to return to the pool under the normal close or
destroy, I just couldn't immediately think of a clean way to do that with
the other requirements.
I'm copying Donal and Sean on this so as to get suggestions from the
> TclOO guru and the architecture astronaut. I could certainly work up
> wrapping logic that would work, but I'm guessing that they will point
> out subtleties that I will miss.
>
> > When tdbc::pool issues a connection from the pool (creating a fresh one
> if needed), and he pool is empty, it initiates a new connection in a
> background thread and adds it to the pool, priming it for the next
> request. The intent of the pooling mechanism is primarily to hide the
> connection setup latency from short-lived users of the db handles, as is
> typical of web server usage patterns.
> >
> > Time to get a handle from the pool on my test machine is between 80 and
> 300 microseconds, vs 7000 to 18000 microseconds without the pool (postgres
> or mysql database on localhost).
>
> Sweet!
>
> Now I'm thinking. Having some sort of wrapper object would be really
> nice, because another potential performance win would be to allow a
> pooled connection to retain its prepared statements. That would be
> trickier, since the Statement object isn't thread-safe at present, but
> most of the stuff in it could be transferred among threads without too
> much baggage. I _think_. No time right now to delve into it, I'm
> afraid.
>
Yes, freezing and thawing statements with the connections would be nice,
but would lead to some complexity in the connection setup process (the
application code getting the handle doesn't know whether the connection
it's received is a new one or previously detached, and so whether there are
associated statements). Some sort of wrapper class for the connections
that are handed out by tdbc::pool could probably address this in a clean
way, but I don't have all the details in sight yet).
> > The detached connections in the pool are periodically checked (every 20
> seconds), and those that are no longer connected to the server or which are
> older than 2 minutes are closed and removed from the pool. If the pool
> size goes to 0 as a result, a new connection is created to prime the pool.
>
> Does this create a blip every 20 seconds on an idle application, where
> a never-used connection is closed and immediately reopened? Is this
> intentional, perhaps to make sure that the server connection is still
> live, or am I misunderstanding what's up here?
>
It does, but the impact is minimal. An idle application will quickly (<=
140 seconds currently) scale down to a pool with a single detached
connection in it. The background poll is very lightweight, essentially:
foreach pool [tsv::array names tdbcPools] {
set driver [lindex $pool 0]
package require tdbc::$driver
set now [clock microseconds]
tsv::lock tdbcPools {
set detachedhandles [tsv::get tdbcPools $pool]
tsv::set tdbcPools $pool {}
}
foreach detachedhandle $detachedhandles {
lassign $detachedhandle handle lastUsed
try {
tdbc::${driver}::connection new -attach $handle
} on ok obj {
if {($now - $lastUsed) / 1e6 > $timeout || ![$obj connected]} {
$obj destroy
} else {
# Still valid - put it back
tsv::lappend tdbcPools $pool [list [$obj detach] $lastUsed]
}
} on error {errmsg options} {}
}
if {[tsv::llength tdbcPools $pool] == 0} {
prime $pool
}
}
So, with the current configuration, an idle application will have a
background thread wake up every 20 seconds, pop a single handle from the
tsv, wrap it in a connection instance, call its connected method (usually a
round trip to the server) and lappend it back onto the tsv. Every 120
seconds or so the connection will hit the idle timeout and be closed and a
new one opened to the server. It's wasteful, but implied by the goal of
the connection pool to hide the latency of creating new connections - it
wants to have an already open and likely valid (no guarantees) connection
to hand to the next caller. Many database servers (like Amazon's Aurora
Serverless versions of MySQL and Postgres) will drop idle connections, so
the periodic check is necessary to avoid always running into that case when
resuming activity after a pause. There is a tdbc::poolDestroy $pool
command to unwind this if the pool is not going to be needed again (soon)
and the application wants to avoid this overhead. On a related point I
would probably want to tweak things so that no background thread is spawned
until the first call of tdbc::pool by an application, so that it doesn't
pay the cost of the thread unless it actually uses the pool mechanism.
> > In order to support connection pooling, a TDBC driver needs 3 things:
> >
> > - Support construction with a single parameter "-attach $handle", which
> should create a new connection instance and attach it to the previously
> detached database handle indicated by $handle:
> >
> > tdbc::${driver}::connection new -attach $handle
> >
> > - A new method "detach" that destroys the connection and returns a
> handle that can be used in the future to reattach to this database
> connection (possibly from another thread):
> >
> > $connection detach
> >
> > - A new method "connected" which verifies that the underlying database
> connection is still valid, returning a boolean:
> >
> > $connection connected
>
> Simple enough!
> >
> > I have implemented these changes for the postgres and mysql drivers,
> using a global Tcl_HashTable protected with a mutex to store the
> ConnectionData struct against the issued handle (with the pidata removed).
>
> Simple enough again. Even an array with linear searching would be
> workable, since we're highly unlikely to be dealing with thousands (or
> even hundreds) of connections in a process.
>
Agreed, and a linear search through an array or linked list would be better
in this case I'm sure, I just did the easy thing :)
> If a driver lacks these additions, the behaviour falls back to detach ->
> destroy, and tdbc::pool will just always construct a fresh connection
> object, so it is backwards and forwards compatible with drivers that don't
> know about the connection pooling.
>
> Perfect!
>
> ODBC would be easy enough to add. There's already a global mutex in
> the driver because the SQLENV (needed for connection construction and
> a few other bits) is per-process. With SQLite3 being entirely
> in-process, and with the fact that we have all the overhead coming
> from the fact that its driver is written in Tcl atop an existing Tcl
> API, it's probably a game not worth the candle. The native Oracle
> driver is moribund, and the only other driver I'm aware of is Steve
> Redler's network-proxy thing. You've done the hard part.
>
Yeah, I left SQLite3 alone because I doubt there is any win doing this
dance just to avoid the database open call, and is probably quite
challenging since the "underlying database connection" in this case is the
sqlite3 tcl command for the open database and its associated state. It
might be interesting to investigate but my intuition suggests that without
an expensive connection setup there isn't much to gain.
ODBC looks like it would be almost identical to the mysql and postgres
drivers, just the Tcl_Obj* connectionString member of the ConnectionData
might need care when detaching from an interp. I don't have an odbc
datasource to test against currently though, and I'm not that familiar with
it in general.
>
> > I have a basic set of test cases for the changes in the postgres and
> mysql drivers, and the connection pool machinery itself (currently
> implemented in the TDBC core package, in Tcl code). If this feature is
> something that could be merged, then I'll write the docs and try to write
> test cases for the more pathological race conditions.
>
> > It currently supports vanilla Tcl using the Thread package, and
> NaviServer using it's own threading (because thread::create interacts badly
> NaviServer during certain parts of the server startup). That means there
> is some polyfill code to provide thread::send when using NaviServer's
> threads, which lack that capability (using [chan pipe]). Since my
> motivation for this connection pooling mechanism is to replace ns_db in
> NaviServer for rubylane.com, it's important to me that it works in
> NaviServer, but the polyfill may be too grubby and specific for TDBC
> generally. It could be merged with just Thread support and I'll just
> maintain the polyfill for NaviServer separately for our use.
>
> Fraser's Law: All the world's problems are solved with one more layer
> of indirection.
>
> I'd be fine separating the communication interface from
> implementation, and if we choose not to ship the NaviServer plugin,
> the API will still be there to support it. (The Notifier in 8.4-8.6
> was a similar decision: it was implemented at C level with an abstract
> API [a structure of function pointers], and only one implementation
> shipped per platform. Browser plugins replaced the implementation as
> part of their initialization.
>
> That's kind of what OO is for, isn't it?
>
> --
> 73 de ke9tv/2, Kevin
>
|