Skip to content

Commit

Permalink
Check for locked database when writing to perfetto db. (#426)
Browse files Browse the repository at this point in the history
Previously we would see an infrequent error message saying the
database was locked. This was due to a missing isLocked check on the
path that attempts to assign a replica number to the current
weavelet. Fixed by retrying on a locking error.

Also improved a corresponding error message.

Added a benchmark for calling component methods when tracing is turned
on. (This benchmark is what would trigger the preceding issue.)
  • Loading branch information
ghemawat committed Jun 28, 2023
1 parent bd33647 commit c173a6e
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 7 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,6 @@ examples/factors/factors
examples/onlineboutique/onlineboutique
examples/hello/hello
website/public/

# Binaries produced by [go test] when passed -c or a profiling option.
*.test
21 changes: 15 additions & 6 deletions runtime/perfetto/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,10 @@ func (d *DB) storeEncoded(ctx context.Context, app, version string, encoded []by
VALUES (?,?,?);
`
_, err := d.execDB(ctx, stmt, app, version, string(encoded))
return err
if err != nil {
return fmt.Errorf("write trace to %s: %w", d.fname, err)
}
return nil
}

func (d *DB) encodeSpans(ctx context.Context, app, version string, spans []sdktrace.ReadOnlySpan) ([]byte, error) {
Expand Down Expand Up @@ -316,12 +319,18 @@ func (d *DB) getReplicaNumber(ctx context.Context, app, version, weaveletId stri
return replicaNum, nil
}

// Get the replica number from the database.
replicaNum, err := d.getReplicaNumberUncached(ctx, app, version, weaveletId)
if err == nil {
d.replicaNumCache.Add(cacheKey, replicaNum)
for r := retry.Begin(); r.Continue(ctx); {
// Get the replica number from the database.
replicaNum, err := d.getReplicaNumberUncached(ctx, app, version, weaveletId)
if isLocked(err) {
continue
}
if err == nil {
d.replicaNumCache.Add(cacheKey, replicaNum)
}
return replicaNum, err
}
return replicaNum, err
return -1, ctx.Err()
}

func (d *DB) getReplicaNumberUncached(ctx context.Context, app, version, weaveletId string) (int, error) {
Expand Down
27 changes: 26 additions & 1 deletion weavertest/internal/simple/simple_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"testing"
"time"

"github.com/ServiceWeaver/weaver/internal/traceio"
"github.com/ServiceWeaver/weaver/weavertest"
"github.com/ServiceWeaver/weaver/weavertest/internal/simple"
"github.com/google/uuid"
Expand Down Expand Up @@ -187,12 +188,36 @@ func TestRoutedCall(t *testing.T) {
func BenchmarkCall(b *testing.B) {
for _, runner := range weavertest.AllRunners() {
runner.Bench(b, func(b *testing.B, dst simple.Destination) {
ctx := context.Background()
for i := 0; i < b.N; i++ {
_, err := dst.Getpid(context.Background())
_, err := dst.Getpid(ctx)
if err != nil {
b.Fatal(err)
}
}
})
}
}

func BenchmarkTracedCall(b *testing.B) {
weavertest.Local.Bench(b, func(b *testing.B, dst simple.Destination) {
ctx, span := traceio.TestTracer().Start(context.Background(), "foo")

// Make a new span every 100 calls.
const k = 100
for i := 0; i < b.N; {
batch := b.N - i
if batch > k {
batch = k // Bound trace length
}
i += batch
for j := 0; j < batch; j++ {
_, err := dst.Getpid(ctx)
if err != nil {
b.Fatal(err)
}
}
span.End()
}
})
}

0 comments on commit c173a6e

Please sign in to comment.