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: Fix deadlock in table_manager relating to Stop/SyncTables #12597

Merged
merged 1 commit into from
Apr 12, 2024
Merged

test: Fix deadlock in table_manager relating to Stop/SyncTables #12597

merged 1 commit into from
Apr 12, 2024

Conversation

paul1r
Copy link
Collaborator

@paul1r paul1r commented Apr 12, 2024

What this PR does / why we need it:
Before fix:

go test -race -count=100 -run TestStore_SyncStopInteraction  -timeout 10s
panic: test timed out after 10s
running tests:
	TestStore_SyncStopInteraction (10s)

goroutine 713 [running]:
testing.(*M).startAlarm.func1()
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:2259 +0x1e0
created by time.goFunc
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/time/sleep.go:176 +0x48

goroutine 1 [chan receive]:
testing.(*T).Run(0xc0012fab60, {0x104ce7f79, 0x1d}, 0x106f59258)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1649 +0x608
testing.runTests.func1(0x0?)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:2054 +0x84
testing.tRunner(0xc0012fab60, 0xc001ca7ae8)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1595 +0x1b4
testing.runTests(0xc0008701e0?, {0x1082fa8c0, 0x22, 0x22}, {0xc000c55b58?, 0x102a04d90?, 0x108310b60?})
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:2052 +0x6e8
testing.(*M).Run(0xc0008701e0)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1925 +0x9f0
main.main()
	_testmain.go:127 +0x298

goroutine 19 [select]:
github.com/baidubce/bce-sdk-go/util/log.NewLogger.func1()
	/Users/progers/dev/src/github.com/grafana/loki/vendor/github.com/baidubce/bce-sdk-go/util/log/logger.go:375 +0xec
created by github.com/baidubce/bce-sdk-go/util/log.NewLogger in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/vendor/github.com/baidubce/bce-sdk-go/util/log/logger.go:368 +0x254

goroutine 5 [select]:
go.opencensus.io/stats/view.(*worker).start(0xc0006d1300)
	/Users/progers/dev/src/github.com/grafana/loki/vendor/go.opencensus.io/stats/view/worker.go:292 +0x128
created by go.opencensus.io/stats/view.init.0 in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/vendor/go.opencensus.io/stats/view/worker.go:34 +0xf4

goroutine 90 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 92 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 91 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 93 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 94 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 95 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 96 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 97 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 130 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 131 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 132 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 133 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 134 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 135 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 136 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 137 [chan receive]:
github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.(*Fetcher).worker(0xc00039b680)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:104 +0x284
created by github.com/grafana/loki/v3/pkg/storage/chunk/fetcher.New in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/fetcher/fetcher.go:86 +0x248

goroutine 138 [select]:
github.com/grafana/loki/v3/pkg/storage/chunk/client/local.(*BoltIndexClient).loop(0xc00004fbd0)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/client/local/boltdb_index_client.go:82 +0x178
created by github.com/grafana/loki/v3/pkg/storage/chunk/client/local.NewBoltDBIndexClient in goroutine 1
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/chunk/client/local/boltdb_index_client.go:71 +0x1d0

