Make Poco HTTP Server zero-copy again by abyss7 · Pull Request #19516 · ClickHouse/ClickHouse

abyss7 · 2021-01-23T17:16:08Z

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

Changelog category (leave one):

Performance Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Partially reimplement HTTP server to make it making less copies of incoming and outgoing data. It gives up to 1.5 performance improvement on inserting long records over HTTP.

abyss7 · 2021-02-11T12:21:26Z

Fuzzer (debug): #20046

alexey-milovidov · 2021-02-19T12:35:01Z

Ok.

Finally we can increase the 16KB limit on URL size. We can make it in next PR after merge.
The code in HTTP directory is based on Poco. We should put README.md file for attribution.
There can be dead code (unused methods) there (like sendFile). We can review and remove these methods.
while (in.read(ch) && !Poco::Ascii::isSpace(ch) && method.size() <= MAX_METHOD_LENGTH) method += ch; - we can introduce "limited read until" in ReadHelpers.
catch (Poco::Net::NoMessageException &) - const.
void stopAll(bool abortCurrent = false); - variable name is out of style.
wrapReadBufferReference maybe just return std::unique_ptr in request.getStream().
Sending responses still involving ostream. Decided to address it in subsequent PR.
in.peek(ch) - after introduction of this method we can search and replace all occurences of !in.eof() && *in.position() == ...

