tsdb: use defer to release locks in mmapHeadChunks#18422
Draft
roidelapluie wants to merge 9 commits intoprometheus:mainfrom
Draft
tsdb: use defer to release locks in mmapHeadChunks#18422roidelapluie wants to merge 9 commits intoprometheus:mainfrom
roidelapluie wants to merge 9 commits intoprometheus:mainfrom
Conversation
handleChunkWriteError panics on chunk disk mapper errors, which occurs under i386 in TestDiskFillingUpAfterDisablingOOO. Without defer, the series mutex held inside mmapHeadChunks was not released during panic unwinding. A subsequent Close() call (e.g. from t.Cleanup) also calls mmapHeadChunks and deadlocks trying to re-acquire the same lock. Extract the per-stripe and per-series iterations into dedicated helpers so that defer can release both the stripe read lock and the series mutex during panic unwinding. Update the test comment to reflect that the deadlock is now resolved. See prometheus#17941 Signed-off-by: Julien Pivotto <[email protected]>
Signed-off-by: Julien Pivotto <[email protected]>
Signed-off-by: Julien Pivotto <[email protected]>
Signed-off-by: Julien Pivotto <[email protected]>
Signed-off-by: Julien Pivotto <[email protected]>
Signed-off-by: Julien Pivotto <[email protected]>
The test only writes ~80 samples, so the default 512MB chunk segment pre-allocation during compaction is unnecessary. Use 1MB instead to avoid large file allocations on constrained CI environments. Signed-off-by: Julien Pivotto <[email protected]>
Running count=500 sequentially on Windows causes SetFileInformationByHandle and FlushFileBuffers to hang, due to I/O pressure from the 128MB head chunk file preallocation on Windows after hundreds of iterations. Use 3 parallel matrix workers with count=50 each to distribute the load while preserving coverage. Signed-off-by: Julien Pivotto <[email protected]>
Signed-off-by: Julien Pivotto <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
handleChunkWriteError panics on chunk disk mapper errors, which occurs in TestDiskFillingUpAfterDisablingOOO. Without defer, the series mutex held inside mmapHeadChunks was not released during panic unwinding. A subsequent Close() call (e.g. from t.Cleanup) also calls mmapHeadChunks and deadlocks trying to re-acquire the same lock.
Extract the per-stripe and per-series iterations into dedicated helpers so that defer can release both the stripe read lock and the series mutex during panic unwinding.
Update the test comment to reflect that the deadlock is now resolved.
See #17941
Which issue(s) does the PR fix:
Release notes for end users (ALL commits must be considered).
Reviewers should verify clarity and quality.