Skip to content

Fix kafka storage does not work with arrow and arrowstream format messages#23415

Merged
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
godliness:fix-kafka-with-arrow
Apr 22, 2021
Merged

Fix kafka storage does not work with arrow and arrowstream format messages#23415
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
godliness:fix-kafka-with-arrow

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 arrow and arrowstream format messages

Detailed description / Documentation draft:

Before:

prepareReader create file_reader or stream_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 force-pushed the fix-kafka-with-arrow branch from b16a253 to 5354951 Compare April 21, 2021 10:09
@godliness godliness force-pushed the fix-kafka-with-arrow branch from 5354951 to 713eb94 Compare April 21, 2021 11:44
@filimonov
Copy link
Copy Markdown
Contributor

filimonov commented Apr 21, 2021

LGTM. So that prepareReader was opening and trying to read the buffer in the constructor. It was obviously wrong. Such operations should not happen at construction time, generate is a proper place for that. Thanks!

Let's just wait for tests to pass.

@godliness
Copy link
Copy Markdown
Contributor Author

godliness commented Apr 22, 2021

Let's just wait for tests to pass.

Thanks for your reviewed.

Checked the failed test, did not find any information related to this PR, so i think it's other reasons occur these test failed.

PTAL

@alexey-milovidov alexey-milovidov merged commit 1fcf198 into ClickHouse:master Apr 22, 2021
@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Apr 22, 2021 •
edited
Loading

Integration tests (thread) — Timeout

This is just a timeout (tests take too long time).

Integration tests (asan) — fail: 1
test_polymorphic_parts/test.py::test_in_memory_wal

This fails currently in master, acknowledged.

SQLancer test — SQLancer test run. See report

This is a brand new check that we added a few days ago.
It does not pass right now, one of the fixes is here: #23359

@godliness
Copy link
Copy Markdown
Contributor Author

godliness commented Apr 22, 2021

Integration tests (thread) — Timeout

This is just a timeout (tests take too long time).

Integration tests (asan) — fail: 1
test_polymorphic_parts/test.py::test_in_memory_wal

This fails currently in master, acknowledged.

SQLancer test — SQLancer test run. See report

This is a brand new check that we added a few days ago.
It does not pass right now, one of the fixes is here: #23359

Learned it!

Thanks!

@godliness godliness mentioned this pull request Apr 22, 2021
Merged
This was referenced Apr 22, 2021
Merged
Merged
Merged
robot-clickhouse pushed a commit that referenced this pull request Apr 22, 2021
Backport #23415 to 21.5: Fix kafka storage does not work with arrow a…
19b80fc 
…nd arrowstream format messages
@robot-clickhouse robot-clickhouse mentioned this pull request Apr 22, 2021
Merged
robot-clickhouse pushed a commit that referenced this pull request Apr 22, 2021
Backport #23415 to 21.4: Fix kafka storage does not work with arrow a…
71c4d6b 
…nd arrowstream format messages
@robot-clickhouse robot-clickhouse mentioned this pull request Apr 22, 2021
Merged
robot-clickhouse pushed a commit that referenced this pull request Apr 22, 2021
Backport #23415 to 21.3: Fix kafka storage does not work with arrow a…
5a43005 
…nd arrowstream format messages
@robot-clickhouse robot-clickhouse mentioned this pull request Apr 22, 2021
Merged
kitaisreal added a commit that referenced this pull request Apr 23, 2021
@kitaisreal
Merge pull request #23478 from ClickHouse/backport/21.4/23415
9c91cd2 
Backport #23415 to 21.4: Fix kafka storage does not work with arrow and arrowstream format messages
kitaisreal added a commit that referenced this pull request Apr 23, 2021
@kitaisreal
Merge pull request #23477 from ClickHouse/backport/21.5/23415
6bad3d3 
Backport #23415 to 21.5: Fix kafka storage does not work with arrow and arrowstream format messages
kitaisreal added a commit that referenced this pull request Apr 23, 2021
@kitaisreal
Merge pull request #23479 from ClickHouse/backport/21.3/23415
72edd8b 
Backport #23415 to 21.3: Fix kafka storage does not work with arrow and arrowstream format messages
@robot-clickhouse robot-clickhouse mentioned this pull request May 28, 2021
Closed
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

@filimonov filimonov

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.

5 participants

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