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

cherry pick upstream fix to memorize tsdb client #112

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

jnyi
Copy link
Collaborator

@jnyi jnyi commented Dec 13, 2024

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Verification

jnyi and others added 2 commits December 13, 2024 11:16
…thanos-io#7941)

* [Receive] fix race condition

Signed-off-by: Yi Jin <[email protected]>

* add a change log

Signed-off-by: Yi Jin <[email protected]>

* memorize tsdb local clients without race condition

Signed-off-by: Yi Jin <[email protected]>

* fix data race in testing with some concurrent safe helper functions

Signed-off-by: Yi Jin <[email protected]>

* address comments

Signed-off-by: Yi Jin <[email protected]>

---------

Signed-off-by: Yi Jin <[email protected]>
@jnyi jnyi requested review from yuchen-db and hczhu-db December 13, 2024 19:24
tenants := make([]string, 0, len(t.tenants))
for tname := range t.tenants {
tenants = append(tenants, tname)
}
defer t.mtx.RUnlock()
Copy link
Collaborator Author

@jnyi jnyi Dec 13, 2024

Choose a reason for hiding this comment

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

aha, found a potential bug cc @yuchen-db, you should defer Unlock right after, otherwise if there is a return statement in between, the lock won't be released ever.

Choose a reason for hiding this comment

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

thanks

@@ -1215,74 +1215,3 @@ func TestReceiveCpnp(t *testing.T) {
}, v)

}

func TestNewTenant(t *testing.T) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove this test introduced in #100 which didn't capture the bug anyway

tenants := make([]string, 0, len(t.tenants))
for tname := range t.tenants {
tenants = append(tenants, tname)
}
defer t.mtx.RUnlock()

Choose a reason for hiding this comment

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

thanks

@jnyi jnyi merged commit 01f4c87 into databricks:db_main Dec 13, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants