Skip to content

fix: TSI concurrent write and iteration fatal errors#27344

Merged
gwossum merged 1 commit intomaster-1.xfrom
gw/27343/concurrent_map_iter_write
Apr 9, 2026
Merged

fix: TSI concurrent write and iteration fatal errors#27344
gwossum merged 1 commit intomaster-1.xfrom
gw/27343/concurrent_map_iter_write

Conversation

@gwossum
Copy link
Copy Markdown
Member

@gwossum gwossum commented Apr 9, 2026

Fix fatal error: concurrent map iteration and map write errors in TSI code. This appears to have been introduced with #27343. This was a backport from main-2.x. A PR comment in #27343 indicates this PR is not necessary for correctness in master-1.x at this time.

To fix the issue, #27343 is rolled back. A test
(TestLogFile_TagValueIterator_ConcurrentWrite) is able consistently reproduce the error when #27343 is present.

This bug was introduced in v1.12.3 and does impact previous versions.

Closes: #27343

Fix `fatal error: concurrent map iteration and map write` errors in TSI
code. This appears to have been introduced with #27343. This was a
backport from main-2.x. A PR comment in #27343 indicates this PR is not
necessary for correctness in master-1.x at this time.

To fix the issue, #27343 is rolled back. A test
(TestLogFile_TagValueIterator_ConcurrentWrite) is able consistently
reproduce the error when #27343 is present.

This bug was introduced in v1.12.3 and does impact previous versions.

Closes: #27343
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a fatal error: concurrent map iteration and map write panic in TSI log file tag value iteration by ensuring the relevant read lock is held during snapshot creation, and adds a regression test that reliably reproduces the issue when the lock is released too early.

Changes:

  • Hold LogFile’s RLock for the full duration of TagValueIterator()’s snapshot/iterator creation.
  • Add a concurrent writer/reader regression test for TagValueIterator() vs AddSeriesList().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tsdb/index/tsi1/log_file.go Keeps RLock held while building the tag value iterator snapshot to prevent concurrent map iteration/write panics.
tsdb/index/tsi1/log_file_test.go Adds a regression test that concurrently mutates and iterates tag values to catch the panic reliably.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +401 to +406
// Ensure TagValueIterator is safe to call concurrently with AddSeriesList.
// Regression test for a "concurrent map iteration and map write" fatal
// introduced when LogFile.TagValueIterator released its RLock before the
// underlying logTagKey.tagValues map was snapshotted. This is able to
// consistently reproduce the fatal error introduced by #26372.
func TestLogFile_TagValueIterator_ConcurrentWrite(t *testing.T) {
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

The regression test comment references "#26372", but the PR description/issue being addressed is #27343. This looks like a typo and may confuse future readers trying to trace the original regression; please update the reference to the correct issue/PR number.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The comment is correct.

Copy link
Copy Markdown
Contributor

@davidby-influx davidby-influx left a comment

Choose a reason for hiding this comment

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

If the test fails, it panics, and the test harness recovers?

Copy link
Copy Markdown
Contributor

@davidby-influx davidby-influx left a comment

Choose a reason for hiding this comment

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

LGTM

@gwossum gwossum marked this pull request as ready for review April 9, 2026 18:15
@gwossum
Copy link
Copy Markdown
Member Author

gwossum commented Apr 9, 2026

If the test fails, it panics, and the test harness recovers?

Yes, the Go testing framework will recover.

@gwossum gwossum changed the title fix: TSI concurrenty write and iteration fatal errors fix: TSI concurrent write and iteration fatal errors Apr 9, 2026
@gwossum gwossum merged commit 7157a36 into master-1.x Apr 9, 2026
14 checks passed
@gwossum gwossum deleted the gw/27343/concurrent_map_iter_write branch April 9, 2026 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants