Adjust the read and write timeout configuration of the http server #2171

nic-6443 · 2021-10-12T14:27:57Z

Why submit this pull request?

Bugfix
New feature provided
Improve performance
Backport patches

What changes will this PR take into?

Adjust the read and write timeout configuration of the http server so that the Dashboard can work normally in a slow network environment. Especially write timeout, because the Dashboard has a relatively large front-end static resource file (>2M). When the bandwidth is limited It is easy to trigger the timeout in the scene, which will cause the page display to fail.

Related issues

fix/resolve #2165

Checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

… that the dashboard can work normally in a slow network environment.

codecov-commenter · 2021-10-13T05:22:35Z

Codecov Report

Merging #2171 (5b1e4c0) into master (a8298f4) will decrease coverage by 3.42%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master    #2171      +/-   ##
==========================================
- Coverage   69.34%   65.92%   -3.43%     
==========================================
  Files         188      188              
  Lines        7161     7161              
  Branches      823      823              
==========================================
- Hits         4966     4721     -245     
- Misses       1906     2153     +247     
+ Partials      289      287       -2

Flag	Coverage Δ
backend-e2e-test	`46.32% <50.00%> (-0.56%)`	⬇️
backend-e2e-test-ginkgo	`?`
backend-unit-test	`52.28% <ø> (-0.04%)`	⬇️
frontend-e2e-test	`68.21% <ø> (+0.05%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
api/internal/core/server/http.go	`56.52% <50.00%> (ø)`
api/internal/core/migrate/conflict.go	`0.00% <0.00%> (-80.00%)`	⬇️
api/internal/core/migrate/migrate.go	`0.00% <0.00%> (-76.20%)`	⬇️
api/internal/core/migrate/dataset.go	`0.00% <0.00%> (-53.04%)`	⬇️
api/internal/handler/migrate/migrate.go	`8.06% <0.00%> (-51.62%)`	⬇️
api/internal/utils/closer.go	`33.33% <0.00%> (-33.34%)`	⬇️
api/internal/handler/healthz/healthz.go	`66.66% <0.00%> (-33.34%)`	⬇️
api/internal/filter/schema.go	`32.80% <0.00%> (-23.21%)`	⬇️
api/internal/core/entity/entity.go	`68.18% <0.00%> (-22.73%)`	⬇️
api/internal/core/server/server.go	`39.34% <0.00%> (-16.40%)`	⬇️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8298f4...5b1e4c0. Read the comment docs.

bzp2010 · 2021-10-13T05:33:54Z

cc @nic-chen

nic-chen · 2021-10-13T06:58:26Z

hi, @nic-6443 which file is so large?

nic-6443 · 2021-10-14T01:11:40Z

hi, @nic-6443 which file is so large?

The front-end static files from the /usr/local/apisix-dashboard/html/ directory. The largest file (p__Route__Create.8a1b69b7.async.js) has 2.1M. Under the current 5s timeout setting, this The transmission speed of the network is required to reach 430K/s, otherwise the file will fail to download.

When users deploy Dashboard on a public cloud, this problem will be easily triggered.

zaunist · 2021-10-14T02:10:58Z

60 seconds seems a bit too long, can we compress the size of the JS file

nic-chen · 2021-10-14T02:41:02Z

hi, @nic-6443 which file is so large?

The front-end static files from the /usr/local/apisix-dashboard/html/ directory. The largest file (p__Route__Create.8a1b69b7.async.js) has 2.1M. Under the current 5s timeout setting, this The transmission speed of the network is required to reach 430K/s, otherwise the file will fail to download.

When users deploy Dashboard on a public cloud, this problem will be easily triggered.

hi, @bzp2010
Could we optimize the size of these files?
I remember there was a saying before, that a single file should be controlled within 100k to ensure user experience.
The internet speed is relatively high now, but a single file should not be that large.

nic-6443 · 2021-10-14T03:08:31Z

60s refers to the default value of send_timeout of nginx (although send_timeout is not for the transmission of the whole response).
I understand that WriteTimeout is used to prevent slow client attacks (Slowloris Attack), and Dashboard is usually not exposed to the public network , So I think there is no risk to increase WriteTimeout. Looking forward to your discussion.

CGerAJ · 2021-10-14T03:49:22Z

Should we add custom configuration timeouts? Allow low bandwidth users access, public network configuration can be limited to IP.

nic-chen · 2021-10-14T05:54:55Z

Should we add custom configuration timeouts? Allow low bandwidth users access, public network configuration can be limited to IP.

@nic-6443 we could support configure them in conf/conf.yaml

nic-6443 · 2021-10-14T05:59:51Z

Should we add custom configuration timeouts? Allow low bandwidth users access, public network configuration can be limited to IP.

@nic-6443 we could support configure them in conf/conf.yaml

I think it's ok, but I still insist the default value can be larger so that users will not easily encounter problems when using it for the first time, just like me.Especially when there is no obvious risk of increasing timeout.

zaunist · 2021-10-14T06:20:50Z

Should we add custom configuration timeouts? Allow low bandwidth users access, public network configuration can be limited to IP.

@nic-6443 we could support configure them in conf/conf.yaml

I think it's ok, but I still insist the default value can be larger so that users will not easily encounter problems when using it for the first time, just like me.Especially when there is no obvious risk of increasing timeout.

My advice is to consider how to compress js files first, and if there is no way to compress them, then discuss increasing the default timeout

nic-6443 · 2021-10-14T06:26:30Z

Should we add custom configuration timeouts? Allow low bandwidth users access, public network configuration can be limited to IP.

@nic-6443 we could support configure them in conf/conf.yaml

I think it's ok, but I still insist the default value can be larger so that users will not easily encounter problems when using it for the first time, just like me.Especially when there is no obvious risk of increasing timeout.

My advice is to consider how to compress js files first, and if there is no way to compress them, then discuss increasing the default timeout

ok, no problem

bzp2010 · 2021-10-15T09:41:36Z

I did some statistics. When you configure gzip for the server, the total file size will drop to less than 3MB. We can try it.

starsz · 2021-10-19T01:59:14Z

I did some statistics. When you configure gzip for the server, the total file size will drop to less than 3MB. We can try it.

That sounds good !

nic-chen · 2021-10-19T06:52:35Z

api/internal/core/server/http.go

-		ReadTimeout:  time.Duration(1000) * time.Millisecond,
-		WriteTimeout: time.Duration(5000) * time.Millisecond,
+		ReadTimeout:  time.Duration(5000) * time.Millisecond,
+		WriteTimeout: time.Duration(60000) * time.Millisecond,


I think we don't need to change the WriteTimeout here?

I think it should be changed to add gzip middleware. It will greatly reduce the volume of transferred files.

I think it should be changed to add gzip middleware. It will greatly reduce the volume of transferred files.

+1. And I'm happy to update the code.

bzp2010 · 2021-10-20T01:21:59Z

@nic-6443 Would you like to try using gin gzip middleware to solve the problem? If you don't have time to do it, I'll try to fix it.

nic-6443 · 2021-10-20T01:23:29Z

@nic-6443 Would you like to try using gin gzip middleware to solve the problem? If you don't have time to do it, I'll try to fix it.

I can do it, do I need a new pull request?

Adjust the read and write timeout configuration of the http server so…

5b1e4c0

… that the dashboard can work normally in a slow network environment.

starsz approved these changes Oct 19, 2021

View reviewed changes

nic-chen reviewed Oct 19, 2021

View reviewed changes

nic-6443 closed this Oct 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adjust the read and write timeout configuration of the http server #2171

Adjust the read and write timeout configuration of the http server #2171

nic-6443 commented Oct 12, 2021

codecov-commenter commented Oct 13, 2021 •

edited

Loading

bzp2010 commented Oct 13, 2021

nic-chen commented Oct 13, 2021

nic-6443 commented Oct 14, 2021

zaunist commented Oct 14, 2021

nic-chen commented Oct 14, 2021

nic-6443 commented Oct 14, 2021

CGerAJ commented Oct 14, 2021

nic-chen commented Oct 14, 2021

nic-6443 commented Oct 14, 2021

zaunist commented Oct 14, 2021

nic-6443 commented Oct 14, 2021

bzp2010 commented Oct 15, 2021 •

edited

Loading

starsz commented Oct 19, 2021

I did some statistics. When you configure gzip for the server, the total file size will drop to less than 3MB. We can try it.

nic-chen Oct 19, 2021

bzp2010 Oct 19, 2021

zaunist Oct 19, 2021

bzp2010 commented Oct 20, 2021

nic-6443 commented Oct 20, 2021 •

edited

Loading

Adjust the read and write timeout configuration of the http server #2171

Adjust the read and write timeout configuration of the http server #2171

Conversation

nic-6443 commented Oct 12, 2021

codecov-commenter commented Oct 13, 2021 • edited Loading

Codecov Report

bzp2010 commented Oct 13, 2021

nic-chen commented Oct 13, 2021

nic-6443 commented Oct 14, 2021

zaunist commented Oct 14, 2021

nic-chen commented Oct 14, 2021

nic-6443 commented Oct 14, 2021

CGerAJ commented Oct 14, 2021

nic-chen commented Oct 14, 2021

nic-6443 commented Oct 14, 2021

zaunist commented Oct 14, 2021

nic-6443 commented Oct 14, 2021

bzp2010 commented Oct 15, 2021 • edited Loading

I did some statistics. When you configure gzip for the server, the total file size will drop to less than 3MB. We can try it.

starsz commented Oct 19, 2021

I did some statistics. When you configure gzip for the server, the total file size will drop to less than 3MB. We can try it.

nic-chen Oct 19, 2021

Choose a reason for hiding this comment

bzp2010 Oct 19, 2021

Choose a reason for hiding this comment

zaunist Oct 19, 2021

Choose a reason for hiding this comment

bzp2010 commented Oct 20, 2021

nic-6443 commented Oct 20, 2021 • edited Loading

codecov-commenter commented Oct 13, 2021 •

edited

Loading

bzp2010 commented Oct 15, 2021 •

edited

Loading

nic-6443 commented Oct 20, 2021 •

edited

Loading