Adding bookmarks concurrently doesn't work

[yonas]

Data

• Shiori version: 1.6.0.r.1
• Database Engine: n/a
• Operating system: FreeBSD 13.2
• CLI/Web interface/Web Extension: n/a

Describe the bug / actual behavior

Adding bookmarks concurrently doesn't work.

Expected behavior

There should be a mutex / lock that allows the first client to write to the database, while the remaining wait / block.

To Reproduce

$ fish
$ cat urls
google.com                                                                                                                                                                                                           
youtube.com                                                                                                                                                                                                          
facebook.com                                                                                                                                                                                                         
instagram.com                                                                                                                                                                                                        
twitter.com                                                                                                                                                                                                          
baidu.com                                                                                                                                                                                                            
wikipedia.org                                                                                                                                                                                                        
yahoo.com                                                                                                                                                                                                            
yandex.ru
whatsapp.com
xvideos.com
amazon.com
pornhub.com
xnxx.com
tiktok.com
yahoo.co.jp
live.com

$ for u in (cat urls) ; shiori add --offline https://$u & ; end

2023/09/20 19:23:40 error during commit: sql: transaction has already been committed or rolled back                                                                                                                  
Failed to save bookmark: database is locked (5) (SQLITE_BUSY)                                                                                                                                                        
2023/09/20 19:23:40 error during commit: sql: transaction has already been committed or rolled back                                                                                                                  
Failed to save bookmark: database is locked (5) (SQLITE_BUSY)
2023/09/20 19:23:40 error during commit: sql: transaction has already been committed or rolled back
Failed to save bookmark: database is locked (5) (SQLITE_BUSY)
FATA[2023-09-20T19:23:40-04:00] Error running migration                       error="database is locked (5) (SQLITE_BUSY)"

Notes

SQLite WAL

[rutkai]

WAL doesn't seem to solve the problem even with shared cache enabled.

I tried sqlx.ConnectContext(ctx, "sqlite", databasePath+"?_journal_mode=WAL&cache=shared") but it still shows error messages (though less often).

A solution can be to throttle the queries on application level and block execution if the database is locked... but would it be an appropriate solution?

[Mxrk]

Would a busy timeout on the connection help?

https://www.sqlite.org/c3ref/busy_timeout.html

[rutkai]

I tried it, the only difference was that it took 5 sec (with timeout 5000) to throw these errors.

I played with shared cache, tx lock, sync, mutex, etc., actually all parameters that make sense and couldn't bring it to a stable state. Somebody may be luckier with the parameters but I don't think that it'd work with sqlite (which is not designed to be written by concurrent processes btw).

Btw here are all tweakable options for the driver: https://github.com/mattn/go-sqlite3#connection-string

[yonas]

I played with shared cache, tx lock, sync, mutex, etc., actually all parameters that make sense and couldn't bring it to a stable state.

@rutkai Did that include manually creating a mutex? For example:

package main

import (
	"fmt"
	"sync"
	)

var x  = 0

func increment(wg *sync.WaitGroup, m *sync.Mutex) {
	m.Lock()
	x = x + 1   // Do SQL transaction here
	m.Unlock()
	wg.Done()	
}

func main() {
	var w sync.WaitGroup
	var m sync.Mutex
	for i := 0; i < 1000; i++ {
		w.Add(1)		
		go increment(&w, &m)
	}
	w.Wait()
	fmt.Println("final value of x", x)
}

[rutkai]

Hey @yonas, the problem cannot be fixed within the application this way. The problem is that each subprocess will have its own memory segment with different mutexes in it, and we're not talking about just threads within the same process.

One dirty solution could be to use a lock file, but that's rather dirty, and imo this issue is supposed to be solved on the sqlite driver level (which is appearantly not).

But is this a usecase btw? I mean, if we need to make bookmark adding sequential, then why not executing the cli command one-by-one? For bigger volumes, using a real db behind shiori is also a viable solution. This is a constraint that may be worth documenting, but apart from that, I think that's all.

[fmartingr]

This is not going to work with SQLite in some way or another, the real solution here for that driver is executing the command one at a time or use the API to add the bookmarks, which should take care of the concurrency though it still may happen with sqlite.

[lenormf]

FWIW I could reproduce the issue with sequential (i.e. not concurrent) requests to the API.

Just calls to curl, one after the other, without any delays between.

The error shows up after about a dozen bookmarks added.

[yonas]

@rutkai

One dirty solution could be to use a lock file, but that's rather dirty, and imo this issue is supposed to be solved on the sqlite driver level (which is appearantly not).

Ok, I'm voting for using a lock file for now, and if/when the sqlite driver can handle atomic transactions.

...if we need to make bookmark adding sequential, then why not executing the cli command one-by-one?

As @lenormf mentioned, we're currently not able to use neither the CLI nor curl one after the other with no delays between calls.

[fmartingr]

Yeah, that's why I mention that it may still happen since even if the command has finished running the OS may be still syncing the changes to the FS. Unsure if we can set up a parameter in sqlite to block calls until the file has been commited and synced to disk.

[stale]

This issue has been automatically marked as stale because it has not had any activity for quite some time.
It will be closed if no further activity occurs.
Thank you for your contributions.