Forum OpenACS Development: Re: nsdbi, nsdbipg

Collapse
3: Re: nsdbi, nsdbipg (response to 2)
Posted by Gustaf Neumann on
A complete wrapper is not feasible: How to implement "ns_db gethandle" on top of the dbi driver, if there is no handle available at the Tcl level? And how useful will such an emulation be?

Unfortunately OpenACS has quite a wide SQL interface, with sometimes complex interactions (e.g. nested db_foreach with transaction management). Making a bug-compliant replacement is quite hard, some of the used constructs are not recommended for many of the usages but often convenient (db_foreach), so piling layers don't look optimal to me. And i don't know, who would volunteer to make this work.

However, if the wrapper doesn't have to be 100% compliant, some wrappers can be certainly be provided. But in general it looks more ┬┤feasible to me to use higher abstractions and to work towards higher abstractions where these do not exist. For example, the xo::db::sql interface generates on the fly the calls to stored procedures differently for Oracle and PostgreSQL. So, one could easily make use of the dbi interface with little code without loosing oracle compliance. The xo::db::sql interface can be potentially used in many places, but unfortunately, not all of the code in OpenACS uses this interface (essentially just the xo* packages).

I've prototyped a dbi interface for xo::db::sql just now. Here is a quick implementation for permissions, using the xo::db::sql interface.

ad_proc -private ::permission::permission_p_not_cached {
    {-no_cache:boolean}
    {-party_id ""}
    {-object_id:required}
    {-privilege:required}
} {
    does party X have privilege Y on object Z
    @see permission::permission_p
} {
  if { $party_id eq "" } {set party_id [ad_conn user_id]}
  ::xo::db::sql::acs_permission permission_p -object_id $object_id -party_id $party_id -privilege $privilege
}
The definition is about twice as fast as the actual OpenACS implementation on my machine when i am using the dbi interface (the sql prepare statement does not help at the same degree as other places, since the actual call is quite short). Note this this definition is compatible with Oracle as well (no .xql files needed). The only blocker is that we have currently acs-core xo*-free.

Also, many other parts of xotcl-core could be relatively easy developed further to support both the classical and the new dbi interface, at least xowiki and xowf and derivatives would benefit from this.

Another place with a common abstraction and potential larger reuse is db_multirow; many code pieces could benefit if there would be a dbi-compliant version of this function.

Collapse
4: Re: nsdbi, nsdbipg (response to 3)
Posted by Antonio Pisano on
I've read and agree with your considerations.

::xo::db::sql would be just the perfect place to add the new interface, as they would be transparent.

I personally wouldn't put xotcl api into the core, but rather "wrap the wrappable" in a cost-benefit fashion for the core packages and let those who want the best performances "do the leap" to the ::xo api.

Collapse
5: Re: nsdbi, nsdbipg (response to 4)
Posted by Gustaf Neumann on
i'm not sore sure about the wrapping and its expected benefits on the longer term. The closet semantics between db_* and dbi_* have probably dbi_1row/dbi_0or1row and db_1row/db_0or1row. Without a deep look i see the following differences
  • dbi_* has no -dbn, but "-db" in dbi-* is probably wrappable
  • dbi_* has no -cache_key and -cache_pool
  • the option "-bind" is probably sufficient compatible
  • the db_* option "-column_array" is probably sufficient compatible with the differently named "-array" in dbi_*
  • the db_* option "-column_set" is not documented, but reading from the source, this is not implemented in dbi
  • the dbi_* option "-timeout" is not available for nd_db
  • the dbi_* option "-db" is not available for nd_db
  • the dbi_* interface has no .xql support
  • the developer support output is missing for dbi, so
    one needs here as wrapper or some sandwiching;
    wrapper require uplevel-ing for bindvars.
  • dbi* has no full sql statement (with variables substituted), since there is no such thing in dbi
  • Interactions with the db_* transcation management are not clear for me, for reading operations, that should not be an issue worse that the current situation

Many of the mentioned options are seldomly used, the developer support should be done anyhow, some more issues will popup probably.

If one overwrites the two db_* functions by a version based on dbi, this "wrapper" function has to support all features used in any obscure niche in OpenACS. Furthermore, one has to install in the long range both drivers in OpenACS, since the hope to make the code fully compatible is very little. Therefore if one "recommends" to install both drivers, the code becomes harder to maintain and to test.

By moving towards higher abstractions, one has more freedom in the implementation, one can mark lower-level access as deprecated and there is a clear development direction. Also, XOTcl's mixins could be used as an extension mechanism not requiring to overwrite existing functions physically.

Antonio, what are your concerns about using ::xo::db::sql in the core? The interface and implementation is rock-stable, these functions are used in the largest OpenACS sites, xotcl is a requirement of every OpenACS installation, all the contributors of the last two years are knowledgeable in xotcl.

all the best
-gustaf

Collapse
6: Re: nsdbi, nsdbipg (response to 5)
Posted by Antonio Pisano on
I was forgetting some OpenAcs features like background delivery leverage xotcl, so I didn't think xotcl-core was a requirement for the minimal OpenACS installation, but just commonly present. This was one of the concerns.

I fully trust xotcl api, and think it gives advantages to developers, mainly related to flexibility and code reuse, but is also a different paradigm, requiring quite some cognitive load to people used to current codebase (here in Italy, most of the programming I've seen was procedural). Of course, this is not a big problem if changes happen "under the hood" in the api.

This said, it is right to move on when this gives improvement for performances and capabilities. xotcl-core is out there since a while now, so eventually a wider adoption into core codebase needs to be considered.

Collapse
7: Re: nsdbi, nsdbipg (response to 5)
Posted by Gustaf Neumann on
i did some more tests and changed all db-reading operations of a warmed up xowiki instance start page to use partly new wrapper functions using the dbi interface, and using as well the xo::db::sql call for permissions as sketched above.

The total performance improvement as measured with a small instance by the developer support is more than 20%, which is quite good. To make the migration easier, i've extended the nsdbi module; not sure whether all my changes should go into the nsdbi module.

Collapse
8: Re: nsdbi, nsdbipg (response to 7)
Posted by Jim Lynch on
heya Gustaf, curious... when you did the testing, did each iteration include preparing a statement? ordid you prepare a statement (say) at openacs instance startup time, and use that statement?
Collapse
9: Re: nsdbi, nsdbipg (response to 8)
Posted by Gustaf Neumann on
the prepare statements are performed by dbi implicitly and their results are cached by dbi. For the xowiki test most prepares operations (maybe all) were performed after startup, but nevertheless most queries could already use a prepared statements (e.g. permissions). From this point of view, the results are realistic.
Collapse
10: Re: nsdbi, nsdbipg (response to 7)
Posted by Gustaf Neumann on
openacs.org uses now an updated xotcl-core infrastructure, xowiki runs now fully on dbi_*