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

[Cosmos] Fix data race issue with cosmos_global_endpoint_manager #22575

Merged
merged 1 commit into from
Mar 13, 2024

Conversation

xsuo2022
Copy link
Contributor

While working with the sdk, I ran into this data race issue

WARNING: DATA RACE01:26
Read at 0x00c000362058 by goroutine 39:01:26
  github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos.(*globalEndpointManager).ShouldRefresh()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/confluentinc/azure-sdk-for-go/sdk/data/azcosmos@v0.0.0-20240306185250-2945d4e6c7bc/cosmos_global_endpoint_manager.go:84 +0x5a01:26
  github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos.(*globalEndpointManagerPolicy).Do()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/confluentinc/azure-sdk-for-go/sdk/data/azcosmos@v0.0.0-20240306185250-2945d4e6c7bc/cosmos_global_endpoint_manager_policy.go:18 +0x2601:26
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/azcore@v1.10.0/internal/exported/request.go:107 +0x21801:26
  github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos.(*headerPolicies).Do()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/confluentinc/azure-sdk-for-go/sdk/data/azcosmos@v0.0.0-20240306185250-2945d4e6c7bc/cosmos_headers_policy.go:51 +0x89201:26
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/azcore@v1.10.0/internal/exported/request.go:107 +0x21801:26
  github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.telemetryPolicy.Do()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/azcore@v1.10.0/runtime/policy_telemetry.go:70 +0x2a401:26
  github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.(*telemetryPolicy).Do()01:26
      <autogenerated>:1 +0x5a01:26
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/azcore@v1.10.0/internal/exported/request.go:107 +0x21801:26
  github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.includeResponsePolicy()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/azcore@v1.10.0/runtime/policy_include_response.go:19 +0x2e01:26
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.PolicyFunc.Do()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/azcore@v1.10.0/internal/exported/request.go:177 +0x3301:26
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/azcore@v1.10.0/internal/exported/request.go:107 +0x21801:26
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.Pipeline.Do()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/azcore@v1.10.0/internal/exported/pipeline.go:76 +0x8f01:26
  github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos.(*Client).executeAndEnsureSuccessResponse()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/confluentinc/azure-sdk-for-go/sdk/data/azcosmos@v0.0.0-20240306185250-2945d4e6c7bc/cosmos_client.go:465 +0x5301:26
  github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos.(*Client).sendQueryRequest()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/confluentinc/azure-sdk-for-go/sdk/data/azcosmos@v0.0.0-20240306185250-2945d4e6c7bc/cosmos_client.go:313 +0x49301:26
  github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos.(*Client).NewQueryDatabasesPager.func2()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/confluentinc/azure-sdk-for-go/sdk/data/azcosmos@v0.0.0-20240306185250-2945d4e6c7bc/cosmos_client.go:249 +0x2b201:26
  github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.(*Pager[go.shape.struct { github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos.Response; ContinuationToken string; Databases []github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos.DatabaseProperties }]).NextPage()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/azcore@v1.10.0/runtime/pager.go:79 +0x6d001:26
  github.com/confluentinc/flink-control-plane/apiserver/storage/cosmosdb/testutil.(*CosmosDB).DropDatabaseSchema()01:26
      /home/semaphore/git/go/1.21.7/src/github.com/confluentinc/flink-control-plane/apiserver/storage/cosmosdb/testutil/cosmosdb.go:90 +0x38c01:26
  github.com/confluentinc/flink-control-plane/lisboa-grpc/storage/cosmosdb_test.glob..func1.2()01:26
      /home/semaphore/git/go/1.21.7/src/github.com/confluentinc/flink-control-plane/lisboa-grpc/storage/cosmosdb/cosmosdb_integration_test.go:34 +0x8401:26
  github.com/confluentinc/flink-control-plane/lisboa-grpc/storage/cosmosdb_test.glob..func1.DescribeStorage.func3.1()01:26
      /home/semaphore/git/go/1.21.7/src/github.com/confluentinc/flink-control-plane/lisboa-grpc/storage/testutil/testutil.go:45 +0x4101:26
  github.com/onsi/ginkgo/v2/internal.extractBodyFunction.func3()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/onsi/ginkgo/v2@v2.15.0/internal/node.go:463 +0x2e01:26
  github.com/onsi/ginkgo/v2/internal.(*Suite).runNode.func3()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/onsi/ginkgo/v2@v2.15.0/internal/suite.go:889 +0x14b01:26
Previous write at 0x00c000362058 by goroutine 22:01:26
  github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos.(*globalEndpointManager).Update()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/confluentinc/azure-sdk-for-go/sdk/data/azcosmos@v0.0.0-20240306185250-2945d4e6c7bc/cosmos_global_endpoint_manager.go:105 +0x47401:26
  github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos.(*globalEndpointManagerPolicy).Do.func1()01:26
      /home/semaphore/git/go/1.21.7/pkg/mod/github.com/confluentinc/azure-sdk-for-go/sdk/data/azcosmos@v0.0.0-20240306185250-2945d4e6c7bc/cosmos_global_endpoint_manager_policy.go:21 +0x4b

@github-actions github-actions bot added Community Contribution Community members are working on the issue Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Mar 11, 2024
Copy link

Thank you for your contribution @xsuo2022! We will review the pull request and get back to you soon.

@xsuo2022
Copy link
Contributor Author

hi @ealsur I found another bug with some data race issue, if you could take a look. Thanks!

@ealsur
Copy link
Member

ealsur commented Mar 12, 2024

@simorenoh Could you take a look?

@ealsur
Copy link
Member

ealsur commented Mar 12, 2024

@xsuo2022 Seems you are using the version in the repo, which has not been released yet officially. Be advised that these files are still currently being worked on: #22394 for example

@xsuo2022
Copy link
Contributor Author

@xsuo2022 Seems you are using the version in the repo, which has not been released yet officially. Be advised that these files are still currently being worked on: #22394 for example

No wonder I didn't hit this issue with the source repo and got this data race issue with my local fork! You can still take a look and see if this makes sense 😄

@simorenoh
Copy link
Member

@ealsur I haven't run into anything like this in my own testing but it makes sense enough to me - doesn't seem like it breaks anything either so I'll go ahead and approve

@xsuo2022 Thanks for finding this and for the contribution!

Copy link
Member

@simorenoh simorenoh left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@simorenoh simorenoh merged commit 5ca7774 into Azure:main Mar 13, 2024
21 checks passed
@xsuo2022 xsuo2022 deleted the xsuo/cosmos-data-race branch March 13, 2024 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Contribution Community members are working on the issue Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants