starred/shiori

Fork 0

mirror of https://github.com/go-shiori/shiori.git synced 2026-04-25 06:25:54 +03:00

[GH-ISSUE #739] Adding bookmarks concurrently doesn't work #361

New issue

Closed

opened 2026-02-25 23:34:03 +03:00 by kerem · 10 comments

kerem commented

2026-02-25 23:34:03 +03:00

Owner

Originally created by @yonas on GitHub (Sep 20, 2023).
Original GitHub issue: https://github.com/go-shiori/shiori/issues/739

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

Originally created by @yonas on GitHub (Sep 20, 2023). Original GitHub issue: https://github.com/go-shiori/shiori/issues/739 ## 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](https://www.sqlite.org/wal.html)

kerem

2026-02-25 23:34:03 +03:00

closed this issue
added the
component:backend

database:sqlite

tag:stale
labels

kerem commented

2026-02-25 23:34:03 +03:00

Author

Owner

@rutkai commented on GitHub (Sep 24, 2023):

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?

@rutkai commented on GitHub (Sep 24, 2023): 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?

kerem commented

2026-02-25 23:34:03 +03:00

Author

Owner

@Mxrk commented on GitHub (Sep 26, 2023):

Would a busy timeout on the connection help?

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

@Mxrk commented on GitHub (Sep 26, 2023): Would a busy timeout on the connection help? https://www.sqlite.org/c3ref/busy_timeout.html

kerem commented

2026-02-25 23:34:03 +03:00

Author

Owner

@rutkai commented on GitHub (Sep 26, 2023):

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

@rutkai commented on GitHub (Sep 26, 2023): 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

kerem commented

2026-02-25 23:34:03 +03:00

Author

Owner

@yonas commented on GitHub (Jan 26, 2024):

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)
}

@yonas commented on GitHub (Jan 26, 2024): > 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) } ```

kerem commented

2026-02-25 23:34:03 +03:00

Author

Owner

@rutkai commented on GitHub (Jan 26, 2024):

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.

@rutkai commented on GitHub (Jan 26, 2024): 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.

kerem commented

2026-02-25 23:34:03 +03:00

Author

Owner

@fmartingr commented on GitHub (Jan 26, 2024):

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.

@fmartingr commented on GitHub (Jan 26, 2024): 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.

kerem commented

2026-02-25 23:34:03 +03:00

Author

Owner

@lenormf commented on GitHub (Jan 27, 2024):

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.

@lenormf commented on GitHub (Jan 27, 2024): 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.

kerem commented

2026-02-25 23:34:03 +03:00

Author

Owner

@yonas commented on GitHub (Jan 28, 2024):

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

@yonas commented on GitHub (Jan 28, 2024): @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.

kerem commented

2026-02-25 23:34:03 +03:00

Author

Owner

@fmartingr commented on GitHub (Feb 4, 2024):

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.

@fmartingr commented on GitHub (Feb 4, 2024): 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.

kerem commented

2026-02-25 23:34:03 +03:00

Author

Owner

@stale[bot] commented on GitHub (Mar 6, 2024):

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.

@stale[bot] commented on GitHub (Mar 6, 2024): 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.