Skip to content

Commit

Permalink
server: restore separate connections for the local state db (#29)
Browse files Browse the repository at this point in the history
In #25 I switched away from using a transaction for queries on the local state
database, because it did not interact properly with the queryable interface we
added in #24. But because I'd switched away from having a separate read-only
connection for these queries, this had the undesirable side-effect of allowing
write queries on the local state database.

Switch back to using a separate read-only connection for local state.  Morally
this is a partial revert of #24, but done in a slightly different way.
  • Loading branch information
creachadair authored Aug 1, 2024
1 parent 501ba63 commit 938e5f4
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 17 deletions.
37 changes: 28 additions & 9 deletions server/tailsql/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"database/sql"
"errors"
"fmt"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -42,17 +43,35 @@ var schema = &squibble.Schema{
type localState struct {
// Exclusive: Write transaction
// Shared: Read transaction
txmu sync.RWMutex
db *sql.DB
txmu sync.RWMutex
rw, ro *sql.DB
}

// newLocalState constructs a new LocalState helper using the given database.
func newLocalState(db *sql.DB) (*localState, error) {
if err := schema.Apply(context.Background(), db); err != nil {
db.Close()
// newLocalState constructs a new LocalState helper for the given database URL.
func newLocalState(url string) (*localState, error) {
if !strings.HasPrefix(url, "file:") {
url = "file:" + url
}
urlRO := url + "?mode=ro"

// Open separate copies of the database for writing query logs vs. serving
// queries to the UI.
rw, err := openAndPing("sqlite", url)
if err != nil {
return nil, err
}
if err := schema.Apply(context.Background(), rw); err != nil {
rw.Close()
return nil, fmt.Errorf("initializing schema: %w", err)
}
return &localState{db: db}, nil

ro, err := openAndPing("sqlite", urlRO)
if err != nil {
rw.Close()
return nil, err
}

return &localState{rw: rw, ro: ro}, nil
}

// LogQuery adds the specified query to the query log.
Expand All @@ -67,7 +86,7 @@ func (s *localState) LogQuery(ctx context.Context, user string, q Query, elapsed
}
s.txmu.Lock()
defer s.txmu.Unlock()
tx, err := s.db.BeginTx(ctx, nil)
tx, err := s.rw.BeginTx(ctx, nil)
if err != nil {
return err
}
Expand Down Expand Up @@ -98,7 +117,7 @@ func (s *localState) LogQuery(ctx context.Context, user string, q Query, elapsed
func (s *localState) Query(ctx context.Context, query string, params ...any) (RowSet, error) {
s.txmu.RLock()
defer s.txmu.RUnlock()
return s.db.QueryContext(ctx, query, params...)
return s.ro.QueryContext(ctx, query, params...)
}

// Close satisfies part of the Queryable interface. For this database the
Expand Down
9 changes: 1 addition & 8 deletions server/tailsql/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,14 +205,7 @@ func (o Options) localState() (*localState, error) {
return nil, nil
}
url := os.ExpandEnv(o.LocalState)
db, err := sql.Open("sqlite", url)
if err != nil {
return nil, fmt.Errorf("open %q: %w", url, err)
} else if err := db.PingContext(context.Background()); err != nil {
db.Close()
return nil, fmt.Errorf("ping %q: %w", url, err)
}
return newLocalState(db)
return newLocalState(url)
}

func (o Options) routePrefix() string {
Expand Down

0 comments on commit 938e5f4

Please sign in to comment.