Skip to content

chore: add retries to read path in postgres datastore impl - #3223

Open
tstirrat15 wants to merge 1 commit into
mainfrom
tstirrat/add-network-level-retries-to-postgres-driver
Open

chore: add retries to read path in postgres datastore impl#3223
tstirrat15 wants to merge 1 commit into
mainfrom
tstirrat/add-network-level-retries-to-postgres-driver

Conversation

@tstirrat15

Copy link
Copy Markdown
Contributor

Description

This was identified as a part of trying to fix a flaky test. The idea is that if the postgres driver gets a network-level error on a read, it should be retryable; we previously weren't retrying these. It should theoretically slightly improve read resiliency.

Changes

  • Add retries on network errors
  • Add tests for those retries

Testing

Review.

@tstirrat15
tstirrat15 marked this pull request as ready for review July 10, 2026 15:45
@tstirrat15
tstirrat15 requested a review from a team as a code owner July 10, 2026 15:45
@github-actions github-actions Bot added area/datastore Affects the storage system area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Jul 10, 2026
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.25000% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/datastore/postgres/postgres.go 78.58% 2 Missing and 1 partial ⚠️
internal/datastore/postgres/revisions.go 83.34% 1 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 1af38ab Previous: cbdf06c Ratio
BenchmarkDatastoreDriver/cockroachdb-overlap-static/TestTuple/SnapshotReverseRead (github.com/authzed/spicedb/internal/datastore/benchmark) 35925563 ns/op 174908 B/op 20200 allocs/op 6805173 ns/op 172708 B/op 20195 allocs/op 5.28
BenchmarkDatastoreDriver/cockroachdb-overlap-static/TestTuple/SnapshotReverseRead (github.com/authzed/spicedb/internal/datastore/benchmark) - ns/op 35925563 ns/op 6805173 ns/op 5.28
BenchmarkDatastoreDriver/cockroachdb-overlap-insecure/TestTuple/SnapshotReverseRead (github.com/authzed/spicedb/internal/datastore/benchmark) 32436578 ns/op 173664 B/op 20197 allocs/op 6853237 ns/op 172793 B/op 20195 allocs/op 4.73
BenchmarkDatastoreDriver/cockroachdb-overlap-insecure/TestTuple/SnapshotReverseRead (github.com/authzed/spicedb/internal/datastore/benchmark) - ns/op 32436578 ns/op 6853237 ns/op 4.73
BenchmarkDatastoreDriver/cockroachdb-overlap-insecure/TestTuple/Touch (github.com/authzed/spicedb/internal/datastore/benchmark) 6044648 ns/op 22170 B/op 287 allocs/op 2857511 ns/op 22094 B/op 287 allocs/op 2.12
BenchmarkDatastoreDriver/cockroachdb-overlap-insecure/TestTuple/Touch (github.com/authzed/spicedb/internal/datastore/benchmark) - ns/op 6044648 ns/op 2857511 ns/op 2.12
BenchmarkDatastoreDriver/cockroachdb-overlap-insecure/TestTuple/Create (github.com/authzed/spicedb/internal/datastore/benchmark) 6124342 ns/op 19543 B/op 281 allocs/op 2004369 ns/op 19414 B/op 281 allocs/op 3.06
BenchmarkDatastoreDriver/cockroachdb-overlap-insecure/TestTuple/Create (github.com/authzed/spicedb/internal/datastore/benchmark) - ns/op 6124342 ns/op 2004369 ns/op 3.06

This comment was automatically generated by workflow using github-action-benchmark.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/datastore Affects the storage system area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant