Don't use shared cache mode, use a busy timeout

Turns out shared cache mode limits db connections, which ultimate triggers a lot of the locking issues:
mattn/go-sqlite3#632 (comment)

Stop using shared cache mode, in favour of a busy timeout. This lets us remove the mutexes, and
appears to solve the issue.

main.go

@@ -516,7 +516,7 @@ func (b *baseCommand) Parse(ctx context.Context, logger logger) {
 		os.Exit(1)
 	}
 
-	st, err := newStorage(ctx, logger, fmt.Sprintf("file:%s?cache=shared&_foreign_keys=on", filepath.Join(b.dbPath, mainDBFile)))
+	st, err := newStorage(ctx, logger, connStr(filepath.Join(b.dbPath, mainDBFile)))
 	if err != nil {
 		logger.Fatalf("creating storage: %v", err)
 	}
@@ -603,3 +603,7 @@ func (s *secretsManager) Save() error {
 	}
 	return nil
 }
+
+func connStr(path string) string {
+	return fmt.Sprintf("file:%s?_busy_timeout=5000&_foreign_keys=on", path)
+}

sql.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"log"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/google/uuid"
@@ -313,12 +312,6 @@ var migrations = []migration{
 type Storage struct {
 	db *sql.DB
 
-	// sqlite supports either one writer, or multiple readers. wrap tables in
-	// our own synchronization to handle this.
-	deviceLocationsMu sync.RWMutex
-	checkinsMu        sync.RWMutex
-	tripsMu           sync.RWMutex
-
 	log logger
 }
 

sql_checkin.go

@@ -16,9 +16,6 @@ import (
 // }
 
 func (s *Storage) Upsert4sqCheckin(ctx context.Context, checkin fsqCheckin) (string, error) {
-	s.checkinsMu.Lock()
-	defer s.checkinsMu.Unlock()
-
 	if checkin.ID == "" {
 		return "", fmt.Errorf("checkin has no foursquare ID")
 	}
@@ -47,9 +44,6 @@ where fsq_id=$9`,
 // up-to-date user entries for them. Also denormalizes the checkin with
 // information in to the database record.
 func (s *Storage) Sync4sqUsers(ctx context.Context) error {
-	s.checkinsMu.Lock()
-	defer s.checkinsMu.Unlock()
-
 	txErr := s.execTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
 		// run against all checkins
 		rows, err := tx.QueryContext(ctx,
@@ -124,9 +118,6 @@ func (s *Storage) Sync4sqUsers(ctx context.Context) error {
 // Sync4sqVenues finds all foursquare checkins in the DB, and ensures there are
 // up-to-date venue entries for them.
 func (s *Storage) Sync4sqVenues(ctx context.Context) error {
-	s.checkinsMu.Lock()
-	defer s.checkinsMu.Unlock()
-
 	txErr := s.execTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
 		// run against all checkins
 		rows, err := tx.QueryContext(ctx,
@@ -198,9 +189,6 @@ func (s *Storage) Sync4sqVenues(ctx context.Context) error {
 }
 
 func (s *Storage) Last4sqCheckinTime(ctx context.Context) (time.Time, error) {
-	s.checkinsMu.RLock()
-	defer s.checkinsMu.RUnlock()
-
 	var latestCheckin *time.Time
 
 	err := s.db.QueryRowContext(ctx, `select checkin_time from checkins order by datetime(checkin_time) desc limit 1`).Scan(&latestCheckin)
@@ -227,9 +215,6 @@ type Checkin struct {
 }
 
 func (s *Storage) GetCheckins(ctx context.Context, from, to time.Time) ([]Checkin, error) {
-	s.checkinsMu.RLock()
-	defer s.checkinsMu.RUnlock()
-
 	rows, err := s.db.QueryContext(ctx,
 		`select c.checkin_time, v.name, v.lng, v.lat, group_concat(p.name, ';') from checkins c
 left outer join checkin_people cp on (c.id = cp.checkin_id)

sql_device_locations.go

@@ -20,9 +20,6 @@ type DeviceLocation struct {
 }
 
 func (s *Storage) AddOTLocation(ctx context.Context, msg owntracksMessage) error {
-	s.deviceLocationsMu.Lock()
-	defer s.deviceLocationsMu.Unlock()
-
 	if !msg.IsLocation() {
 		return fmt.Errorf("message needs to be location")
 	}
@@ -53,9 +50,6 @@ func (s *Storage) AddOTLocation(ctx context.Context, msg owntracksMessage) error
 }
 
 func (s *Storage) AddGoogleTakeoutLocations(ctx context.Context, locs []takeoutLocation) error {
-	s.deviceLocationsMu.Lock()
-	defer s.deviceLocationsMu.Unlock()
-
 	err := s.execTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
 		for _, loc := range locs {
 			if loc.Raw == nil || len(loc.Raw) < 1 {
@@ -99,9 +93,6 @@ func (s *Storage) AddGoogleTakeoutLocations(ctx context.Context, locs []takeoutL
 }
 
 func (s *Storage) RecentLocations(ctx context.Context, from, to time.Time) ([]DeviceLocation, error) {
-	s.deviceLocationsMu.RLock()
-	defer s.deviceLocationsMu.RUnlock()
-
 	rows, err := s.db.QueryContext(ctx,
 		`select lat, lng, accuracy, timestamp, velocity from device_locations where timestamp > ? and timestamp < ? order by timestamp asc`, from, to)
 	if err != nil {
@@ -132,9 +123,6 @@ func (s *Storage) RecentLocations(ctx context.Context, from, to time.Time) ([]De
 // LatestLocationTimestamp returns the time at which the latest location was
 // recorded
 func (s *Storage) LatestLocationTimestamp(ctx context.Context) (time.Time, error) {
-	s.deviceLocationsMu.RLock()
-	defer s.deviceLocationsMu.RUnlock()
-
 	var timestamp time.Time
 
 	if err := s.db.QueryRowContext(ctx, `select timestamp from device_locations order by timestamp desc limit 1`).Scan(&timestamp); err != nil {

sql_test.go

@@ -131,8 +131,7 @@ func setupDB(t *testing.T) (ctx context.Context, s *Storage) {
 	ctx = context.Background()
 
 	tr := rand.New(rand.NewSource(time.Now().UnixNano())).Int63()
-
-	connStr := fmt.Sprintf("file:%s/test-%d.db?cache=shared&_foreign_keys=on", t.TempDir(), tr)
+	connStr := connStr(fmt.Sprintf("%s/test-%d.db?", t.TempDir(), tr))
 
 	db, err := sql.Open("sqlite3", connStr)
 	if err != nil {

sql_trips.go

@@ -10,9 +10,6 @@ import (
 )
 
 func (s *Storage) UpsertTripitTrip(ctx context.Context, trip *tripit.Trip, raw []byte) error {
-	s.tripsMu.Lock()
-	defer s.tripsMu.Unlock()
-
 	if trip.Id == "" {
 		return fmt.Errorf("trip has no id")
 	}
@@ -45,9 +42,6 @@ where tripit_id=?`,
 }
 
 func (s *Storage) LatestTripitID(ctx context.Context) (string, error) {
-	s.tripsMu.RLock()
-	defer s.tripsMu.RUnlock()
-
 	var (
 		tripitID string
 	)