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

[Colin Guthrie]

Jeff Mitchell wrote:
>> 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).
>>   
> So, if you want to know why that method is getting called, it's in 
> ScanController::startElement().  There's a line:
> 
> bundle.setPath ( attrs.value( "path" ) );

I've done some more testing and I don't think it's that call that's
getting it, but rather the PlaylistFile::loadM3u()....

I put in some debug (see attached patch) into the loadM3u() and
triggered the error with my known test case. I got my debug output
immediatelly before the errors started. See debug.1.txt.

It clearly shows a new sqlite connection being created immediately
before the error. This makes sense as this connection will not be able
to access the temporary tables.



I then added the flags to MediaBundle::setUrl() and setPath() and set
them to true in PlaylistFile::loadM3u() (the very last hunk in the
attached patch) and reran my test case. The error did not happen. See
debug.2.txt.

Quite why you don't see the bug I don't know, but this seems a pretty
clear scenario now.

So I guess the PlaylistLoader is in a different thread and apparently
creates it's own CollectionDb instance but shares the ScanController
instance.


Going back you your comments:
> When a MetaBundle gets a new path set, it checks to find out if there is 
> an existing unique id for that path, and if so, fetches it.  In 
> metabundle.cpp you can see that setPath calls setUniqueId (with no 
> arguments) which is basically a two-line job that checks if it's a local 
> file and if so it calls that function in collectiondb (you saw this when 
> creating your patch).

Indeed yes. :)

> That right there is the *why* of the function getting called during the 
> scan.
> 
> Now, your patch is essentially making the issue, so let's look at what 
> could be the problem here.
> 
> 1) ScanController::instance()->tablesCreated() is returning true when it 
> shouldn't.  The ScanController runs in its own thread, so it's possible 
> that the tables are deleted during accessing of m_tablesCreated by that 
> function, which then returns.  In that case you could theoretically 
> attempt to access uniqueid_temp when you shouldn't, but it certainly 
> shouldn't be for every entry in the database.  But maybe on your system, 
> for some reason, this function is locking up (as it's a simple accessor 
> I don't see how this would be the case).

I'm not 100% sure if this is relevant now considering the above.

It's the PlaylistLoader that's ultimately making the call, and this is
perhaps in a different thread context.
> 
> As for testing this out, change your code to the following:
>  
>     bool throwawaybool = ScanController::instance() && 
> ScanController::instance()->tablesCreated();
>     bool scanning = false;
>     if (!ignorescanning)
>         scanning = ( ScanController::instance() && 
> ScanController::instance()->tablesCreated() );
> 
> That way you'll always run tablesCreated(), and can see if that is 
> causing the lockup.

This didn't make any odds so I think this is not the issue.

> 2) sqlite doesn't like accessing tables that don't exist.  For fun, I 
> opened up my collection.db with sqlite3 and attempted, using the up 
> arrow and enter, to enter "select * from uniqueid_temp;" as fast as I 
> possibly could.  You may not be surprised to learn that I didn't have 
> any luck locking up my system, but maybe it'd be worth you trying it 
> too.  What I would do is try building Amarok with external sqlite, and 
> see if you see the same issue.  If you do, close Amarok and open up the 
> collection.db with sqlite and try that select command on the 
> non-existent table, and see if sqlite is hanging for some reason.

I don't think it's a problem with hangging. Basically I think the
problme is that due to the other sqlite connection triggered by the
playlistloader, the scanner no longer has completely free access to the
db (although I don't see the busy loop debug kicking in).

Still the query will fail for every file in every playlist found and
thus that's a fair few pointless queries in my case which probably
accounts for the slowdown.


>> 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 ).
>>   
> Yep.

Which I can now further confirm is called from the Playlistloader which
is where this whole issue apparently stems from.

>> 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.
>>   
> Yep.  Didn't see your analysis here before I wrote my text above :-)

This appears to be a red hering. That particular instance is left
unhacked in my latest attached patch.

> At this point, barring your experiences from my two suggestions above, 
> I'm pretty confident saying that either: you are looking at the wrong 
> place (reading the playlists in your collection are in totally different 
> code from here and have nothing to do with AFT) and are mistaking your 
> freezing issue with AFT because of the sqlite errors you see; have some 
> weird sqlite issue that is being exposed both in uniqueIdFromUrl as well 
> as somewhere in the playlist reading code; or both.

It should be clear from the attached patch what the codepath is:
1. PlaylistLoader::loadM3U()
2. MetaBundle::setPath/Url()
3. MetaBundle::setUniqueId()
4. CollectionDB::uniqueIdFromUrl()

You say above that the reading of playlists is in a totally different
bit of the code but the above chain seems to connect it to AFT....

> I'd start looking into more esoteric things.  What version of gcc are 
> you using, what optimization flags are you using, etc.  What version of 
> gcc/optimization flags were used for your sqlite build.  Etc.

-O2 for optimisation is standard in Mandriva IIRC... but when building
from SVN checkout I just ran configure and that resulted in:

 -Wno-long-long -Wundef -ansi -D_XOPEN_SOURCE=500 -D_BSD_SOURCE
-Wcast-align -Wchar-subscripts -Wall -W -Wpointer-arith -fno-builtin -g3
-fno-inline -Wformat-security -Wmissing-format-attribute
-Wno-non-virtual-dtor -fno-exceptions -fno-check-new -fno-common
-DQT_CLEAN_NAMESPACE -DQT_NO_ASCII_CAST -DQT_NO_STL -DQT_NO_COMPAT
-DQT_NO_TRANSLATION  -avoid-version -module -no-undefined
-Wl,--no-undefined -Wl,--allow-shlib-undefined -R

$ gcc --version
gcc (GCC) 4.2.2 20071128 (prerelease) (4.2.2-2mdv2008.1)


Like I say the attached patch "fixes things" for me. It would be
interesting to see a scan log from another machine with a large
collection and some playlists (although I think a critical part is
deleting the playlists before doing the scan as IIRC playlists
themselves are stored in the config file not the db and thus are loaded
when amarok starts, before the scanner runs and doesn't create the
sequence of events that trigger this).


I really appreciate the time you're spending on this. I hope it does
turn out to be a bug and not just user support ;)

Col

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: debug.1.txt
URL: <http://mail.kde.org/pipermail/amarok/attachments/20080226/29468c48/attachment.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: debug.2.txt
URL: <http://mail.kde.org/pipermail/amarok/attachments/20080226/29468c48/attachment-0001.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: playlist.patch
URL: <http://mail.kde.org/pipermail/amarok/attachments/20080226/29468c48/attachment.ksh>