GNOME.org
 

Re: [Vala] sqlheavy?

Jim,


Thanks, this is good information.

I'm going to reply in-line people who are interested, but here is the
TL;DR version (of this whole thread, really):  SQLHeavy currently has
issues with parallelism.  If you need parallel execution of queries,
SQLHeavy is currently as big of a pain as SQLite.  If you just want an
API for SQLite which easier to use, or if you just want to avoid
blocking the UI (the original use case for the async stuff), SQLHeavy
may be a good fit.

To anyone who is reading this at some point in the future:  if there is
an SQLHeavy >= 0.2 release, the issues with parallelism have been
resolved.


On Mon, 2012-07-02 at 20:07 -0700, Jim Nelson wrote:

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.


You're absolutely right about this, though I'd like to emphasize the
distinction between "async" and "parallel".  Geary's problems with
SQLHeavy come mostly from trying to use the async interfaces to achieve
parallelism, as opposed to simply not blocking the UI thread (which was
the original purpose of the async API).

Most of the issues below are actually related pretty closely under the
hood and the proper fix is really to separate the connection from the
database so you can transparently execute multiple queries, which
completely replaces the existing async support (as well as the
synchronous support).  Some of these issues are probably resolvable
within the existing code base, but it would take a lot of time and only
mostly work.  I would rather use that time to get the proper fix in
place.

For anything which is missing a response, just imagine I wrote something
like "The right solution for this issue relies on the parallel execution
stuff."  I think that actually covers most of this message.


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.


The ORM stuff is, AFAIK, fully functional.  It is a bit limited since I
don't actually have a lot of experience with ORM (and no experience that
I enjoyed)...  I had hoped people who actually liked ORM would have some
ideas about how to make it a bit more full-featured, but nobody has come
up with anything so far (and now even if they did I don't really have
time to implement them anymore).

What *is* a problem is the sqlheavy-gen-orm tool (which generates Vala
source code which uses the ORM stuff in the library).  Though it will
live on in the 0.1 branch for all eternity, I'm removing it in master
(which will become 0.2 when I'm done with the parallel execution stuff).


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.


Actually SQLite is serializing access anyways, so from a performance
perspective it probably isn't a big deal.  That's how SQLite deals with
concurrency unless you create multiple Sqlite.Database objects.  See
http://www.sqlite.org/threadsafe.html


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 reason I've been dragging my feet on this one is that using
CANCELLED would actually require an API break.  Since Vala has typed
exceptions, everything would have to be changed to "throws
SQLHeavy.Error, GLib.Error", and valac would start emitting warnings
everywhere for existing code about uncaught exceptions.

My current plan is to just get rid of typed exceptions and just throw
GLib.Error everywhere, like Bump is doing.  I just don't think it's an
API break is justified just for this, so I'm planning on combining it
with the parallel execution stuff.  This will actually be the biggest
part of the API break--almost everything else should just work.


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.


I think these two are actually pretty easy to fix... I can take care of
them over the weekend if that would help you.


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.


Thanks, but I disagree.  Once I have the new engine in place it will be
a solid foundation.  Right now it's just a thin wrapper on top of SQLite
(which was the original purpose of the library).


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.


The Yorba team has been very patient, and I know they don't expect me to
drop everything to work on this.  I certainly didn't mean to imply
anything else, and if I did I apologize.

I'm pretty sure nobody is exercising the async code paths as hard as
Geary (at least not for parallelism), and frankly you've pushed it well
past what it was originally intended to do, which explains the issues.
I would love to support these uses, and I have plans that will allow
SQLHeavy to do so, but the current code simply isn't capable of that.


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.


Sorry, I didn't mean to suggest someone from Yorba should be helping out
with these issues.  They are clearly with SQLHeavy and not your
responsibility to fix, and I certainly don't expect you to be
responsible for the database/connection split (which is basically a
rewrite of the SQLHeavy innards).

I was merely suggesting that if the issue was about making releases and
you didn't feel I was responsive enough last time you asked for one then
I would be happy to allow one of the Yorba people to do that without my
intervention.


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.


All true.  I didn't end up actually including any of that code in Bump
due to architectural differences but it really helped me flesh out the
API, as did those emails.  Furthermore, a lot of the things I'm working
on for SQLHeavy are a result of Geary pushing SQLHeavy to do more.  In
short, Yorba, and Jim in particular, have been tremendously helpful.


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
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]