[D] Fix socket read error

[waghanza]

Hi @cyrusmsk,

I open this PR in order to solve #6609

---

Closes #6609

[cyrusmsk]

Did it solved also handy server issues?

[waghanza]

Nop, I opened this PR in order to find a solution, but I didn't find yet

[waghanza]

@cyrusmsk I managed to run also on CI and I have the same kind of results https://github.com/the-benchmarker/web-frameworks/actions/runs/5866851930/job/15906476528?pr=6623#step:10:119

This is however an ubuntu 22.04 not like my dist (fedora 38).

Not sure to understand how you can have different results using docker layer.

[waghanza]

May you have a clue @zoujiaqing

I have the issue for handy, serverino and vibe.d only. The other are OK

• https://github.com/the-benchmarker/web-frameworks/actions/runs/5866851930/job/15906475910?pr=6623#step:10:119
• https://github.com/the-benchmarker/web-frameworks/actions/runs/5866851930/job/15906476068?pr=6623#step:10:119
• https://github.com/the-benchmarker/web-frameworks/actions/runs/5866851930/job/15906476371?pr=6623#step:10:119

[cyrusmsk]

Not sure to understand how you can have different results using docker layer.

@waghanza I'm running docker on M1 Mac. This is what I got for handy: wrk -H 'Connection: keep-alive' -d 5s -c 64 --timeout 8 -t 8 http://127.0.0.1:3000 Running 5s test @ http://127.0.0.1:3000 8 threads and 64 connections Thread Stats Avg Stdev Max +/- Stdev Latency 21.98ms 67.59ms 428.76ms 93.65% Req/Sec 1.25k 411.63 1.65k 87.50% 16393 requests in 5.04s, 1.24MB read Socket errors: connect 0, read 16393, write 0, timeout 0 Requests/sec: 3254.27 Transfer/sec: 251.06KB

For serverino almost the same result: wrk -H 'Connection: keep-alive' -d 5s -c 64 --timeout 8 -t 8 http://127.0.0.1:3000 Running 5s test @ http://127.0.0.1:3000 8 threads and 64 connections Thread Stats Avg Stdev Max +/- Stdev Latency 25.50ms 77.43ms 473.15ms 93.27% Req/Sec 1.20k 463.77 1.63k 82.35% 16389 requests in 5.03s, 0.00B read Socket errors: connect 0, read 16389, write 0, timeout 0 Requests/sec: 3255.35 Transfer/sec: 0.00B

And the same results I've got even without using Docker. Just build and run server locally and run wrk in other tab of terminal.

[waghanza]

So all requests has a socket read error like me.

[cyrusmsk]

So all requests has a socket read error like me.

Yes. But author of handy used Apache JMeter and the server for his projects and he said there were no any errors or problems. So he doesn’t know why wrk thinks there is read errors. So maybe we could then just exclude those frameworks from the list.

[waghanza]

For the moment they are hidden (I don't mind doing it manually), but I'm very curious about the reason(s) behind. Let's keep them for now, at least before I automate the results publication

[waghanza]

Probably, Apache communications are made differently. For example keeping an opened socket

[waghanza]

It could be interesting though to replicate the handy author configuration (having an Apache in front of handy) inside a docker and see what is going on.

Maybe handy handle well Unix socket communication but not http based communication @cyrusmsk

[cyrusmsk]

Maybe we can ping @tchaloupka if he had any similar issues in his benchmark. He used serverino and wrk as well.

Hi Tomáš. Have you faced with any socket errors in your run with wrk?

[cyrusmsk]

It could be interesting though to replicate the handy author configuration (having an Apache in front of handy) inside a docker and see what is going on.

Maybe handy handle well Unix socket communication but not http based communication @cyrusmsk

sounds too advanced for me :) I've prepared a Dockerfile update to switch on LTO (send it soon). But I'm not sure how to deal with low-level errors of sockets.

[waghanza]

With a nginx in front of handy, I have

➜  web git:(d/avoid_root_on_docker) ✗ wrk http://127.0.0.1:3000/user/0
Running 10s test @ http://127.0.0.1:3000/user/0
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     7.41ms    3.89ms  49.13ms   78.82%
    Req/Sec   694.99    104.95     0.88k    74.50%
  13870 requests in 10.03s, 2.16MB read
Requests/sec:   1382.30
Transfer/sec:    220.03KB

Maybe handy, serverino and vibe.d use the same http lib that miss something (socket closing ?)