* master: (160 commits) Make Poco HTTP Server zero-copy again (ClickHouse#19516) Fixed documentation ccache 4.2+ does not requires any quirks for SOURCE_DATE_EPOCH Add a function `htmlOrXmlCoarseParse` to extract content from html or xml format string. (ClickHouse#19600) Reinterpret function added Decimal, DateTim64 support Add test Update InterpreterSelectQuery.cpp Improved serialization for data types combined of Arrays and Tuples. Improved matching enum data types to protobuf enum type. Fixed serialization of the Map data type. Omitted values are now set by default. Log stdout and stderr when failed to start docker in integration tests. Added comment Don't backport base commit of branch in the same branch (ClickHouse#20628) Fix fasttest retry for failed tests Dictionary create source with functions crash fix Added error reinterpretation tests Update run.sh Updated documentation fix subquery with limit Rename untyped function reinterpretAs into reinterpret ignore data store files Support vhost ...

* master: Add gdb to fasttest image Make Poco HTTP Server zero-copy again (ClickHouse#19516) Use fixed version for aerospike Fixed documentation ccache 4.2+ does not requires any quirks for SOURCE_DATE_EPOCH Reinterpret function added Decimal, DateTim64 support test/stress: fix permissions for clickhouse directories test/stress: improve backtrace catching on server failures test/stress: use clickhouse builtin start/stop to run server from the same user Added error reinterpretation tests Updated documentation Rename untyped function reinterpretAs into reinterpret # Conflicts: # src/Server/HTTPHandler.cpp # src/Server/HTTPHandler.h

* master: (153 commits) Add gdb to fasttest image Make Poco HTTP Server zero-copy again (ClickHouse#19516) Use fixed version for aerospike Fixed documentation ccache 4.2+ does not requires any quirks for SOURCE_DATE_EPOCH Add a function `htmlOrXmlCoarseParse` to extract content from html or xml format string. (ClickHouse#19600) Reinterpret function added Decimal, DateTim64 support test/stress: fix permissions for clickhouse directories test/stress: improve backtrace catching on server failures test/stress: use clickhouse builtin start/stop to run server from the same user Add test Update InterpreterSelectQuery.cpp Improved serialization for data types combined of Arrays and Tuples. Improved matching enum data types to protobuf enum type. Fixed serialization of the Map data type. Omitted values are now set by default. Log stdout and stderr when failed to start docker in integration tests. Added comment Don't backport base commit of branch in the same branch (ClickHouse#20628) Fix fasttest retry for failed tests Dictionary create source with functions crash fix Added error reinterpretation tests Update run.sh ...

filimonov · 2021-02-20T07:32:35Z

Why creating copy not changing the poco itself? (we already have a list of own patches to poco).

Not easy to see the diff with the current poco version.

Is there some chances to push that into the poco upstream?

alexey-milovidov · 2021-02-20T07:38:23Z

No chance.

The whole purpose of this patch is to replace iostreams with Read/WriteBuffers.

alexey-milovidov · 2021-02-20T07:39:28Z

Also we are looking into perspective to also rewrite TCPServer.

alexey-milovidov · 2021-02-20T09:06:03Z

@filimonov See #8441 (comment)

abyss7 · 2021-02-20T10:21:07Z

Why creating copy not changing the poco itself? (we already have a list of own patches to poco).

Not easy to see the diff with the current poco version.

Is there some chances to push that into the poco upstream?

At lease now we can remove our unused patches from Poco - for simpler upstream update in the future.

filimonov · 2021-02-25T19:20:31Z

Cool. Thanks, now it's clear. Makes 100% sence.

Follow up to #19516

Previously HTTP server does not tries to send exception if something had been written to the client already, but after rewriting to "zero-copy" HTTP implementation (back in ClickHouse#19516) this got broken, and you can get pretty "dangerous" at least at first glance errors, like: - Malformed chunked encoding - or most likely to CHECKSUM_DOESNT_MATCH error This is not crucial because it will never pass checksum checks, but still icky. So let's fix it. Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

abyss7 added 2 commits January 23, 2021 18:18

Refactoring: part 1

eba43c9

Refactoring: part 2

8e7a181

robot-clickhouse added the pr-performance Pull request with some performance improvements label Jan 23, 2021

abyss7 added 5 commits January 25, 2021 17:52

Handle request using ReadBuffer interface

d30a56c

Struggles with ReadBuffer's

a41c187

Merge remote-tracking branch 'upstream/master' into poco-http

1c67780

Fix URI parsing

a0e3ead

Implement parsing of multipart/form-data

9e41754

abyss7 force-pushed the poco-http branch from 448ec4e to 9e41754 Compare January 29, 2021 19:06

abyss7 added 5 commits February 5, 2021 21:45

Merge branch 'better-read-buffers-2' into poco-http

6240ce9

Merge branch 'better-read-buffers-4' into poco-http

db2d3e3

Merge remote-tracking branch 'upstream/master' into poco-http

6f93375

Check HTTP_LENGTH_REQUIRED before eof() or will hang

e5bef02

Fix HTTPChunkedReadBuffer

55ce804

abyss7 changed the title ~~[WIP] Make Poco HTTP Server zero-copy again~~ Make Poco HTTP Server zero-copy again Feb 8, 2021

abyss7 marked this pull request as ready for review February 8, 2021 15:34

abyss7 added 3 commits February 8, 2021 21:05

Fix build and style

1928b1e

Fix test

3a3a90a

Resist double-eof

c31215b

abyss7 added 3 commits February 11, 2021 15:39

Fix arcadian build

5691ac3

Merge remote-tracking branch 'upstream/master' into poco-http

dc96b5c

Merge remote-tracking branch 'upstream/master' into poco-http

c7b2886

tavplubix mentioned this pull request Feb 16, 2021

Better distributed DDL queue cleanup #20448

Merged

alexey-milovidov self-assigned this Feb 17, 2021

alexey-milovidov approved these changes Feb 19, 2021

View reviewed changes

abyss7 merged commit 414f470 into ClickHouse:master Feb 19, 2021

abyss7 mentioned this pull request Feb 19, 2021

Improve performance of HTTP interface #11305

Closed

alexey-milovidov mentioned this pull request Feb 20, 2021

Performance degradation when inserting long rows through HTTP #8441

Closed

abyss7 mentioned this pull request Mar 29, 2021

Follow up to #19516 #22323

Merged

alexey-milovidov added a commit that referenced this pull request Apr 2, 2021

Merge pull request #22323 from abyss7/poco-http2

9f5cd35

Follow up to #19516

alexey-milovidov mentioned this pull request Apr 12, 2021

Authorization via "Authorization" header fails when passing query via query-string #2719

Closed

azat mentioned this pull request Apr 25, 2024

Fix possible CHECKSUM_DOESNT_MATCH (and others) during replicated fetches #62987

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Poco HTTP Server zero-copy again#19516

Make Poco HTTP Server zero-copy again#19516
abyss7 merged 18 commits intoClickHouse:masterfrom
abyss7:poco-http

abyss7 commented Jan 23, 2021 •

edited by alexey-milovidov

Loading

Uh oh!

abyss7 commented Feb 11, 2021

Uh oh!

alexey-milovidov commented Feb 19, 2021 •

edited

Loading

Uh oh!

filimonov commented Feb 20, 2021

Uh oh!

alexey-milovidov commented Feb 20, 2021 •

edited

Loading

Uh oh!

alexey-milovidov commented Feb 20, 2021

Uh oh!

alexey-milovidov commented Feb 20, 2021

Uh oh!

abyss7 commented Feb 20, 2021

Uh oh!

filimonov commented Feb 25, 2021 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

abyss7 commented Jan 23, 2021 • edited by alexey-milovidov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abyss7 commented Feb 11, 2021

Uh oh!

alexey-milovidov commented Feb 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

filimonov commented Feb 20, 2021

Uh oh!

alexey-milovidov commented Feb 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexey-milovidov commented Feb 20, 2021

Uh oh!

alexey-milovidov commented Feb 20, 2021

Uh oh!

abyss7 commented Feb 20, 2021

Uh oh!

filimonov commented Feb 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

abyss7 commented Jan 23, 2021 •

edited by alexey-milovidov

Loading

alexey-milovidov commented Feb 19, 2021 •

edited

Loading

alexey-milovidov commented Feb 20, 2021 •

edited

Loading

filimonov commented Feb 25, 2021 •

edited

Loading