[vlc-devel] [RFC PATCH 1/8] contrib: Update & reactivate sqlite

[Rémi Denis-Courmont]

Le mardi 26 juin 2018, 14:45:51 EEST Hugo Beauzée-Luyssen a écrit :
> On Tue, Jun 26, 2018, at 4:29 AM, Rémi Denis-Courmont wrote:
> > Hmm, my understanding of SQLite locking is probably out of date, and it
> > seems that the  analysis depends on the SQLite method used (e.g. WAL).
> > 
> > But in any case, using SQLite, or really any read/write storage in VLC
> > poses certain potential problems:
> > - users with network mounted home directories
> 
> I suppose if that folder is not available the medialibrary initialization
> would fail, and that case must be handled gracefully (ie. not like the
> current temporary initialization) I suppose that would be similar to
> launching VLC with --no-media-library.

Yes. At least provided the player is still practically usable without ML.

> Should the folder be unmounted
> during runtime, the medialibrary can fail gracefully as well, and would
> restore any potential write operation during startup.

That should be impossible if you keep an open handles to at least one of the 
DB files.

> > - VLC crashing during an update
> 
> My understanding is that SQLite's journal covers this

My understanding is that it depends on the mode that SQLite is set to use.

> > - VLC freezing during an update
> 
> I'm not sure there's something to handle there, as far as the medialibrary
> is concerned

I mean, you don't want the concurrent or consequent VLC instances to fail, 
e.g. waiting for a lock that will never be released.

> > - writes in one VLC process desynching data in another VLC process (e.g.
> > deleting or updating an entry)
> 
> That could be an issue indeed. SQLite would return SQLITE_BUSY, it's up to
> us to determine if we want to retry, and if so for how long, or if we want
> to do something a bit more clever by communicating across VLC processes, or
> limitating the ML instances to a single VLC process

I would expect the busy error on conflicting transactions. That's certainly 
something that needs to be handled gracefully.

But my main concern is what do we do if an entry changes after it has been 
read. From SQL point of view, there is probably no conflict. The conflict is 
higher level.

> > - storage space accumulation / check-pointing
> 
> I don't think that's something specific to sqlite or any database for that
> matter.

None of the issues are specific to SQLite. That's not an excuse for not 
addressing them.

> > - database layout changes upon software updates
> 
> This is handled in the medialibrary and tested by quite a few VLC for
> Android updates
> > - I/O error handling
> 
> In case of reading, we retry for some time and eventually give up. For
> writing, it's a bit trickier and depends on the failing write. We handle
> quite a few cases already, but I can't swear all of them are. The code
> infrastructure to handle such errors is present, for what it's worth, and
> at the end of the day, I'm not sure we can do much if a write to disk
> fails, except for not crashing & retrying later

IMO, retrying is kinda pointless because it's unlikely that it will work, and 
even if it does, it might make things worse rather than better (e.g. keep 
filling the disk). All you can do is forget the data that was not written. The 
real difficulty is ensuring that the software does not end up in inconsistent 
state.

-- 
Rémi Denis-Courmont