[Opensim-dev] Solution attempt about code duplication in database plugins

Tue Jan 8 13:02:35 UTC 2008

On Tue, Jan 08, 2008 at 12:41:51AM +0100, Laurent B. wrote:
> Taking a close look at the plugins database, I noticed that the
> persistence logic was replicated in each, which must be difficult to
> maintain, and it may leads to ennoying bugs... That's why I started to
> write a plugin generic database in an attempt to find a solution to
> this problem.

We've definitely gotten here in a very organic way, which has led to a
lot of cruft in the code.  New eyes and new approaches are definitely welcomed.

> It consists of an additional OpenSim.Framework.Data.DbEngine assembly
> which contains the usual classes you know that implements
> IAssetProvider, IGridData, IInventoryData, IUserData. Instead of
> creating ADO.NET Connection, Command objects, theses classes logic
> relies on an IDbEngine helper that creates thoses objects and handle
> queries parameters.
> 
> IDbEngine is an interface that has to be implemented by as many
> classes as desired available database engine. This class helps to
> instantiate ADO.NET objects the best way the specifics of the database
> engine allows to.
>
> As an example of code is better than too much explanations, please
> find AssetData.cs here: http://pastebin.ca/845808 and MSSQLDbEngine
> here: http://pastebin.ca/845813. GridData.cs, InventoryData.cs,
> UserData.cs have also been implemented. Also SQLiteDbEngine.cs is
> ready.

Have you attempted this with the Prims tables yet?  One of the reasons
of the current approach is that once you get 30 columns in a table you
end up having column definition duplication a lot with the approach you
took, which is equally bad in terms of getting code duplication.  The
original sqlite approach was attempting to solve that, but caused other
duplication issues in the process.

> However, in trying to design this plugin, I encountered several
> problems. The first one stucks me a little...
> 
> * The structure of the database differs from a plug to another.  For
> example, there is a field called assets.id in MySQL plugin, although
> it is called assets.uuid in SQLite. The same way, some varchar field
> size are fixed to 255 in all SQLite tables that are generated from
> DataSet. What about to keep a database structure common to all
> plug-ins ? This would be better to keep this new approach simple.

There are enough differences in SQL engine implementations where field
types may need to vary between implementations (though field names
should not).

> * For the moment, there is two different strategies of access to the
> data: one with DataSets implemented, the other with SQL queries. I
> think it's an opportunity to experiment other approaches, knowing that
> the datasets eats a lot of memory when they are well filled, and chews
> up performance when commit is done.  Maybe we could implement stored
> procedures ? The proposed approach will make possible to implement a
> variety of strategies, which could be activated in turn in order to
> choose the most effective for a given database engine.
> 
> That is, if you like the idea, I would love to hear feedback, comments
> and suggestions before to continue to work on it.

The ADO.NET approach was my idea early on to avoid duplication in the
code base.  I consider it a failed experiment, as it didn't give nearly
the amount of benefit as I was expecting, and it definitely through in a
bunch of overhead that wasn't entirely anticipated.

My theories on moving forward with cleaning up the database code is as
follows (these are up for debate, just trying to show my own perspective
here):

1. take another stab at nhibernate.  There were issues with mono when
this was attempted 6 months ago which I'm hoping we can get past.  I'm
going to do some experiments here this week to see if nhibernate will
work for us.

2. Move out the ADO.NET implementations.  The approach we should take is
one similar to the MySQL Assets plugin where the SQL definitions are
done in a seperate resource file.  We can do upgrades via this model as
well.

3. Clean up the storage interfaces.  IRegionStore is really focussed on
the way that DB4O worked (load everything at start) which I think is
less than optimal for many of our objects.

I'm still suffering from jet lag a bit (I appologize for any incoherency
in this email because of it).  Over the next few days I should have my
head fully back in the game to get cranking on this.  I'd love to see
more approaches here like yours Laurent, as I think the more eyes on
this one will help to ensure we've got a good model moving forward.

     -Sean

-- 
__________________________________________________________________

Sean Dague                                       Mid-Hudson Valley
sean at dague dot net                            Linux Users Group
http://dague.net                                 http://mhvlug.org

There is no silver bullet.  Plus, werewolves make better neighbors
than zombies, and they tend to keep the vampire population down.
__________________________________________________________________
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://opensimulator.org/pipermail/opensim-dev/attachments/20080108/3fd1e9dd/attachment-0001.pgp>