Pitfalls

Common mistakes and "do not do this" patterns. Most of these have lint rules or roachvet checks that catch them in CI; the list below is what those rules exist to prevent.

Concurrency

• Bare goroutines — production code must launch goroutines through stop.Stopper.RunAsyncTask. A bare go func() will leak on shutdown.
• sync.Mutex with maps — use syncutil.Mutex (deadlock-detecting under race) or one of the in-tree concurrent maps.
• time.Now() outside clock code — most non-clock code should use *hlc.Clock (passed via testing knobs in tests).
• Goroutine leaks in tests — every test that creates goroutines should defer leaktest.AfterTest(t)().

KV operations

• Bypassing kv.Txn — direct BatchRequests to Sender skip pipelining and refresh and are usually wrong. Use db.Txn(ctx, fn) unless you have a specific reason not to.
• Holding locks across RPCs — a SQL handler that holds a Go mutex across a KV call can deadlock with a concurrent path that takes the mutex from the other direction.
• Skipping latches — the only valid way to elide latches is via a request type registered in batcheval's DeclareKeys.

SQL

• Implicit transactions for multi-statement work — wrap in BEGIN/COMMIT or use db.Txn to make the boundary explicit.
• Hardcoded session variable assumptions — read from sessiondata instead of consulting global cluster settings.
• Long-running queries without admission — use pkg/util/admission/ or accept that overload will affect tail latency.

Cluster version

• Adding a new behavior without a version gate — the change will break mixed-version clusters. Add a clusterversion.Vxx_yFoo constant and gate.
• Removing a version constant before its removal version — old binaries still need to recognize the constant or upgrades break. Removal is a separate, later change.

Error handling

• fmt.Errorf instead of errors.Newf — non-redactable. Switch to errors.Newf or errors.Wrapf.
• Returning a new error type without wiring it to the wire format — KV errors must be encodable via errors.EncodeError or they will break across nodes.

Tests

• Using t.Skip without a reason — use skip.UnderRace, skip.UnderStress, or skip.WithIssue(...) so the skip is auditable.
• Tests that rely on a specific cluster setting being default — pin the setting in TestServerArgs.
• Tests that mutate global state — wrap with defer to reset; otherwise other parallel tests will fail.

Configuration

• Committing changes to BUILD.bazel by hand — usually wrong; run ./dev gen bazel.
• Adding a new go:generate directive — does not run inside Bazel; mirror the work in a Bazel rule.
• Adding a CGo dependency — affects every cross-compile target. Discuss with dev-inf first.

Multi-tenant

• Calling host RPCs from tenant code without checking capabilities — a tenant should never be able to call a host API it has not been granted. Guard with tenantcapabilities.Authorizer.
• Using node-local file paths — tenants run on shared nodes; treat the filesystem as ephemeral.

Storage

• Reading/writing the engine without a *pebble.Snapshot for stable reads — concurrent writes can change the view mid-iteration.
• Holding a Pebble batch across many MVCC ops — each batch is in-memory; long batches lead to heap pressure.
• Writing to MVCC at a wall-time-after-the-future timestamp — clock-skew checks will reject the value.

Logging and tracing

• Dropping ctx — every log line and span must carry the operation context.
• Logging redactable values without the r verb — strips redaction markers.
• Verbose logging at high V-levels — usually a performance footgun; prefer log.VEventf so the message also appears in the trace.

Related pages

• How to contribute / patterns and conventions.
• How to contribute / debugging.