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.

Sign up for GitHub

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

Jump to bottom

test: Update azure-storage-blob-go to include a data race fix #12325

Merged
merged 2 commits into from
Mar 25, 2024
Merged

test: Update azure-storage-blob-go to include a data race fix #12325

merged 2 commits into from
Mar 25, 2024

Conversation

paul1r
Copy link
Collaborator

@paul1r paul1r commented Mar 22, 2024 •
edited
Loading

What this PR does / why we need it:
Pulls in a data race fix for the azure-storage-blob library. External fork PR fix is: MasslessParticle/azure-storage-blob-go#1

Before fix:

go test -race -count=1 .
==================
WARNING: DATA RACE
Write at 0x00c00061f5d0 by goroutine 46:
  github.com/Azure/azure-storage-blob-go/azblob.(*copier).waitForFinish.func1()
      /Users/progers/dev/src/github.com/grafana/loki/vendor/github.com/Azure/azure-storage-blob-go/azblob/chunkwriting.go:195 +0x50

Previous read at 0x00c00061f5d0 by goroutine 37:
  github.com/Azure/azure-storage-blob-go/azblob.(*copier).waitForFinish()
      /Users/progers/dev/src/github.com/grafana/loki/vendor/github.com/Azure/azure-storage-blob-go/azblob/chunkwriting.go:200 +0x168
  github.com/Azure/azure-storage-blob-go/azblob.(*copier).close()
      /Users/progers/dev/src/github.com/grafana/loki/vendor/github.com/Azure/azure-storage-blob-go/azblob/chunkwriting.go:179 +0x30
  github.com/Azure/azure-storage-blob-go/azblob.copyFromReader()
      /Users/progers/dev/src/github.com/grafana/loki/vendor/github.com/Azure/azure-storage-blob-go/azblob/chunkwriting.go:64 +0x360
  github.com/Azure/azure-storage-blob-go/azblob.UploadStreamToBlockBlob()
      /Users/progers/dev/src/github.com/grafana/loki/vendor/github.com/Azure/azure-storage-blob-go/azblob/highlevel.go:553 +0x104
  github.com/grafana/loki/pkg/storage/chunk/client/azure.(*BlobStorage).PutObject.func1()
      /Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/client/azure/blob_storage_client.go:290 +0x200
  github.com/grafana/loki/pkg/util/instrument.TimeRequest()
      /Users/progers/dev/src/github.com/grafana/loki/pkg/util/instrument/instrument.go:20 +0xac
  github.com/grafana/loki/pkg/storage/chunk/client/azure.(*BlobStorage).PutObject()
      /Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/client/azure/blob_storage_client.go:282 +0x134
  github.com/grafana/loki/pkg/storage/chunk/client/azure.Test_Hedging.func1()
      /Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/client/azure/blob_storage_client_test.go:99 +0x154
  github.com/grafana/loki/pkg/storage/chunk/client/azure.Test_Hedging.func4()
      /Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/client/azure/blob_storage_client_test.go:148 +0x278
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1595 +0x1b0
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1648 +0x40

Goroutine 46 (running) created at:
  github.com/Azure/azure-storage-blob-go/azblob.(*copier).waitForFinish()
      /Users/progers/dev/src/github.com/grafana/loki/vendor/github.com/Azure/azure-storage-blob-go/azblob/chunkwriting.go:192 +0x148
  github.com/Azure/azure-storage-blob-go/azblob.(*copier).close()
      /Users/progers/dev/src/github.com/grafana/loki/vendor/github.com/Azure/azure-storage-blob-go/azblob/chunkwriting.go:179 +0x30
  github.com/Azure/azure-storage-blob-go/azblob.copyFromReader()
      /Users/progers/dev/src/github.com/grafana/loki/vendor/github.com/Azure/azure-storage-blob-go/azblob/chunkwriting.go:64 +0x360
  github.com/Azure/azure-storage-blob-go/azblob.UploadStreamToBlockBlob()
      /Users/progers/dev/src/github.com/grafana/loki/vendor/github.com/Azure/azure-storage-blob-go/azblob/highlevel.go:553 +0x104
  github.com/grafana/loki/pkg/storage/chunk/client/azure.(*BlobStorage).PutObject.func1()
      /Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/client/azure/blob_storage_client.go:290 +0x200
  github.com/grafana/loki/pkg/util/instrument.TimeRequest()
      /Users/progers/dev/src/github.com/grafana/loki/pkg/util/instrument/instrument.go:20 +0xac
  github.com/grafana/loki/pkg/storage/chunk/client/azure.(*BlobStorage).PutObject()
      /Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/client/azure/blob_storage_client.go:282 +0x134
  github.com/grafana/loki/pkg/storage/chunk/client/azure.Test_Hedging.func1()
      /Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/client/azure/blob_storage_client_test.go:99 +0x154
  github.com/grafana/loki/pkg/storage/chunk/client/azure.Test_Hedging.func4()
      /Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/client/azure/blob_storage_client_test.go:148 +0x278
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1595 +0x1b0
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1648 +0x40

Goroutine 37 (running) created at:
  testing.(*T).Run()
      /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1648 +0x5e8
  github.com/grafana/loki/pkg/storage/chunk/client/azure.Test_Hedging()
      /Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/client/azure/blob_storage_client_test.go:122 +0x190
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1595 +0x1b0
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1648 +0x40
==================
--- FAIL: Test_Hedging (0.26s)
    --- FAIL: Test_Hedging/delete/put/list_are_not_hedged (0.16s)
        testing.go:1465: race detected during execution of test
    testing.go:1465: race detected during execution of test
FAIL
FAIL	github.com/grafana/loki/pkg/storage/chunk/client/azure	0.742s
FAIL

After fix:

go test -race -count=10 .
ok  	github.com/grafana/loki/pkg/storage/chunk/client/azure	4.177s

Which issue(s) this PR fixes:
Relates to: #8586

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
    • If the change is worth mentioning in the release notes, add add-to-release-notes label
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@paul1r paul1r requested a review from a team as a code owner March 22, 2024 19:56
MichelHollands
MichelHollands approved these changes Mar 25, 2024
View reviewed changes
Copy link
Contributor

@MichelHollands MichelHollands left a comment

Choose a reason for hiding this comment

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

LGTM

@paul1r paul1r merged commit bd2ee0b into main Mar 25, 2024
12 checks passed
@paul1r paul1r deleted the paul1r/azure_blob_sdk_update branch March 25, 2024 14:00
rhnasc pushed a commit to inloco/loki that referenced this pull request Apr 12, 2024
@paul1r @rhnasc
test: Update azure-storage-blob-go to include a data race fix (grafan…
d5ff080 
…a#12325)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichelHollands MichelHollands MichelHollands approved these changes

Assignees
No one assigned
Labels
size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@paul1r @MichelHollands