[waghanza]

Seems that everything goes as expect with a nginx in front

• archttpd => https://github.com/the-benchmarker/web-frameworks/actions/runs/5878592708/job/15940992886?pr=6623#step:10:119
• cgi => https://github.com/the-benchmarker/web-frameworks/actions/runs/5878592708/job/15940993106?pr=6623#step:10:119
• handy => https://github.com/the-benchmarker/web-frameworks/actions/runs/5878592708/job/15940993498?pr=6623#step:10:119
• lighttp => https://github.com/the-benchmarker/web-frameworks/actions/runs/5878592708/job/15940993716?pr=6623#step:10:86
• serverino => https://github.com/the-benchmarker/web-frameworks/actions/runs/5878592708/job/15940993935?pr=6623#step:10:119
• vibe.d => https://github.com/the-benchmarker/web-frameworks/actions/runs/5878592708/job/15940994141?pr=6623#step:10:119

Is it a common practice to have use a front reverse proxy in production @cyrusmsk @kubo39 @andrewlalis @trikko @adamdruppe ?

I mean some languages (php) has no http capability, that's why I put nginx as a front layer, but d do not have http in it stdlib ?

[waghanza]

cc @zoujiaqing

[andrewlalis]

@waghanza yes, I use nginx as a reverse proxy for handy-http so that I get HTTP 2 and other benefits, but I suspect that there's an implementation detail in wrk that's causing it to report socket read errors where there are none. I've ran tests with Apache JMeter and it does not report errors when load-testing handy-http, and various browsers have no problem interacting with it.

[adamdruppe]

wow i don't have the wrk program installed on my computer, amazing i never installed it over all these years. I tend to use ab lol

anyway, there is no standard http server or parser in D, and basically every lib author ignores what came before so each thing tends to have its own implementation. i don't think there's any shared code between cgi, handy, serverino, and vibe. I don't know about arc and light.

I do recommend people use a reverse proxy because it gives you some advantages in configuration and maybe some security, but it isn't necessary.

But I remember many years ago seeing similar read errors using my library too and it was a bug I fixed. iirc about the connection close header or something, but it was a long time ago. I wouldn't be surprised if the other implementations had bugs too.

[waghanza]

I'll be very interested @adamdruppe if you try to find ~since~ this bug in history

[trikko]

If I remember correctly, I've tried to debug this problem and it was something related to connection closing after the request was fulfilled. Nothing important, I think.

[trikko]

https://github.com/wg/wrk/blob/a211dd5a7050b1f9e8a9870b95513060e72ac4a0/src/wrk.c#L446

Probably this fails because server closed connection: https://github.com/wg/wrk/blob/a211dd5a7050b1f9e8a9870b95513060e72ac4a0/src/net.c#L17.

[waghanza]

@trikko should the server close connection with keep alive header ?

[trikko]

@trikko should the server close connection with keep alive header ?

From RFC:

«Servers will usually have some time-out value beyond which they will no longer maintain an inactive connection. Proxy servers might make this a higher value since it is likely that the client will be making more connections through the same server. The use of persistent connections places no requirements on the length (or existence) of this time-out for either the client or the server.

When a client or server wishes to time-out it SHOULD issue a graceful close on the transport connection. Clients and servers SHOULD both constantly watch for the other side of the transport close, and respond to it as appropriate. If a client or server does not detect the other side's close promptly it could cause unnecessary resource drain on the network.»

[adamdruppe]

a server closing the connection gracefully returns 0 to that read function but wrk only reports as an error if it is -1.

But yeah, i don't remember exactly what I did to change this in mine but it seems to work ok now.

[trikko]

shutdown() vs close() ?

[waghanza]

or both https://dlang.org/library/std/socket/socket.close.html

[waghanza]

as I understand, this made in https://github.com/trikko/serverino/blob/e425ffcb2090bd7fd6f9d1db7c230a5dbc7035e5/source/serverino/daemon.d#L121 but not https://github.com/trikko/serverino/blob/e425ffcb2090bd7fd6f9d1db7c230a5dbc7035e5/source/serverino/worker.d#L159

@trikko

[trikko]

Yes but I think you should wait for shutdown message to be received. But, as said, too bad if they didn't already received it: I close connection anyway. It doesn't really change anything in this practical case.

[trikko]

