Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix race in syncer test #43

Merged
merged 1 commit into from
Mar 24, 2023
Merged

Fix race in syncer test #43

merged 1 commit into from
Mar 24, 2023

Conversation

wojas
Copy link
Member

@wojas wojas commented Mar 24, 2023

The test did not ensure that the previous Sync run was finished when the next Sync run was started.

It was reproducible locally with:

GOMAXPROCS=1 go test -race  -count=100 ./syncer | grep -A30 'DATA RACE'

Output:

WARNING: DATA RACE
Write at 0x00c000161e60 by goroutine 71:
  runtime.mapassign_faststr()
      /usr/local/Cellar/go/1.19.4/libexec/src/runtime/map_faststr.go:203 +0x0
  powerdns.com/platform/lightningstream/syncer.(*Syncer).LoadOnce()
      /Users/wojas/pdns/work-ls/lightningstream/syncer/sync.go:505 +0xc66
  ...

Previous read at 0x00c000161e60 by goroutine 64:
  runtime.mapiterinit()
      /usr/local/Cellar/go/1.19.4/libexec/src/runtime/map.go:815 +0x0
  powerdns.com/platform/lightningstream/syncer/cleaner.(*Worker).SetCommitted()
      /Users/wojas/pdns/work-ls/lightningstream/syncer/cleaner/cleaner.go:55 +0xee
  ...

@wojas wojas added this to the v0.3.1 milestone Mar 24, 2023
@wojas wojas force-pushed the fix-test-data-race branch 2 times, most recently from 20c4eee to 17aa8cc Compare March 24, 2023 03:37
The test did not ensure that the previous Sync run was finished when
the next Sync run was started.

It was reproducible locally with:

GOMAXPROCS=1 go test -race  -count=100 ./syncer | grep -A30 'DATA RACE'

Output:

WARNING: DATA RACE
Write at 0x00c000161e60 by goroutine 71:
  runtime.mapassign_faststr()
      /usr/local/Cellar/go/1.19.4/libexec/src/runtime/map_faststr.go:203 +0x0
  powerdns.com/platform/lightningstream/syncer.(*Syncer).LoadOnce()
      /Users/wojas/pdns/work-ls/lightningstream/syncer/sync.go:505 +0xc66
  ...

Previous read at 0x00c000161e60 by goroutine 64:
  runtime.mapiterinit()
      /usr/local/Cellar/go/1.19.4/libexec/src/runtime/map.go:815 +0x0
  powerdns.com/platform/lightningstream/syncer/cleaner.(*Worker).SetCommitted()
      /Users/wojas/pdns/work-ls/lightningstream/syncer/cleaner/cleaner.go:55 +0xee
  ...
@wojas wojas force-pushed the fix-test-data-race branch from 17aa8cc to f1d03ce Compare March 24, 2023 03:55
@wojas wojas requested a review from nvaatstra March 24, 2023 06:39
Copy link
Contributor

@nvaatstra nvaatstra left a comment

Choose a reason for hiding this comment

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

LGTM

@wojas wojas merged commit 3786c3a into main Mar 24, 2023
@wojas wojas deleted the fix-test-data-race branch March 24, 2023 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants