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

Un-flake 01079_new_range_reader_segfault #48934

Merged
merged 2 commits into from
Apr 20, 2023

Conversation

rschu1ze
Copy link
Member

@rschu1ze rschu1ze commented Apr 19, 2023

Usage of rand() means that the SELECT produces with probability 0.5^20 = 0.000000953 no result. This happend in (*).

The query is designed to trigger some specific logic in MergeTreeReader. Rewriting it is not a good idea. I therefore increased the amount of test data to reduce the probability of an empty result significantly. I hope that the same logic as before is triggered.

EDIT: Instead replaced rand() by the more deterministic rowNumberInBlock().

The test currently runs 507 times / day, this means this PR will prevent a failure in ca. 5.6 years from now. Happy 2028!

(*) https://s3.amazonaws.com/clickhouse-test-reports/0/905587b39def432667437efd92c28dde9cc0dfb2/stateless_tests__release__s3_storage__[1/2].html

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-not-for-changelog This PR should not be mentioned in the changelog label Apr 19, 2023
Usage of `rand()` means that the SELECT produces with probability
0.5^20 = 0.000000953 no result. This happend in (*).

The query is build to trigger some specific logic in MergeTreeReader.
Rewriting it is not a good idea. I therefore increased the amout of
test data to reduce the probability of en empty result siginficantly. I
hope that the same logic as before is triggered.

(*) https://s3.amazonaws.com/clickhouse-test-reports/0/905587b39def432667437efd92c28dde9cc0dfb2/stateless_tests__release__s3_storage__[1/2].html
@rschu1ze rschu1ze force-pushed the rs/unflake-01079_new_range_reader_segfault branch from c965756 to 1036d21 Compare April 19, 2023 09:54
@rschu1ze rschu1ze changed the title Less flaky 01079_new_range_reader_segfault Un-flake 01079_new_range_reader_segfault Apr 19, 2023
@nickitat nickitat self-assigned this Apr 19, 2023
@@ -3,7 +3,7 @@ drop table if exists t;
create table t (a Int) engine = MergeTree order by a;

-- some magic to satisfy conditions to run optimizations in MergeTreeRangeReader
insert into t select number < 20 ? 0 : 1 from numbers(50);
insert into t select number < 20000 ? 0 : 1 from numbers(50000);
alter table t add column s String default 'foo';

select s from t prewhere a != 1 where rand() % 2 = 0 limit 1;
Copy link
Member

Choose a reason for hiding this comment

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

maybe modify condition in to smth determenistic like where rowNumberInBlock() % 2 = 0?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, done.

@rschu1ze rschu1ze merged commit e543496 into master Apr 20, 2023
140 checks passed
@rschu1ze rschu1ze deleted the rs/unflake-01079_new_range_reader_segfault branch April 20, 2023 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-not-for-changelog This PR should not be mentioned in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants