google-cloud-go icon indicating copy to clipboard operation
google-cloud-go copied to clipboard
verified publisher icon googleapis

datastore: data race in transaction retryer

Open maros7 opened this issue 1 year ago • 0 comments

Client

Datastore

Environment

Any

Go Environment

go version go1.22.4 darwin/arm64

Code

WARNING: DATA RACE
Read at 0x000102683318 by goroutine 383:
  github.com/googleapis/gax-go/v2.(*Backoff).Pause()
      /Users/myUser/go/pkg/mod/github.com/googleapis/gax-go/[email protected]/call_option.go:186 +0x60
  github.com/googleapis/gax-go/v2.(*boRetryer).Retry()
      /Users/myUser/go/pkg/mod/github.com/googleapis/gax-go/[email protected]/call_option.go:118 +0x108
  cloud.google.com/go/datastore.backoffBeforeRetry()
      /Users/myUser/go/pkg/mod/cloud.google.com/go/[email protected]/transaction.go:290 +0x50
  cloud.google.com/go/datastore.(*Client).RunInTransaction()
      /Users/myUser/go/pkg/mod/cloud.google.com/go/[email protected]/transaction.go:417 +0x454
  github.com/myOrg/sdk-wrapper/datastore.(*Client).RunInTransaction.func2()
      /Users/myUser/git/sdk-wrapper/datastore/client.go:394 +0x298
  github.com/myOrg/sdk-wrapper/internal/retry.Run.func1()
      /Users/myUser/go/pkg/mod/github.com/myOrg/[email protected]/internal/retry/retry.go:39 +0x68
  github.com/cenkalti/backoff/v4.RetryNotifyWithTimer.Operation.withEmptyData.func1()
      /Users/myUser/go/pkg/mod/github.com/cenkalti/backoff/[email protected]/retry.go:18 +0x30
  github.com/cenkalti/backoff/v4.doRetryNotify[go.shape.struct {}]()
      /Users/myUser/go/pkg/mod/github.com/cenkalti/backoff/[email protected]/retry.go:88 +0x15c
  github.com/cenkalti/backoff/v4.RetryNotifyWithTimer()
      /Users/myUser/go/pkg/mod/github.com/cenkalti/backoff/[email protected]/retry.go:61 +0x80
  github.com/cenkalti/backoff/v4.RetryNotify()
      /Users/myUser/go/pkg/mod/github.com/cenkalti/backoff/[email protected]/retry.go:49 +0x290
  github.com/cenkalti/backoff/v4.Retry()
      /Users/myUser/go/pkg/mod/github.com/cenkalti/backoff/[email protected]/retry.go:38 +0x274
  github.com/myOrg/sdk-wrapper/internal/retry.Run()
      /Users/myUser/go/pkg/mod/github.com/myOrg/[email protected]/internal/retry/retry.go:71 +0x21c
  github.com/myOrg/sdk-wrapper/datastore.(*Client).RunInTransaction()
      /Users/myUser/git/sdk-wrapper/datastore/client.go:412 +0x1c8
  github.com/myOrg/sdk-wrapper/datastore.(*Client).put()
      /Users/myUser/git/sdk-wrapper/datastore/client.go:301 +0x540
  github.com/myOrg/sdk-wrapper/datastore.(*Client).PutMulti()
      /Users/myUser/git/sdk-wrapper/datastore/client.go:256 +0x15c
  github.com/myOrg/sdk-wrapper/datastore_test.IntegrationTestSuite.TestClient_PutMulti_concurrent_transactions.func1.1()
      /Users/myUser/git/sdk-wrapper/datastore/client_test.go:694 +0x120
  testing.tRunner()
      /Users/myUser/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /Users/myUser/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1742 +0x40

Previous write at 0x000102683318 by goroutine 374:
  github.com/googleapis/gax-go/v2.(*Backoff).Pause()
      /Users/myUser/go/pkg/mod/github.com/googleapis/gax-go/[email protected]/call_option.go:200 +0x15c
  github.com/googleapis/gax-go/v2.(*boRetryer).Retry()
      /Users/myUser/go/pkg/mod/github.com/googleapis/gax-go/[email protected]/call_option.go:118 +0x108
  cloud.google.com/go/datastore.backoffBeforeRetry()
      /Users/myUser/go/pkg/mod/cloud.google.com/go/[email protected]/transaction.go:290 +0x50
  cloud.google.com/go/datastore.(*Client).RunInTransaction()
      /Users/myUser/go/pkg/mod/cloud.google.com/go/[email protected]/transaction.go:417 +0x454
  github.com/myOrg/sdk-wrapper/datastore.(*Client).RunInTransaction.func2()
      /Users/myUser/git/sdk-wrapper/datastore/client.go:394 +0x298
  github.com/myOrg/sdk-wrapper/internal/retry.Run.func1()
      /Users/myUser/go/pkg/mod/github.com/myOrg/[email protected]/internal/retry/retry.go:39 +0x68
  github.com/cenkalti/backoff/v4.RetryNotifyWithTimer.Operation.withEmptyData.func1()
      /Users/myUser/go/pkg/mod/github.com/cenkalti/backoff/[email protected]/retry.go:18 +0x30
  github.com/cenkalti/backoff/v4.doRetryNotify[go.shape.struct {}]()
      /Users/myUser/go/pkg/mod/github.com/cenkalti/backoff/[email protected]/retry.go:88 +0x15c
  github.com/cenkalti/backoff/v4.RetryNotifyWithTimer()
      /Users/myUser/go/pkg/mod/github.com/cenkalti/backoff/[email protected]/retry.go:61 +0x80
  github.com/cenkalti/backoff/v4.RetryNotify()
      /Users/myUser/go/pkg/mod/github.com/cenkalti/backoff/[email protected]/retry.go:49 +0x290
  github.com/cenkalti/backoff/v4.Retry()
      /Users/myUser/go/pkg/mod/github.com/cenkalti/backoff/[email protected]/retry.go:38 +0x274
  github.com/myOrg/sdk-wrapper/internal/retry.Run()
      /Users/myUser/go/pkg/mod/github.com/myOrg/[email protected]/internal/retry/retry.go:71 +0x21c
  github.com/myOrg/sdk-wrapper/datastore.(*Client).RunInTransaction()
      /Users/myUser/git/sdk-wrapper/datastore/client.go:412 +0x1c8
  github.com/myOrg/sdk-wrapper/datastore.(*Client).put()
      /Users/myUser/git/sdk-wrapper/datastore/client.go:301 +0x540
  github.com/myOrg/sdk-wrapper/datastore.(*Client).PutMulti()
      /Users/myUser/git/sdk-wrapper/datastore/client.go:256 +0x15c
  github.com/myOrg/sdk-wrapper/datastore_test.IntegrationTestSuite.TestClient_PutMulti_concurrent_transactions.func1.1()
      /Users/myUser/git/sdk-wrapper/datastore/client_test.go:694 +0x120
  testing.tRunner()
      /Users/myUser/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /Users/myUser/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1742 +0x40

Expected behavior

No data race like before https://github.com/googleapis/google-cloud-go/pull/10349. For example, if I downgrade to v1.17.0, then go test -race doesn't report a data race.

Actual behavior

Data race as seen above. Due to global variable being accessed concurrently: https://github.com/googleapis/google-cloud-go/blob/0c17df27b06c4d67bc9a52ffad88fc58589d0096/datastore/transaction.go#L52

Screenshots

Additional context

Started after https://github.com/googleapis/google-cloud-go/pull/10349.

maros7 avatar Jun 17 '24 15:06 maros7