Skip to content

feat(006): API datastore abstraction with memdb and ScyllaDB backends#138

Merged
juliuskrah merged 5 commits into
mainfrom
006-api-datastore-abstraction
May 12, 2026
Merged

feat(006): API datastore abstraction with memdb and ScyllaDB backends#138
juliuskrah merged 5 commits into
mainfrom
006-api-datastore-abstraction

Conversation

@juliuskrah
Copy link
Copy Markdown
Contributor

Summary

  • Introduces datastore.Datastore interface (18 CRUD methods + Close) decoupling the GraphQL API from its persistence layer; backend selected at startup via GITSTORE_DATASTORE_BACKEND (default: memdb)
  • Implements go-memdb backend for zero-dependency local development and ScyllaDB backend for production, with automatic schema migrations on first start (distributed LWT lock prevents concurrent races)
  • Wraps every backend in InstrumentedDatastore — per-operation Prometheus histogram + error counter + structured zap error log
  • Replaces stale HTTP contract tests with a backend-agnostic RunContractSuite (30 sub-tests); moves gRPC tests to tests/contract/grpc/; adds //go:build scylla integration tests via testcontainers

What changed

Area Change
internal/datastore/ New Datastore interface, entities.go, InstrumentedDatastore, metrics, memdb + ScyllaDB backends, factory
internal/graph/ Resolvers and Service rewritten to use Datastore; cache.Manager removed from the service layer
internal/loader/ DataLoaders migrated from *catalog.Catalog to Datastore
internal/health/ Health check uses ListProducts instead of cache ping
cmd/server/main.go Wires factory.NewDatastore + InstrumentedDatastore at startup
tests/contract/ New datastore contract suite (memdb + scylla wiring); gRPC tests moved here
compose.scylla.yml Docker Compose override for ScyllaDB (mirrors compose.admin.yml pattern)
.github/workflows/ci-integration.yml Added datastore-contract-test-scylla job; restored integration-test job

Test plan

  • go test -race ./... — 13 packages, all pass
  • go fmt, go vet, staticcheck — clean
  • Go licence header checks (--all and --diff-base origin/main) — pass
  • go test -tags scylla -timeout 10m ./tests/contract/datastore/... ./internal/datastore/scylla/... — runs in CI via datastore-contract-test-scylla job (requires Docker)
  • go test -tags grpc -timeout 5m ./tests/contract/grpc/... — runs in CI via grpc-contract-test job

🤖 Generated with Claude Code

Introduces a pluggable Datastore interface decoupling the GraphQL API from
its persistence layer. Backend is selected at runtime via GITSTORE_DATASTORE_BACKEND
(default: memdb). ScyllaDB backend auto-migrates on first start using a
distributed LWT lock to prevent concurrent migration races.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2d55da557

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread gitstore-api/internal/datastore/scylla/backend.go Outdated
Comment thread gitstore-api/internal/graph/category.resolvers.go
Comment thread gitstore-api/internal/datastore/scylla/backend.go Outdated
juliuskrah and others added 4 commits May 12, 2026 14:05
- ScyllaDB backend: open a keyspace-free session for migrations then a
  keyspaced session for data ops; fixes "Keyspace does not exist" on first
  start because gocql refused to connect before CREATE KEYSPACE ran
- ScyllaDB tests: switch to TestMain/shared-container pattern; eliminates
  per-test container spawning that exhausted aio-max-nr on GitHub Actions
- Integration test: skip TestTagPushPublishesToGraphQL with an explanatory
  message; the git→datastore ingestion path was removed in feature 006 and
  is tracked as a future pipeline task

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…it step

The application now assumes the keyspace already exists (operator's
responsibility, enforced by the compose scylla-init service). Changes:
- Remove CREATE KEYSPACE from migration CQL files; use unqualified table
  names so migrations run in the session keyspace
- RunMigrations creates schema_migrations_lock itself (migration runner
  infrastructure, like Flyway's own schema table)
- backend.New uses a single keyspace-scoped session; no bootstrap logic
- compose.scylla.yml adds a scylla-init service that runs cqlsh to create
  the keyspace before the api service starts
- testcontainers TestMain provisions the keyspace via a no-keyspace session
  before any test opens a store, mirroring the compose init pattern

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…kend

- Switch gocql fork from v1.18.0 to v1.15.3 to fix shard-aware port
  issues with Docker mapped ports on macOS (Rancher Desktop)
- Use MapScanCAS instead of ScanCAS for LWT INSERT/DELETE so variable
  column counts on conflict do not cause unmarshal errors
- Embed migrations as a flat FS via fs.Sub so gocqlx/migrate can locate
  *.cql files; merge 002_add_indexes.cql into 001_initial_schema.cql
- Add `-- CALL await_tables;` callback to poll tables after CREATE TABLE
  before running CREATE INDEX — on single-node --developer-mode=1 ScyllaDB
  schema agreement returns immediately but storage may not be initialised
- Place a semicolon in the header comment block so ReadString(';') splits
  it off before CREATE TABLE, preventing isComment from swallowing the
  first CREATE TABLE statement
- Pass explicit non-key column list to table.Update() to avoid empty SET
  clause (CQL parse error) in UpdateProduct/Category/Collection
- Add DisableShardAwarePort config field and parse host:port in parseHosts;
  set cluster.Port explicitly (gocql.NewCluster does not parse ports)
- Use wait.ForExec(cqlsh) and 30 s provisionKeyspace retry loop to handle
  ScyllaDB accepting connections slightly after its startup log message
- price column is decimal (CQL) / *inf.Dec (Go) not double/float64

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ackend errors

- Move table metadata from package-level vars into the scyllaDatastore
  struct, built at New() time with the configured keyspace prefix so
  deployments using a non-default keyspace are routed correctly
- Thread keyspace into RunMigrations and awaitTablesCallback so the
  await-poll queries also target the right keyspace
- Distinguish gocql.ErrNotFound from other errors in all Get* methods:
  only ErrNotFound maps to datastore.ErrNotFound; timeouts and network
  failures are surfaced as-is instead of masking them as missing entities
- Update* and Delete* now propagate the error from their existence check
  directly rather than double-wrapping it

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@juliuskrah juliuskrah enabled auto-merge May 12, 2026 15:40
@juliuskrah juliuskrah added this pull request to the merge queue May 12, 2026
Merged via the queue into main with commit bf5f57f May 12, 2026
25 checks passed
@juliuskrah juliuskrah deleted the 006-api-datastore-abstraction branch May 12, 2026 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant