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

Fixed panic on concurrent context key map write #1284

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

Wang
Copy link
Contributor

@Wang Wang commented Apr 25, 2024

Summary

When open a ClickHouse DB connection through the HTTP protocol and using the WithDeadline context during transaction execution, there is a small probability of concurrent writes to a map, which can lead to a panic. The reason is that when the PrepareContext function is called, it creates a goroutine to read data through the readData function. The readData function updates the key of “max_execution_time” in the map by calling the queryOptions function. Finally, when the transaction executes the Commit, it also calls the Send function, which in turn calls the queryOptions function to update the key of ”max_execution_time“ in the map.

fatal error: concurrent map writes
goroutine 4257644 [running]:
[github.com/ClickHouse/clickhouse-go/v2.queryOptions](http://github.com/ClickHouse/clickhouse-go/v2.queryOptions)({0x2219688, 0xc0238c33e0})
        /home/test/go1.19/global/pkg/mod/[github.com/!click!house/clickhouse-go/[email protected]/context.go:163](http://github.com/!click!house/clickhouse-go/[email protected]/context.go:163) +0x196
[github.com/ClickHouse/clickhouse-go/v2.(*httpConnect).readData](http://github.com/ClickHouse/clickhouse-go/v2.(*httpConnect).readData)(0xc01322e9b0, {0x2219688?, 0xc0238c33e0?}, 0xc031b80f00)
        /home/test/go1.19/global/pkg/mod/[github.com/!click!house/clickhouse-go/[email protected]/conn_http.go:367](http://github.com/!click!house/clickhouse-go/[email protected]/conn_http.go:367) +0x85
[github.com/ClickHouse/clickhouse-go/v2.(*httpConnect).query.func2()](http://github.com/ClickHouse/clickhouse-go/v2.(*httpConnect).query.func2())
        /home/test/go1.19/global/pkg/mod/[github.com/!click!house/clickhouse-go/[email protected]/conn_http_query.go:100](http://github.com/!click!house/clickhouse-go/[email protected]/conn_http_query.go:100) +0x8c
created by [github.com/ClickHouse/clickhouse-go/v2.(*httpConnect).query](http://github.com/ClickHouse/clickhouse-go/v2.(*httpConnect).query)
        /home/test/go1.19/global/pkg/mod/[github.com/!click!house/clickhouse-go/[email protected]/conn_http_query.go:98](http://github.com/!click!house/clickhouse-go/[email protected]/conn_http_query.go:98) +0x7ec

@CLAassistant
Copy link

CLAassistant commented Apr 25, 2024

CLA assistant check
All committers have signed the CLA.

@jkaflik jkaflik self-requested a review April 25, 2024 09:39
@jkaflik jkaflik changed the title fixed panic concurrent map wirtes Fixed panic on concurrent context key map write Apr 25, 2024
@jkaflik jkaflik merged commit ff97cb8 into ClickHouse:main Apr 25, 2024
10 checks passed
@Wang Wang deleted the fix-panic-0425 branch April 25, 2024 10:39
codeboten referenced this pull request in open-telemetry/opentelemetry-collector-contrib Jul 10, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[github.com/ClickHouse/clickhouse-go/v2](https://togithub.com/ClickHouse/clickhouse-go)
| `v2.23.0` -> `v2.26.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fClickHouse%2fclickhouse-go%2fv2/v2.26.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fClickHouse%2fclickhouse-go%2fv2/v2.26.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fClickHouse%2fclickhouse-go%2fv2/v2.23.0/v2.26.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fClickHouse%2fclickhouse-go%2fv2/v2.23.0/v2.26.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>ClickHouse/clickhouse-go
(github.com/ClickHouse/clickhouse-go/v2)</summary>

###
[`v2.26.0`](https://togithub.com/ClickHouse/clickhouse-go/blob/HEAD/CHANGELOG.md#v2260-2024-06-25----Release-notes-generated-using-configuration-in-githubreleaseyml-at-main---)

[Compare
Source](https://togithub.com/ClickHouse/clickhouse-go/compare/v2.25.0...v2.26.0)

#### What's Changed

##### Enhancements 🎉

- Reintroduce the random connection strategy by
[@&#8203;larry-cdn77](https://togithub.com/larry-cdn77) in
[https://github.com/ClickHouse/clickhouse-go/pull/1313](https://togithub.com/ClickHouse/clickhouse-go/pull/1313)
- Make custom debug log function on-par with the built-in one by
[@&#8203;vespian](https://togithub.com/vespian) in
[https://github.com/ClickHouse/clickhouse-go/pull/1317](https://togithub.com/ClickHouse/clickhouse-go/pull/1317)
- Remove date overflow check as it's normalised by ClickHouse server by
[@&#8203;gogingersnap777](https://togithub.com/gogingersnap777) in
[https://github.com/ClickHouse/clickhouse-go/pull/1315](https://togithub.com/ClickHouse/clickhouse-go/pull/1315)
- Batch: impl `Columns() []column.Interface` method by
[@&#8203;egsam98](https://togithub.com/egsam98) in
[https://github.com/ClickHouse/clickhouse-go/pull/1277](https://togithub.com/ClickHouse/clickhouse-go/pull/1277)

##### Fixes 🐛

- Fix rows.Close do not return too early by
[@&#8203;yujiarista](https://togithub.com/yujiarista) in
[https://github.com/ClickHouse/clickhouse-go/pull/1314](https://togithub.com/ClickHouse/clickhouse-go/pull/1314)
- Setting `X-Clickhouse-SSL-Certificate-Auth` header correctly given
`X-ClickHouse-Key` by
[@&#8203;gogingersnap777](https://togithub.com/gogingersnap777) in
[https://github.com/ClickHouse/clickhouse-go/pull/1316](https://togithub.com/ClickHouse/clickhouse-go/pull/1316)
- Retry on network errors and fix retries on async inserts with
`database/sql` interface by
[@&#8203;tommyzli](https://togithub.com/tommyzli) in
[https://github.com/ClickHouse/clickhouse-go/pull/1330](https://togithub.com/ClickHouse/clickhouse-go/pull/1330)
- BatchInsert parentheses issue fix by
[@&#8203;ramzes642](https://togithub.com/ramzes642) in
[https://github.com/ClickHouse/clickhouse-go/pull/1327](https://togithub.com/ClickHouse/clickhouse-go/pull/1327)

##### Other Changes 🛠

- ClickHouse 24.5 by [@&#8203;jkaflik](https://togithub.com/jkaflik) in
[https://github.com/ClickHouse/clickhouse-go/pull/1319](https://togithub.com/ClickHouse/clickhouse-go/pull/1319)
- Align `allow_suspicious_low_cardinality_types` and `
allow_suspicious_low_cardinality_types ` settings in tests due to
ClickHouse Cloud incompatibility by
[@&#8203;jkaflik](https://togithub.com/jkaflik) in
[https://github.com/ClickHouse/clickhouse-go/pull/1331](https://togithub.com/ClickHouse/clickhouse-go/pull/1331)
- Use HTTPs scheme in std connection failover tests by
[@&#8203;jkaflik](https://togithub.com/jkaflik) in
[https://github.com/ClickHouse/clickhouse-go/pull/1332](https://togithub.com/ClickHouse/clickhouse-go/pull/1332)

#### New Contributors

- [@&#8203;larry-cdn77](https://togithub.com/larry-cdn77) made their
first contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1313](https://togithub.com/ClickHouse/clickhouse-go/pull/1313)
- [@&#8203;vespian](https://togithub.com/vespian) made their first
contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1317](https://togithub.com/ClickHouse/clickhouse-go/pull/1317)
- [@&#8203;gogingersnap777](https://togithub.com/gogingersnap777) made
their first contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1315](https://togithub.com/ClickHouse/clickhouse-go/pull/1315)
- [@&#8203;yujiarista](https://togithub.com/yujiarista) made their first
contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1314](https://togithub.com/ClickHouse/clickhouse-go/pull/1314)
- [@&#8203;egsam98](https://togithub.com/egsam98) made their first
contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1277](https://togithub.com/ClickHouse/clickhouse-go/pull/1277)
- [@&#8203;tommyzli](https://togithub.com/tommyzli) made their first
contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1330](https://togithub.com/ClickHouse/clickhouse-go/pull/1330)
- [@&#8203;ramzes642](https://togithub.com/ramzes642) made their first
contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1327](https://togithub.com/ClickHouse/clickhouse-go/pull/1327)

**Full Changelog**:
ClickHouse/clickhouse-go@v2.25.0...v2.26.0

###
[`v2.25.0`](https://togithub.com/ClickHouse/clickhouse-go/blob/HEAD/CHANGELOG.md#v2250-2024-05-28----Release-notes-generated-using-configuration-in-githubreleaseyml-at-main---)

[Compare
Source](https://togithub.com/ClickHouse/clickhouse-go/compare/v2.24.0...v2.25.0)

#### What's Changed

##### Fixes 🐛

- Add a compatibility layer for a database/sql driver to work with
sql.NullString and ClickHouse nullable column by
[@&#8203;jkaflik](https://togithub.com/jkaflik) in
[https://github.com/ClickHouse/clickhouse-go/pull/1306](https://togithub.com/ClickHouse/clickhouse-go/pull/1306)

##### Other Changes 🛠

- Use Go 1.22 in head tests by
[@&#8203;jkaflik](https://togithub.com/jkaflik) in
[https://github.com/ClickHouse/clickhouse-go/pull/1305](https://togithub.com/ClickHouse/clickhouse-go/pull/1305)
- Skip flaky 1127 test by
[@&#8203;jkaflik](https://togithub.com/jkaflik) in
[https://github.com/ClickHouse/clickhouse-go/pull/1307](https://togithub.com/ClickHouse/clickhouse-go/pull/1307)

**Full Changelog**:
ClickHouse/clickhouse-go@v2.24.0...v2.25.0

###
[`v2.24.0`](https://togithub.com/ClickHouse/clickhouse-go/blob/HEAD/CHANGELOG.md#v2240-2024-05-08----Release-notes-generated-using-configuration-in-githubreleaseyml-at-main---)

[Compare
Source](https://togithub.com/ClickHouse/clickhouse-go/compare/v2.23.2...v2.24.0)

#### What's Changed

##### Enhancements 🎉

- Always compress responses when the client compression is on by
[@&#8203;zhkvia](https://togithub.com/zhkvia) in
[https://github.com/ClickHouse/clickhouse-go/pull/1286](https://togithub.com/ClickHouse/clickhouse-go/pull/1286)
- Optional flag to close query with flush by
[@&#8203;hongker](https://togithub.com/hongker) in
[https://github.com/ClickHouse/clickhouse-go/pull/1276](https://togithub.com/ClickHouse/clickhouse-go/pull/1276)

##### Fixes 🐛

- Fix prepare batch does not break on `values` substring in table name
by [@&#8203;Wang](https://togithub.com/Wang) in
[https://github.com/ClickHouse/clickhouse-go/pull/1290](https://togithub.com/ClickHouse/clickhouse-go/pull/1290)
- Fix nil checks when appending slices of pointers by
[@&#8203;markandrus](https://togithub.com/markandrus) in
[https://github.com/ClickHouse/clickhouse-go/pull/1283](https://togithub.com/ClickHouse/clickhouse-go/pull/1283)

##### Other Changes 🛠

- Don't recreate keys from LC columns from direct stream by
[@&#8203;genzgd](https://togithub.com/genzgd) in
[https://github.com/ClickHouse/clickhouse-go/pull/1291](https://togithub.com/ClickHouse/clickhouse-go/pull/1291)

#### New Contributors

- [@&#8203;zhkvia](https://togithub.com/zhkvia) made their first
contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1286](https://togithub.com/ClickHouse/clickhouse-go/pull/1286)

**Full Changelog**:
ClickHouse/clickhouse-go@v2.23.2...v2.24.0

###
[`v2.23.2`](https://togithub.com/ClickHouse/clickhouse-go/blob/HEAD/CHANGELOG.md#v2232-2024-04-25----Release-notes-generated-using-configuration-in-githubreleaseyml-at-main---)

[Compare
Source](https://togithub.com/ClickHouse/clickhouse-go/compare/v2.23.1...v2.23.2)

#### What's Changed

##### Fixes 🐛

- Fixed panic on concurrent context key map write by
[@&#8203;Wang](https://togithub.com/Wang) in
[https://github.com/ClickHouse/clickhouse-go/pull/1284](https://togithub.com/ClickHouse/clickhouse-go/pull/1284)

##### Other Changes 🛠

- Fix ClickHouse Terraform provider version by
[@&#8203;jkaflik](https://togithub.com/jkaflik) in
[https://github.com/ClickHouse/clickhouse-go/pull/1285](https://togithub.com/ClickHouse/clickhouse-go/pull/1285)

#### New Contributors

- [@&#8203;Wang](https://togithub.com/Wang) made their first
contribution in
[https://github.com/ClickHouse/clickhouse-go/pull/1284](https://togithub.com/ClickHouse/clickhouse-go/pull/1284)

**Full Changelog**:
ClickHouse/clickhouse-go@v2.23.1...v2.23.2

###
[`v2.23.1`](https://togithub.com/ClickHouse/clickhouse-go/blob/HEAD/CHANGELOG.md#v2231-2024-04-15----Release-notes-generated-using-configuration-in-githubreleaseyml-at-main---)

[Compare
Source](https://togithub.com/ClickHouse/clickhouse-go/compare/v2.23.0...v2.23.1)

#### What's Changed

##### Fixes 🐛

- Zero-value timestamp to be formatted as toDateTime(0) in bind by
[@&#8203;jkaflik](https://togithub.com/jkaflik) in
[https://github.com/ClickHouse/clickhouse-go/pull/1260](https://togithub.com/ClickHouse/clickhouse-go/pull/1260)

##### Other Changes 🛠

- Update
[#&#8203;1127](https://togithub.com/ClickHouse/clickhouse-go/issues/1127)
test case to reproduce a progress handle when exception is thrown by
[@&#8203;jkaflik](https://togithub.com/jkaflik) in
[https://github.com/ClickHouse/clickhouse-go/pull/1259](https://togithub.com/ClickHouse/clickhouse-go/pull/1259)
- Set max parallel for GH jobs by
[@&#8203;jkaflik](https://togithub.com/jkaflik) in
[https://github.com/ClickHouse/clickhouse-go/pull/1261](https://togithub.com/ClickHouse/clickhouse-go/pull/1261)
- Ensure test container termination by
[@&#8203;jkaflik](https://togithub.com/jkaflik) in
[https://github.com/ClickHouse/clickhouse-go/pull/1274](https://togithub.com/ClickHouse/clickhouse-go/pull/1274)

**Full Changelog**:
ClickHouse/clickhouse-go@v2.23.0...v2.23.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any
time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MTMuMiIsInVwZGF0ZWRJblZlciI6IjM3LjQyNS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJkZXBlbmRlbmNpZXMiLCJyZW5vdmF0ZWJvdCJdfQ==-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: opentelemetrybot <[email protected]>
Co-authored-by: Yang Song <[email protected]>
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