Skip to content

Fix !hasPendingData() failed assertion in hdfs read buffer#29276

Merged
kssenii merged 3 commits intoClickHouse:masterfrom
kssenii:hdfs-fix-assertion
Sep 27, 2021
Merged

Fix !hasPendingData() failed assertion in hdfs read buffer#29276
kssenii merged 3 commits intoClickHouse:masterfrom
kssenii:hdfs-fix-assertion

Conversation

@kssenii
Copy link
Copy Markdown
Member

@kssenii kssenii commented Sep 22, 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 (user-visible misbehaviour in official stable or prestable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix failed assertion in ReadBufferFromHDFS. Update libhdfs3 library to be able to run in tests in debug. Closes #29251. Closes #27814.

@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Sep 22, 2021
@robot-ch-test-poll2 robot-ch-test-poll2 added the submodule changed At least one submodule changed in this PR. label Sep 22, 2021
@kssenii kssenii marked this pull request as draft September 23, 2021 07:08
@kssenii kssenii marked this pull request as ready for review September 23, 2021 07:21
alesapin
alesapin approved these changes Sep 27, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@alesapin alesapin left a comment

Choose a reason for hiding this comment

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

Ok, but not clear why we have separate ReadBufferFromHDFSImpl at all?

@alesapin alesapin self-assigned this Sep 27, 2021
@kssenii
Copy link
Copy Markdown
Member Author

kssenii commented Sep 27, 2021 •
edited
Loading

Ok, but not clear why we have separate ReadBufferFromHDFSImpl at all?

For simplicity I guess. Reading from hdfs must be done into BufferWithOwnMemory and the hdfs buffer itself must be SeekableReadBuffer so without impl will have to have something like BufferWithOwnMemory<SeekableReadBuffer> and it does not look correct to have it this way.

@kssenii kssenii merged commit beda6be into ClickHouse:master Sep 27, 2021
@alesapin
Copy link
Copy Markdown
Member

alesapin commented Sep 27, 2021

Screenshot_2021-09-27_15-48-48
@kssenii Any comments for these failures?

@kssenii
Copy link
Copy Markdown
Member Author

kssenii commented Sep 27, 2021

Ok..

Functional stateless tests (debug) fail: 1 -- 02015_async_inserts_5 not related.

integration tests all failed with test_dictionaries_ddl/* not related and was fixed in #29365 and test_rocksdb_options/test.py::test_invalid_column_family_options is being fixed by azat.

Unit tests release gcc fail: 1 -- Executor.RemoveTasksStress -- not related

This was referenced Sep 27, 2021
Merged
Merged
Merged
Closed
Merged
robot-clickhouse pushed a commit that referenced this pull request Sep 27, 2021
Backport #29276 to 21.8: Fix !hasPendingData() failed assertion in hd…
ef84536 
…fs read buffer
@robot-clickhouse robot-clickhouse mentioned this pull request Sep 27, 2021
Merged
@robot-clickhouse robot-clickhouse mentioned this pull request Sep 27, 2021
Closed
robot-clickhouse pushed a commit that referenced this pull request Sep 27, 2021
Backport #29276 to 21.7: Fix !hasPendingData() failed assertion in hd…
374645f 
…fs read buffer
robot-clickhouse pushed a commit that referenced this pull request Sep 27, 2021
Backport #29276 to 21.10: Fix !hasPendingData() failed assertion in h…
65b24f7 
…dfs read buffer
@robot-clickhouse robot-clickhouse mentioned this pull request Sep 27, 2021
Merged
robot-clickhouse pushed a commit that referenced this pull request Sep 27, 2021
Backport #29276 to 21.9: Fix !hasPendingData() failed assertion in hd…
ac8ff01 
…fs read buffer
@robot-clickhouse robot-clickhouse mentioned this pull request Sep 27, 2021
Merged
kssenii added a commit that referenced this pull request Sep 28, 2021
@kssenii
Merge pull request #29447 from ClickHouse/backport/21.8/29276
ea9e533 
Backport #29276 to 21.8: Fix !hasPendingData() failed assertion in hdfs read buffer
kssenii added a commit that referenced this pull request Sep 28, 2021
@kssenii
Merge pull request #29451 from ClickHouse/backport/21.9/29276
2482f5b 
Backport #29276 to 21.9: Fix !hasPendingData() failed assertion in hdfs read buffer
kssenii added a commit that referenced this pull request Sep 28, 2021
@kssenii
Merge pull request #29450 from ClickHouse/backport/21.10/29276
40ca491 
Backport #29276 to 21.10: Fix !hasPendingData() failed assertion in hdfs read buffer
@tavplubix tavplubix mentioned this pull request Nov 3, 2021
Closed
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-bugfix Pull request with bugfix, not backported by default submodule changed At least one submodule changed in this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ClickHouse raise an Assertion when execute select on hdfs engine on 21.8.3-lts libhdfs3 use rand function from libc

4 participants

@kssenii @alesapin @robot-clickhouse @robot-ch-test-poll2