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

Fix stress test after #40420 #40608

Merged
merged 4 commits into from Aug 26, 2022
Merged

Fix stress test after #40420 #40608

merged 4 commits into from Aug 26, 2022

Conversation

kssenii
Copy link
Member

@kssenii kssenii commented Aug 25, 2022

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Broken in #40420.

@robot-ch-test-poll robot-ch-test-poll added the pr-not-for-changelog This PR should not be mentioned in the changelog label Aug 25, 2022
@nickitat nickitat mentioned this pull request Aug 25, 2022
Closed
@kssenii
Copy link
Member Author

kssenii commented Aug 26, 2022

Screenshot 2022-08-26 at 12 50 43

@kssenii kssenii mentioned this pull request Aug 26, 2022
Merged
@azat
Copy link
Collaborator

azat commented Aug 26, 2022

@kssenii But is this enough to fix the data-race (#40643 (comment)) ? there is still no lock for FileSegment::range()

@kssenii
Copy link
Member Author

kssenii commented Aug 26, 2022 •
edited

@azat, yes, because cache->getSnapshot() will lock cache->mutex inside before doing FileSegment::range()

@alesapin alesapin self-assigned this Aug 26, 2022
@kssenii
Copy link
Member Author

kssenii commented Aug 26, 2022

Screenshot 2022-08-26 at 22 53 53

@kssenii kssenii merged commit 0cf76da into master Aug 26, 2022
@kssenii kssenii deleted the kssenii-patch-4 branch August 26, 2022 20:54
@vdimir vdimir mentioned this pull request Aug 29, 2022
Merged
@kssenii kssenii mentioned this pull request Sep 27, 2022
Merged
@robot-ch-test-poll2 robot-ch-test-poll2 mentioned this pull request Sep 27, 2022
Merged
@robot-clickhouse robot-clickhouse mentioned this pull request Sep 28, 2022
Merged
robot-clickhouse pushed a commit that referenced this pull request Sep 28, 2022
Backport #40608 to 22.8: Fix stress test after #40420
1aa7a0b
@robot-clickhouse robot-clickhouse added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Sep 28, 2022
KochetovNicolai added a commit that referenced this pull request Sep 28, 2022
@KochetovNicolai
Merge pull request #41864 from ClickHouse/backport/22.8/40608
8965fe9 
Backport #40608 to 22.8: Fix stress test after #40420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alesapin alesapin alesapin approved these changes

Assignees

@alesapin alesapin

Labels
pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-not-for-changelog This PR should not be mentioned in the changelog v22.8-must-backport
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@kssenii @azat @alesapin @robot-clickhouse @robot-ch-test-poll