goroutine 665 [semacquire]:
sync.runtime_Semacquire(0xc000713780?)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/runtime/sema.go:62 +0x2c
sync.(*WaitGroup).Wait(0xc000713778)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/sync/waitgroup.go:116 +0x7c
github.com/grafana/loki/v3/pkg/storage/stores/shipper/indexshipper/downloads.(*tableManager).Stop(0xc0007136c0)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/stores/shipper/indexshipper/downloads/table_manager.go:159 +0xa4
github.com/grafana/loki/v3/pkg/storage/stores/shipper/indexshipper.(*indexShipper).stop(0xc0006c4000)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/stores/shipper/indexshipper/shipper.go:252 +0x98
sync.(*Once).doSlow(0xc0006c4240, 0xc001331918)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/sync/once.go:74 +0xb4
sync.(*Once).Do(0xc0006c4240, 0x102a62708?)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/sync/once.go:65 +0x44
github.com/grafana/loki/v3/pkg/storage/stores/shipper/indexshipper.(*indexShipper).Stop(0xc0006c4000)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/stores/shipper/indexshipper/shipper.go:243 +0x5c
github.com/grafana/loki/v3/pkg/storage/stores/shipper/indexshipper/tsdb.(*store).Stop-fm.(*store).Stop.func1()
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/stores/shipper/indexshipper/tsdb/store.go:148 +0x200
sync.(*Once).doSlow(0xc0001d58a0, 0xc001331ac8)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/sync/once.go:74 +0xb4
sync.(*Once).Do(0xc0001d58a0, 0xc0001d57c0?)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/sync/once.go:65 +0x44
github.com/grafana/loki/v3/pkg/storage/stores/shipper/indexshipper/tsdb.(*store).Stop(...)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/stores/shipper/indexshipper/tsdb/store.go:142
github.com/grafana/loki/v3/pkg/storage.(*LokiStore).storeForPeriod.func2()
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/store.go:313 +0xa8
github.com/grafana/loki/v3/pkg/storage/stores.(*storeEntry).Stop(0xc00084c340)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/stores/composite_store_entry.go:201 +0x5c
github.com/grafana/loki/v3/pkg/storage/stores.CompositeStore.Stop(...)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/stores/composite_store.go:306
github.com/grafana/loki/v3/pkg/storage.TestStore_SyncStopInteraction(0xc0012fad00)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/store_test.go:2003 +0xb74
testing.tRunner(0xc0012fad00, 0x106f59258)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1595 +0x1b4
created by testing.(*T).Run in goroutine 1
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1648 +0x5ec

goroutine 747 [sync.RWMutex.RLock]:
sync.runtime_SemacquireRWMutexR(0xc000713740?, 0x0?, 0x0?)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/runtime/sema.go:82 +0x28
sync.(*RWMutex).RLock(0xc000713730)
	/opt/homebrew/Cellar/go/1.21.6/libexec/src/sync/rwmutex.go:71 +0x60
github.com/grafana/loki/v3/pkg/storage/stores/shipper/indexshipper/downloads.(*tableManager).syncTables(0xc0007136c0, {0x106f85608, 0xc0001d5900})
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/stores/shipper/indexshipper/downloads/table_manager.go:214 +0x4c
github.com/grafana/loki/v3/pkg/storage/stores/shipper/indexshipper/downloads.(*tableManager).loop(0xc0007136c0)
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/stores/shipper/indexshipper/downloads/table_manager.go:133 +0x4a8
created by github.com/grafana/loki/v3/pkg/storage/stores/shipper/indexshipper/downloads.NewTableManager in goroutine 665
	/Users/progers/dev/src/github.com/grafana/loki/pkg/storage/stores/shipper/indexshipper/downloads/table_manager.go:114 +0x408
exit status 2
FAIL	github.com/grafana/loki/v3/pkg/storage	10.856s

After fix:

go test -race -count=100 -run TestStore_SyncStopInteraction  -timeout 10s
PASS
ok  	github.com/grafana/loki/v3/pkg/storage	2.668s

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
  • 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 April 12, 2024 15:43
DylanGuedes
DylanGuedes approved these changes Apr 12, 2024
View reviewed changes
Copy link
Contributor

@DylanGuedes DylanGuedes left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm!

btw your fix is what is being done by the others Stop() implementations (/boltdb and /uploads)

@paul1r paul1r merged commit 13d45bc into main Apr 12, 2024
12 checks passed
@paul1r paul1r deleted the paul1r/fix_deadlock_in_table_manager branch April 12, 2024 19:50
@RodrigoCMoraes RodrigoCMoraes mentioned this pull request Jul 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DylanGuedes DylanGuedes DylanGuedes approved these changes

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

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