Re: [Vala] sqlheavy?

[Jim Nelson <jim yorba org>]

Let me explain the situation with Geary and SQLHeavy.  The ticket Eric
referred to earlier does not fully describe our issues.  (Warning:
this is *long*.)

First, I have to correct what Eric said earlier.  It's not that
SQLHeavy isn't up-to-date, rather, the features Geary most heavily
relies upon are not as well- or fully-implemented as we need.
Specifically, the Geary engine is a fully asynchronous library
designed to handle multiple requests at once.  Virtually all our
interaction with SQLHeavy is through its asynchronous interfaces.
That was the strongest motivation toward using it rather than coding
directly to SQLite.  However, during development we discovered serious
flaws in SQLHeavy's async implementation.

But Evan's right, as far as using it as a direct replacement for
SQLite, it seems as full-featured as one would want it to be.  Perhaps
the ORM stuff isn't up to snuff, as Evan said, so people wanting to
adopt SQLHeavy should consider that.  I would also examine the bug
database (found at http://code.google.com/p/sqlheavy/issues/list)
before jumping in with both feet.

What's motivating our decision to move away from SQLHeavy are the following:

* Each asynchronous operation creates and destroys a thread:
http://code.google.com/p/sqlheavy/issues/detail?id=11

When I say operation, I don't mean transaction, I mean each
*operation*.  Thus, if your QueryResult has 50 items in its set, 50
threads are created and destroyed while iterating the result.  1000
items, 1000 threads.  This is a major performance hit, especially at
startup, as Geary does a lot of database work while connecting to the
server.  It's painful to run Geary under gdb because of this problem.
Every time I go into Geary to tune or optimize, this problem is in the
mix affecting performance.

I reported this bug a year ago, almost to the day.

* Async transactions aren't asynchronous:
http://code.google.com/p/sqlheavy/issues/detail?id=12

This was almost a showstopper for us.  Without diving in too deep to
the mechanics of this one, Geary uses transactions throughout its
database layer to guarantee atomicity.  SQLHeavy's async transactions
can't make that guarantee.  I didn't discover this until gobs of
database code had already been written.  The only short-term solution
I could find meant, in essence, there's a master lock on Geary's
database that guarantees only one block of async code may perform
operations at a time.  In other words, we're serializing database
access.  It's an ugly hack, but I coded it thinking the problem would
be solved shortly, at which time I could rip it out.

This bug is also almost a year old.

* Cancelling an async operation doesn't cancel the scheduled thread:
http://code.google.com/p/sqlheavy/issues/detail?id=17
* Use IOError.CANCELLED: http://code.google.com/p/sqlheavy/issues/detail?id=18
* QueryResult.finished doesn't respect Cancellable:
http://code.google.com/p/sqlheavy/issues/detail?id=19
* next_async() can segfault if cancelled:
http://code.google.com/p/sqlheavy/issues/detail?id=20

These are related in an oblique way.  The first one hurts performance
because there might be a lot of I/O against the database when the user
switches Folders, causing all of it to be cancelled at once.  Because
the scheduled threads are not cancelled, the fresh, relevant I/O the
user has requested is queued up waiting for all those threads ahead of
it to die.

The second bug is not as serious, but it means that callers to the
Geary engine had to do special-case checking for two different types
of exceptions rather than one (since CANCELLED is ok if the caller
did, indeed, cancel the operation).  I had to work around this to
convert SQLHeavy's INTERRUPTED error to CANCELLED.  This check has to
happen throughout Geary's database code.

The third and fourth required checking Cancellable.is_cancelled()
everywhere in our database code.  Again, a utility method that must be
called from a lot of places.

These three problems required me to go into our database layer and add
a lot of special checking to correct and work around them.  These were
reported 6 to 8 months ago.

* VersionedDatabase upgrade scripts don't run:
http://code.google.com/p/sqlheavy/issues/detail?id=21

This is one of those features I was really excited about, so I was
pretty disappointed when it didn't work.  In Shotwell, we do all our
database upgrades through manual code.  I thought it was a great idea
to handle it all through SQL scripts.  But it looks like an internal
SQLHeavy lock prevents the upgrade scripts from running, causing the
app to hang at startup.

We worked around this by implementing our own upgrade path.

I want to make it clear: Evan is fulfilling a major need in the GNOME
community with SQLHeavy.  The alternative (GNOME-DB) is unpalatable
for an app that needs an in-proc database and knows it's going to use
SQLite.  He's done a ton of work and I think it's a solid foundation
for the future.

I also don't think Evan's "responsible" to solve these bugs for us.
He's doing this in his free time, he's not being paid, and he's
provided quite a lot of usable code that's working for others.  I
honestly wonder if anyone else out there is exercising the async code
paths as hard as Geary.  The expectations on my end are somewhat low.
I only mention the time we've waited on these bugs to emphasize that
we're not being capricious.

But we do have our requirements and deadlines and we need a proper
solution for our issues.  The workarounds we've put in place make
maintaining and expanding the database layer difficult and unnatural.
Our local performance is severely under par.  That puts a drain on the
application as a whole.

Evan suggested someone from Yorba help out with these problems.  I've
looked at the async code quite a bit, and to be honest, I don't think
it's one week worth of work on my part.  Evan's familiar with the
code, perhaps he can do it in one week, I won't speak for him.  But it
looks to me what's needed is a complete rewrite of the thread,
locking, and maybe even connection model, from the ground up.  The
problem is architectural and not in implementation.  My experience is
that sort of task is best suited for the project lead, not an outside
contributer coming in to scratch an itch.  It would be great if
someone stepped up to help Evan, but I think that person needs to be
dedicated to the task for quite a while.

I'll point out that I offered to patch the thread creation/destruction
problem last year using a ThreadPool, but Evan himself said the whole
thing needed a rewrite, so I backed away.  (Reviewing the code later,
I realized the problem was more fundamental than simply adding a
ThreadPool.)  Also, I did make code from Geary available to Evan for
Bump.  I don't know how much of that made it in, but it's there for
the taking.  In fact, the idea from Bump came from a private email
discussion between me and Evan.

I've already coded a thin wrapper around SQLite and built a simple
thread/connection/transaction model for Geary's async needs.  That
wasn't the hard part; I coded the wrapper in less than a day, the
async stuff in about a day more.  The bulk of the work is moving the
existing database layer to this new model.  This is in a private repo
and hasn't been fully tested yet, but it should solve all the issues
described above and make Geary's database layer easier to maintain.

I know this is long-winded, I wanted to be complete.  There's been
some discussion in the GNOME community lately about transparency in
design decisions.  I realized I hadn't verbalized my thoughts about
SQLHeavy on our Redmine ticket, so I'm doing it here.

-- Jim

On Mon, Jul 2, 2012 at 4:24 PM, Evan Nemerson <evan coeus-group com> wrote:
> On Mon, 2012-07-02 at 11:22 -0700, Eric Gregory wrote:
> > On Thu, Jun 21, 2012 at 9:44 AM, Jonas Kulla <nyocurio googlemail com>wrote:
> >
> > > I assume it's pretty up to date.
> >
> > Sadly that's not the case, which is why we're in the process of removing
> > SQLHeavy from Geary.
>
> I don't really understand this.  First, based on
> http://redmine.yorba.org/issues/5034 it sounds like that's not the
> primary reason.  If it were the issue, it would be trivial to resolve.
> There isn't really anything that needs to be done to keep SQLHeavy up to
> date, since SQLite isn't exactly exploding with new features.  If you
> just want new tarballs released more regularly that's really not a
> problem--I released one last time you guys requested it and I can do it
> again.  The other option is that I can just add someone from Yorba to
> the list of maintainers and you're welcome to make releases whenever you
> want.  Removing the sqlheavy-gen-orm utility would make this even more
> simple (and likely unnecessary), since it would remove the dependency on
> libvala.
>
> The text of that bug leads me to believe that the primary reason is that
> "... it's not clear that it will ever support concurrent database access
> from multiple threads...".  That's actually not completely true--you can
> still create multiple Database objects pointed to the same file on disk
> (just like in SQLite).  What is missing is a layer to have
> SQLHeavy.Datbase automatically create multiple connections to a single
> database in order to support concurrent queries transparently.  That's
> not a matter of keeping things up to date, that's a *major* new feature
> which has basically necessitated the creation of a whole new library
> (http://code.google.com/p/bump) as well as a rewrite of the SQLHeavy
> internals.
>
> I would be disappointed to see Geary move away from SQLHeavy, but if you
> have an alternative which works I certainly can't fault you for
> migrating to it.  I think I need about a week of full-time work in order
> to finish the database/connection split, and since I'm very busy at work
> these days (plus other open source stuff) I don't know when I'll have
> that kind of time.  Until I do, SQLHeavy is in basically the same
> situation as SQLite, which isn't a problem for the majority of users
> (including the software I created SQLHeavy for in the first place).
>
>
> -Evan
>
> _______________________________________________
> vala-list mailing list
> vala-list gnome org
> https://mail.gnome.org/mailman/listinfo/vala-list