Sorry I deleted the last comment by mistake. Today I have access to my laptop. I'm doing some tests. I see you use -H "connection:keep-alive" but http 1.1 use keep-alive by default.

Removing that argument from command line, it works fine. No errors are reported and serverino has better performance.

I'm trying to understand if it is a server-side or a client-side problem.

[trikko]

Hmm I wonder if it is a bug in wrk.
Using -H "Connection:keep-alive" works fine. Using -H "Connection: keep-alive" gives errors.

It doesn't seem a bug in serverino.

Probably they don't check for spaces after ":"? Their header parser is a quite complex state machine :) https://github.com/wg/wrk/blob/a211dd5a7050b1f9e8a9870b95513060e72ac4a0/src/http_parser.c#L1414

[cyrusmsk]

@trikko When I tried -H "Connection:keep-alive" I've got segfaults.. Can you share whole wrk command please? I've used this one wrk -H 'Connection:keep-alive' -d 5s -c 64 --timeout 8 -t 8 http://127.0.0.1:3000

On Docker it was just some logs: https://paste.myst.rs/9um95tts

When I tried to run locally (macOS) I've got also extra rows:

★ [051763] [l] 2023-08-17 15:53 [connectionhandler.d:0315] Socket error on read. Resource temporarily unavailable core.exception.ArrayIndexError@../../../../../.dub/packages/serverino-0.4.5/serverino/source/serverino/daemon.d(437): index [119] is out of bounds for array of length 119

??:? onArrayIndexError [0x104d8e36f] ??:? _d_arraybounds_index [0x104d8e8ff] ../../../../../.dub/packages/serverino-0.4.5/serverino/source/serverino/daemon.d:437 void serverino.daemon.Daemon.wake!(app).wake(std.typecons.Typedef!(serverino.config.DaemonConfig*, null, null).Typedef) [0x104ba74fb] ../../../../../.dub/packages/serverino-0.4.5/serverino/source/serverino/main.d:175 int serverino.main.wakeServerino!(app).wakeServerino(ref serverino.config.ServerinoConfig) [0x104ba3997] ../../../../../.dub/packages/serverino-0.4.5/serverino/source/serverino/main.d:147 int app.mixin8.mixin2.mainServerinoLoop(immutable(char)[][]) [0x104ba3813] ../../../../../.dub/packages/serverino-0.4.5/serverino/source/serverino/main.d:109 _Dmain [0x104ba39db] ■ [051766] [l] 2023-08-17 15:53 [worker.d:0241] Killing worker. [REASON: socket closed?]

[trikko]

Oh, yes. That's a regression i fixed some days ago. Just update to last serverino.

On Thu, Aug 17, 2023 at 1:56 PM Serg Gini @.***> wrote:

@trikko https://github.com/trikko When I tried -H "Connection:keep-alive" I've got segfaults.. Can you share whole wrk command please? I've used this one wrk -H 'Connection:keep-alive' -d 5s -c 64 --timeout 8 -t 8 http://127.0.0.1:3000

On Docker it was just some logs: https://paste.myst.rs/9um95tts

When I tried to run locally (macOS) I've got also extra rows: ★ [051763] [l] 2023-08-17 15:53 [connectionhandler.d:0315] Socket error on read. Resource temporarily unavailable @.***/../../../../.dub/packages/serverino-0.4.5/serverino/source/serverino/daemon.d(437): index [119] is out of bounds for array of length 119

??:? onArrayIndexError [0x104d8e36f] ??:? _d_arraybounds_index [0x104d8e8ff] ../../../../../.dub/packages/serverino-0.4.5/serverino/source/serverino/daemon.d:437 void serverino.daemon.Daemon.wake!(app).wake(std.typecons.Typedef!(serverino.config.DaemonConfig*, null, null).Typedef) [0x104ba74fb] ../../../../../.dub/packages/serverino-0.4.5/serverino/source/serverino/main.d:175 int serverino.main.wakeServerino!(app).wakeServerino(ref serverino.config.ServerinoConfig) [0x104ba3997] ../../../../../.dub/packages/serverino-0.4.5/serverino/source/serverino/main.d:147 int app.mixin8.mixin2.mainServerinoLoop(immutable(char)[][]) [0x104ba3813] ../../../../../.dub/packages/serverino-0.4.5/serverino/source/serverino/main.d:109 _Dmain [0x104ba39db] ■ [051766] [l] 2023-08-17 15:53 [worker.d:0241] Killing worker. [REASON: socket closed?]

