Skip to content

Fix kafka storage does not work with parquet format messages#23412

Merged
kitaisreal merged 3 commits intoClickHouse:masterfrom
godliness:fix-kafka-with-parquet
May 15, 2021
Merged

Fix kafka storage does not work with parquet format messages#23412
kitaisreal merged 3 commits intoClickHouse:masterfrom
godliness:fix-kafka-with-parquet

Conversation

@godliness
Copy link
Copy Markdown
Contributor

@godliness godliness commented Apr 21, 2021 •
edited
Loading

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Bug Fix

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Kafka storage may support parquet format messages

Detailed description / Documentation draft:

Before:

prepareReader create file_reader before kafka messages received and available

After:

Put prepareReader into generate, when generate called kafka messages should be available

@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Apr 21, 2021
@godliness godliness changed the title Fix kafka storage not work with parquet format massages Fix kafka storage does not work with parquet format messages Apr 21, 2021
@godliness godliness force-pushed the fix-kafka-with-parquet branch from 1dd97ac to f689308 Compare April 21, 2021 06:07
filimonov
filimonov approved these changes Apr 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@filimonov filimonov left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Lgtm. But check tests

@godliness
Copy link
Copy Markdown
Contributor Author

godliness commented Apr 22, 2021 •
edited
Loading

Lgtm. But check tests

Thanks @filimonov,

Other failed tests are not related to this PR.
Reasons: #23415 (comment)

test_kafka_engine_put_errors_to_stream_with_random_malformed_json test failed(timeout>300s)

image

query sent but can not get the answer back.

From now on, i hav no idea about why test_kafka_engine_put_errors_to_stream_with_random_malformed_json is failed, the format is JSONEachRow that is not related to 'Parquet' format, maybe it's due to they share the same kafka instance? but i can not to check the log info of clickhouse-server or kafka log in your CI environment, so i can not found the reason.

PTAL

@godliness godliness force-pushed the fix-kafka-with-parquet branch from f689308 to ad3de45 Compare April 23, 2021 01:23
@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Apr 24, 2021

Need help from @filimonov to fix flaky test.

@godliness godliness requested a review from filimonov April 26, 2021 07:09
@kitaisreal kitaisreal self-assigned this Apr 29, 2021
@kitaisreal
Copy link
Copy Markdown
Contributor

kitaisreal commented Apr 29, 2021

@filimonov can you take a look at broken asan test ?

@godliness godliness force-pushed the fix-kafka-with-parquet branch from ad3de45 to ec8ca5b Compare May 6, 2021 03:33
@godliness
Copy link
Copy Markdown
Contributor Author

godliness commented May 13, 2021

@kitaisreal @filimonov

Hello friends, what should i do next? ^ _ ^

@filimonov
Copy link
Copy Markdown
Contributor

filimonov commented May 13, 2021 •
edited
Loading

Sorry for delay @godliness

The failure does not look related to your changes (rather test is not stable #21850 ), but I still need to try to figure out how to fix it before merging. Had no time yet, but will try to look at that today.

filimonov
filimonov approved these changes May 13, 2021
View reviewed changes
@filimonov
Copy link
Copy Markdown
Contributor

filimonov commented May 14, 2021

@Mergifyio update

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented May 14, 2021

Command update: success

Branch has been successfully updated

@filimonov
Copy link
Copy Markdown
Contributor

filimonov commented May 15, 2021 •
edited
Loading

@kitaisreal the rest of CI failures are not Kafka / PR related.

@kitaisreal kitaisreal merged commit 3dd7401 into ClickHouse:master May 15, 2021
This was referenced May 15, 2021
Closed
Merged
Merged
robot-clickhouse pushed a commit that referenced this pull request May 15, 2021
Backport #23412 to 21.5: Fix kafka storage does not work with parquet…
56fc731 
… format messages
@robot-clickhouse robot-clickhouse mentioned this pull request May 15, 2021
Merged
kitaisreal added a commit that referenced this pull request May 15, 2021
@kitaisreal
Merge pull request #24156 from ClickHouse/backport/21.5/23412
9647ac1 
Backport #23412 to 21.5: Fix kafka storage does not work with parquet format messages
@robot-clickhouse robot-clickhouse mentioned this pull request May 28, 2021
Closed
robot-clickhouse pushed a commit that referenced this pull request Jun 21, 2021
Backport #23412 to 21.3: Fix kafka storage does not work with parquet…
492cae3 
… format messages
@robot-clickhouse robot-clickhouse mentioned this pull request Jun 21, 2021
Merged
vitlibar pushed a commit that referenced this pull request Jun 22, 2021
Merge pull request #25567 from ClickHouse/backport/21.3/23412
4d29ff8 
Backport #23412 to 21.3: Fix kafka storage does not work with parquet format messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@filimonov filimonov filimonov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kitaisreal kitaisreal

Labels

pr-bugfix Pull request with bugfix, not backported by default

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@godliness @alexey-milovidov @kitaisreal @filimonov @robot-clickhouse @kssenii