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

[Hugo Beauzée-Luyssen]

On Tue, Jun 26, 2018, at 10:37 AM, Rémi Denis-Courmont wrote:
> 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.
> 

That's the goal yes.

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

I think we agree, yes.

> > > - 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
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr