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 "Unexpected packet Data received from client" for Distributed queries #17254

Merged
merged 4 commits into from Nov 26, 2020

Conversation

azat
Copy link
Collaborator

@azat azat commented Nov 21, 2020

Changelog category (leave one):

  • Bug Fix

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix possible Unexpected packet Data received from client error for Distributed queries with LIMIT.

Distributed query consist from multiple packets:

  • Query
  • Data (multiple times)

And it cannot be canceled in the middle of the send query, since there are multiple packages, and after the Cancel packet none Data packet can be sent, otherwise the remote side will throw:

Unexpected packet Data received from client

This should fix 01247_optimize_distributed_group_by_sharding_key/01247_optimize_distributed_group_by_sharding_key flaky test

Refs: #17006

…h it

Otherwise flaky tests will run then both, not a problem, but this is
just to trigger flaky check, since there is some tricky issue [1]:

    2020-11-20 00:35:51 01247_optimize_distributed_group_by_sharding_key:                       [ FAIL ] 0.67 sec. - return code 101
    2020-11-20 00:35:51 Received exception from server (version 20.12.1):
    2020-11-20 00:35:51 Code: 101. DB::Exception: Received from localhost:9000. DB::Exception: Received from 127.0.0.2:9000. DB::Exception: Unexpected packet Data received from client.

  [1]: https://clickhouse-test-reports.s3.yandex.net/16996/8d71564056925df1415880f382aaa169cbdf37b0/functional_stateless_tests_flaky_check_(address)/test_run.txt.out.log
So now you will get additional message:

    2020.11.21 13:37:15.429767 [ 380840 ] {47e1540d-f4cd-4f2f-9383-f1216e8328dc} <Trace> StorageDistributed (dist_01247): (127.0.0.2:9000) Cancelling query
@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Nov 21, 2020
@azat azat marked this pull request as draft November 21, 2020 10:42
@azat azat force-pushed the fix-dist-query-cancelation branch 3 times, most recently from bf97343 to b7e2614 Compare November 21, 2020 21:36
@azat
Copy link
Collaborator Author

azat commented Nov 21, 2020

@KochetovNicolai this is similar to #17006, can you also take a look?

@azat azat marked this pull request as ready for review November 21, 2020 21:40
@azat azat marked this pull request as draft November 22, 2020 06:04
Fix query cancelation in case of Distributed queries with LIMIT (when
the initator does not required to read all the data), since this cannot
be done until the query was sent (from the Query packet up to the empty
data Block), otherwise you will get:

    2020.11.21 21:47:23.297161 [ 184 ] {} <Error> TCPHandler: Code: 101, e.displayText() = DB::Exception: Unexpected packet Data received from client, Stack trace:

    0. /build/obj-x86_64-linux-gnu/../contrib/libcxx/include/exception:129: Poco::Exception::Exception(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int) @ 0x244f5bc9 in /usr/bin/clickhouse
    1. /build/obj-x86_64-linux-gnu/../src/Common/Exception.cpp:40: DB::Exception::Exception(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int) @ 0xa14a421 in /usr/bin/clickhouse
    2. /build/obj-x86_64-linux-gnu/../src/Common/NetException.h:0: DB::TCPHandler::receiveUnexpectedData() @ 0x1e032a74 in /usr/bin/clickhouse
    3. /build/obj-x86_64-linux-gnu/../src/Server/TCPHandler.cpp:824: DB::TCPHandler::receivePacket() @ 0x1e024685 in /usr/bin/clickhouse
    4. /build/obj-x86_64-linux-gnu/../src/Server/TCPHandler.cpp:178: DB::TCPHandler::runImpl() @ 0x1e01736b in /usr/bin/clickhouse
    5. /build/obj-x86_64-linux-gnu/../src/Server/TCPHandler.cpp:0: DB::TCPHandler::run() @ 0x1e035c1b in /usr/bin/clickhouse
    6. /build/obj-x86_64-linux-gnu/../contrib/poco/Net/src/TCPServerConnection.cpp:57: Poco::Net::TCPServerConnection::start() @ 0x243559cf in /usr/bin/clickhouse
    7. /build/obj-x86_64-linux-gnu/../contrib/poco/Net/src/TCPServerDispatcher.cpp:0: Poco::Net::TCPServerDispatcher::run() @ 0x24356521 in /usr/bin/clickhouse
    8. /build/obj-x86_64-linux-gnu/../contrib/poco/Foundation/src/ThreadPool.cpp:0: Poco::PooledThread::run() @ 0x24609175 in /usr/bin/clickhouse
    9. /build/obj-x86_64-linux-gnu/../contrib/poco/Foundation/src/Thread_POSIX.cpp:0: Poco::ThreadImpl::runnableEntry(void*) @ 0x24603cb7 in /usr/bin/clickhouse
    10. start_thread @ 0x9669 in /usr/lib/x86_64-linux-gnu/libpthread-2.30.so
    11. __clone @ 0x1222b3 in /usr/lib/x86_64-linux-gnu/libc-2.30.so
@azat azat marked this pull request as ready for review November 22, 2020 09:46
///
/// Unexpected packet Data received from client
///
std::lock_guard guard(was_cancelled_mutex);
Copy link
Member

Choose a reason for hiding this comment

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

It is ok, but not clear why cancel_mutex from MultiplexedConnections does not help:

void MultiplexedConnections::sendQuery(
const ConnectionTimeouts & timeouts,
const String & query,
const String & query_id,
UInt64 stage,
const ClientInfo & client_info,
bool with_pending_data)
{
std::lock_guard lock(cancel_mutex);

Probably, because we send empty block here:

I suppose it may be more logical to add sendScalars() and sendExternalTables logic into MultiplexedConnections::sendQuery.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably, because we send empty block here:

Indeed.

I suppose it may be more logical to add sendScalars() and sendExternalTables logic into MultiplexedConnections::sendQuery.

Agree

@alexey-milovidov alexey-milovidov merged commit fb3a69b into ClickHouse:master Nov 26, 2020
robot-clickhouse pushed a commit that referenced this pull request Nov 26, 2020
@azat azat deleted the fix-dist-query-cancelation branch November 26, 2020 19:00
vitlibar pushed a commit that referenced this pull request Nov 30, 2020
Backport #17254 to 20.12: Fix "Unexpected packet Data received from client"  for Distributed queries
@yangmeng66
Copy link

Hello, everyone, I used clickhouse's third-party JDBC driver (ClickHouse-Native-JDBC) to report an error when importing data. Can anyone explain why?

image

@alexey-milovidov
Copy link
Member

ClickHouse-Native-JDBC is unsupported, migrate to ClickHouse-JDBC.

@yyw105
Copy link

yyw105 commented Jan 27, 2022

Hello, everyone, I used clickhouse's third-party JDBC driver (ClickHouse-Native-JDBC) to report an error when importing data. Can anyone explain why?

image

@yangmeng66 hi guys. I meet too. Did you finally work it out?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix Pull request with bugfix, not backported by default
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants