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

Fix possible 01506_buffer_table_alter_block_structure_2 flakiness #35249

Conversation

azat
Copy link
Collaborator

@azat azat commented Mar 13, 2022

SELECT from Buffer table is racy, so you can get data from the
underlying table but not from the Buffer itself, since in parallel with
SELECT, Buffer, can flush it's data to the underlying table.

It is hard to avoid with the current architecture, since this will
require to holding lock until the data will be read from the Buffer, and
this is not a good alternative.

So let's fix the test instead, but not relying on background flush (TTL
increased).

Here is an example of a test failure 1:

2022.03.12 20:56:58.141182 [ 678 ] {011e7d25-82a9-4ab6-8cb0-dcbbc84f9581} <Debug> executeQuery: (from [::1]:33324) (comment: 01506_buffer_table_alter_block_structure_2.sql) SELECT * FROM buf ORDER BY timestamp;
2022.03.12 20:56:58.162709 [ 678 ] {011e7d25-82a9-4ab6-8cb0-dcbbc84f9581} <Trace> MergeTreeInOrderSelectProcessor: Reading 1 ranges in order from part 20200101_1_1_0, approx. 1 rows starting from 0
2022.03.12 20:56:59.144663 [ 615 ] {} <Trace> test_bdtzgu.buf_dest (79ba36b2-0e90-4bbb-b55f-a42b605b362b): Renaming temporary part tmp_insert_20200101_2_2_0 to 20200101_2_2_0.
2022.03.12 20:56:59.147550 [ 615 ] {} <Debug> StorageBuffer (test_bdtzgu.buf): Flushing buffer with 1 rows, 18 bytes, age 1 seconds, took 19 ms (bg).
2022.03.12 20:56:59.391774 [ 678 ] {011e7d25-82a9-4ab6-8cb0-dcbbc84f9581} <Information> executeQuery: Read 1 rows, 13.00 B in 1.250102785 sec., 0 rows/sec., 10.40 B/sec.

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Cc: @alexey-milovidov
Cc: @KochetovNicolai

SELECT from Buffer table is racy, so you can get data from the
underlying table but not from the Buffer itself, since in parallel with
SELECT, Buffer, can flush it's data to the underlying table.

It is hard to avoid with the current architecture, since this will
require to holding lock until the data will be read from the Buffer, and
this is not a good alternative.

So let's fix the test instead, but not relying on background flush (TTL
increased).

Here is an example of a test failure [1]:

    2022.03.12 20:56:58.141182 [ 678 ] {011e7d25-82a9-4ab6-8cb0-dcbbc84f9581} <Debug> executeQuery: (from [::1]:33324) (comment: 01506_buffer_table_alter_block_structure_2.sql) SELECT * FROM buf ORDER BY timestamp;
    2022.03.12 20:56:58.162709 [ 678 ] {011e7d25-82a9-4ab6-8cb0-dcbbc84f9581} <Trace> MergeTreeInOrderSelectProcessor: Reading 1 ranges in order from part 20200101_1_1_0, approx. 1 rows starting from 0
    2022.03.12 20:56:59.144663 [ 615 ] {} <Trace> test_bdtzgu.buf_dest (79ba36b2-0e90-4bbb-b55f-a42b605b362b): Renaming temporary part tmp_insert_20200101_2_2_0 to 20200101_2_2_0.
    2022.03.12 20:56:59.147550 [ 615 ] {} <Debug> StorageBuffer (test_bdtzgu.buf): Flushing buffer with 1 rows, 18 bytes, age 1 seconds, took 19 ms (bg).
    2022.03.12 20:56:59.391774 [ 678 ] {011e7d25-82a9-4ab6-8cb0-dcbbc84f9581} <Information> executeQuery: Read 1 rows, 13.00 B in 1.250102785 sec., 0 rows/sec., 10.40 B/sec.

  [1]: https://s3.amazonaws.com/clickhouse-test-reports/0/044cd6b861c1f4f00c6c24c4020799b676de6d34/stateless_tests__memory__actions__[1/3].html

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
@robot-clickhouse robot-clickhouse added the pr-not-for-changelog This PR should not be mentioned in the changelog label Mar 13, 2022
@alexey-milovidov alexey-milovidov self-assigned this Mar 14, 2022
@alexey-milovidov alexey-milovidov merged commit eb11929 into ClickHouse:master Mar 14, 2022
@azat azat deleted the fix-01506_buffer_table_alter_block_structure_2 branch March 14, 2022 11:33
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