— Reply to this email directly, view it on GitHub https://github.com/the-benchmarker/web-frameworks/pull/6623#issuecomment-1682156002, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE575JFNZRY4NZR5XAGJMDXVYBHRANCNFSM6AAAAAA3P5N5KM . You are receiving this because you were mentioned.Message ID: @.***>

[waghanza]

Same for me @trikko https://github.com/the-benchmarker/web-frameworks/actions/runs/5891481803/job/15978641101?pr=6623#step:10:125

[cyrusmsk]

For me 0.4.6 also hasn't solved the issue with socket read error(

[trikko]

Also removing the whole connection header?

[cyrusmsk]

Also removing the whole connection header?

Yes. I’ve tried all with space and without space and without this -H option at all

[trikko]

On linux:

andrea /tmp/serv > wrk -H 'connection: keep-alive' -d 5s -c 512 --timeout 8 -t 4 http://127.0.0.1:3000
Running 5s test @ http://127.0.0.1:3000
  4 threads and 512 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    45.89ms  264.41ms   3.36s    97.32%
    Req/Sec     8.78k     1.25k   11.57k    66.00%
  174673 requests in 5.05s, 20.16MB read
  Socket errors: connect 0, read 174670, write 0, timeout 0
Requests/sec:  34597.58
Transfer/sec:      3.99MB
andrea /tmp/serv > wrk -H 'connection:keep-alive' -d 5s -c 512 --timeout 8 -t 4 http://127.0.0.1:3000
Running 5s test @ http://127.0.0.1:3000
  4 threads and 512 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   159.11ms  683.46ms   5.01s    94.42%
    Req/Sec    19.46k     7.52k   29.90k    39.61%
  298293 requests in 5.06s, 34.42MB read
Requests/sec:  58951.33
Transfer/sec:      6.80MB

Just to be sure, add this to your serverino config:

.setHttpTimeout(10.seconds)
.enableKeepAlive(180.seconds);

[cyrusmsk]

@waghanza I've tested different tool instead of wrk (https://github.com/codesenberg/bombardier) And it seems it doesn't have issues for serverino.

Did you test proposed changes on linux btw? Because on macOS even with 'connection:keep-alive' wrk is still showing socket read errors.

[waghanza]

Yes, both macros and Linux remain in error.

Will give another try will timeout set on the servernio side.

[waghanza]

Maybe using https://github.com/lnx-search/rewrk could be a workaround

[waghanza]

Wdyt about this issue @OvermindDL1 @Ethosa @tbrand @jeremyevans @ahopkins @krishnaTORQUE @ioquatix ?

[ioquatix]

Try my fork of wrk it fixed some bugs.

[waghanza]

Mmh, seems interesting https://github.com/wg/wrk/compare/master...ioquatix:wrk:master

[krishnaTORQUE]

@waghanza Definitely there are some bugs in header that I can see. Also from ubuntu-latest >> ubuntu-22 make sense to me.

[cyrusmsk]

Maybe using https://github.com/lnx-search/rewrk could be a workaround

Do you think it is possible to move to this?

Serverino for example is not supporting Pipelining. And this is from rewrk repo: The issue is that wrk only handle some of the HTTP spec and is entirely biased towards frameworks and servers that can make heavy use of HTTP/1 Pipelining which is no longer enabled in most modern browsers or clients, this can give a very unfair and unreasonable set of stats when comparing frameworks as those at the top are simply better at using a process which is now not used greatly.

[trikko]

rewrk sounds like another app anyway, not a fork.

[waghanza]

I think we should disable http pipelining. As I know, this feature is enabled by some browsers by default. But what you check here is like an API, not a website so http pipelining could be inaccurate here

[waghanza]

Related to https://github.com/the-benchmarker/web-frameworks/issues/5329

[trikko]

Which browsers? I don't think it is enabled by anyone. Curl completly removed pipeline support in 2019:

https://daniel.haxx.se/blog/2019/04/06/curl-says-bye-bye-to-pipelining/

[ioquatix]

FWIW, I did not observe wrk doing pipelining, but maybe I did not look close enough.

We use header in https://github.com/the-benchmarker/web-frameworks/blob/master/.tasks/collect.rake#L70

PS : I confess that the codebase is not very clean, I should clean it before considering this project stable

@waghanza did you mean to edit my comment with your reply? :)

[trikko]

If only wrk could give better error reports...