Skip to content

test(server): remove the unlocked log-capture pattern from the remaining cmd/server tests (#301 follow-up) #310

Closed

Description

@dborup

Follow-up to #301 / #307. #307 fixed the harness-only race in TestHashMigrate_LogsMaxWriteLockHold_215: an unlocked read of the log-capture bytes.Buffer while background goroutines were still logging. The same anti-pattern remains in other cmd/server tests. The review of #307 also left four nits.

Same unlocked-capture pattern in 7 test files

These test files read a plain bytes.Buffer while the standard logger still points at it:

  • cdn_detection_test.go (two sites, plus one writerFunc)
  • issue199_node_not_found_test.go (two sites)
  • memlimit_test.go
  • neighbor_graph_geo_test.go
  • nodes_pagination_postfilter_test.go
  • ping_score_history_production_test.go
  • schema_degradation_per_store_test.go

The hazard is not limited to the index builders. Several other goroutines can outlive a test and write into a later test's capture:

  • OpenDB starts go d.healSchemaFlags().
  • runDistanceIndexBuild logs every build.
  • (*DB).Close logs the WAL checkpoint.
  • Several recomputers log from their own goroutines.

Two fixes would close the class:

Nits from the #307 review

  1. neighbor_graph_geo_test.go:20-22. The comment claims a plain bytes.Buffer capture is safe under concurrent log.Printf. That is the reasoning test(server): data race between startBackgroundIndexBuilds and PacketStore.Load under -race (TestHashMigrate_LogsMaxWriteLockHold_215) #301 disproved: the logger orders writers against each other, but not against the harness's unlocked String(). Correct it.
  2. hash_migrate_log_race_301_test.go:41. The fixed time.Sleep(200 * time.Millisecond) is what makes the test a reliable race detector. Consider polling on syncBuffer's written channel, or a read func() string handed to fn.
  3. hash_migrate_log_race_301_test.go:28-31. If store.Load() fails, t.Fatal fires while the gate goroutine is still blocked on <-release. Add defer close(release) after installing the gate.

Acceptance

  • No test in cmd/server reads a log-capture buffer without one of the two fixes above.
  • go test -race -count=20 on the touched tests is green.
  • No production code change.

Activity

  1. added a commit that references this issue on Oct 6, 2026
  2. dborup commented on Oct 6, 2026

    @dborup
    OwnerAuthor

    Fixed by #311 (merged as 629a254a3b75f89d3ccdfca3f80edbc7960d662f).

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions