Problems with sqlite support with 1.4.8: no such table: uniqueid_temp

[Colin Guthrie]

Jeff Mitchell wrote:
> Hey, you jogged my memory.  Now I know why you get the errors about _temp, 
> although I still have no answers for you about your CPU issue.
> 
> On Monday 25 February 2008, Colin Guthrie wrote:
>> I think it has something to do with playlists.
> 
> It could have something to do with the main Amarok Playlist, but other than 
> that none of them use AFT.
> 
> So here's why you get those _temp errors.
> 
> You said you were doing full rescans.  When the collection scanner is doing an 
> incremental scan it creates temporary tables populated with old information; 
> then it drops the normal tables, scans, and copies the info from the temp 
> tables back (I think I have that sequence right).  However, when doing a full 
> rescan it drops all tables and just scans directly in, it doesn't create 
> temporary tables.  So when the unique ID for a URL is wanted, the function 
> checks to see if scanning is currently happening.  If so, it defaults to 
> looking up the UID in the temp table (because the scan isn't yet complete).  
> If it gets no value back, it then checks the normal table.  Since Amarok 
> prints out these errors from the database what you're seeing is simply a 
> result of checking the temp table first, which may not exist.  It's a totally 
> harmless error.  The reason it checks the temp tables first is because it if 
> you have information about a unique ID in both locations it defaults to the 
> old information in case they've not yet gotten merged.  That was actually 
> based on an old architecture of AFT and is probably not relevant 
> anymore...rethinking the issue, I don't see why it couldn't be flipped.  
> Although you could still see the errors if you're scanning and the new 
> database tables do not yet have that URL value in them (so a better way is if 
> you know a backend-agnostic query to find out whether or not a table 
> exists...).

Hmmm, I'm not 100% sure that's the case to be honest. You know way more
about the code than me so I'm apprehensive to question you but from what
I can tell the _temp tables are only accessed when it's appropriate to
do so:

The error I see is definitely from the CollectionDB::uniqueidFromUrl()
method. See below for a patch which "masks" the error (I wouldn't say
"fix" as there may be other issues).


For what it's worth, I am not touching the GUI at all when doing a full
scan. The GUI is sitting with an active playlist (not playing, just
populated) and the Colletion tab open (but empty if starting with a
blank db)


Re: backend agnostic way to determin if tables exist is difficult. With
Mysql a simple "SHOW TABLES LIKE 'uniqueid_temp" would probably do, for
SQLite you'd have to SELECT COUNT(*) FROM sqlite_temp_master WHERE name
= 'uniqueid_temp'; For PG, I have no idea!

FWIW, tho' the errors we're seeing may look harmless but they are not.
They turn a scan that should take a few minutes into one that takes well
over an hour with very high CPU usage all the time.... (running with
MySQL backend and the whole issue is sidestepped and the scan completes
successfully and quickly).

>> As you are rebuilding it, the scanner runs for a long time. It finds
>> playlists and populates these in the system. Meanwhile the
>> contextbrowser  may try and refresh the it's files in some capacity
>> (perhaps due to the fact playlists are found/updated?) which it does via
>> collectiondb instance.
> 
> No, whatever issue you're seeing, it definitely has nothing to do with the 
> contextbrowser (or non-Playlist playlists for that matter).  Neither have 
> anything to do with AFT.

There is a single call to uniqueIdFromUrl() in metabundle.cpp. It's in
the MetaBundle::setUniqueId() overloaded method which in turn is called
in that format from MetaBundle::setUrl( const KURL &url ) or
MetaBundle::setPath( const QString &path ).

CollectionDB::updateURL() will call MetaBundle::setPath() and
subsequently go on to call doAFTStuff with a flag of false hard coded
with regards to accessing the temporary tables. But in calling
MetaBundle::setPath() before hand, CollectionDB::uniqueIdFromUrl() will
be called and due to the way the calculation of whether to check the
temporary tables is checked, it *will* check the temp tables...

This is me just looking at the code now, but I think this could be the
codepath I'm hitting. I'll verify this later.


> No, but if you are e.g. adding items to the Playlist during the scan, it will 
> read metabundles for those items, causing the uniqueIdForURL function to be 
> called.

Nope. Not touching the GUI, although the scanner is finding lots of
playlists as it scanns and is probably "adding" the tracks to those
Playlists....

>> I do see a QDeepCopy relating to bundles in playlistbrowseritem.cpp and
>> a call to uniqueIdFromUrl() from within metabundle so this looks like a
>> possible candidate as IIRC QDeepCopy is not thread safe?
> 
> No, because this is all going on in the GUI thread.
> 
>> To at least attempt to verify this theory I added a flag to
>> CollectionDB::uniqueIdFromUrl() to ignore the scanning detection and
>> setup metabundle's call to it to use this flag (the only place where I
>> set this flag). Low and behold the scan worked!!!!!!!!!!!!
> 
> Well, then, send the code, man.  :-)

OK, see attached. It's a pretty simple change and probably has knock on
effects that I don't appreciate, but it does avoid the error.

I will try improve it to check to see if I'm hitting the codepath I
highlighted above.


> Have you tried replicating this problem on MySQL or PostgreSQL?  For the 
> record, I'm currently using Amarok's internal sqlite, and I've tried things 
> like adding files to the playlist while a scan is running, and I've not been 
> able to run into issues.

Sorry, I thought I'd mentioned this already, but using MySQL neatly
avoids all this, which is why I think it's a thread related issue with
sqlite.

As I clarified above, I am not manupilating playlists manually when it's
scanning but I do have a lot of m3u/pls files in the collection that are
being interpreted.

I'll get some more debug output for you later as I think I've probably
missed out some of the critical debug text that highlights where the
problem is, some of which (from memory) relates to the
PlaylistReader::doJob() and PlaylistReader::completeJob() methods one of
which has the QDeepCopy of the MetaBundles object and one which
doesn't..... I think this is the main hub of this threading issue!!

Anyway you can't be expected to suddenly solve this so I'll attempt to
get a really good debug dump for you (full output) to help me nail this.

If I can get a reproducible test case (e.g. commands to build up the a
simple collection + playlists that trigger this) then I'll do that too.

Cheers for you help and patience so far!!

Col
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: avoid.patch
URL: <http://mail.kde.org/pipermail/amarok/attachments/20080226/c5e2ecf1/attachment.ksh>