openlmis-nginx

Clone Tools
  • last updated a few seconds ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
OLMIS-6365: Improved HTTP traffic logging

Sure, done.

Sure, done.

We should verify with the requisition service, but it doesn't mean we shouldn't add to all of them. Thanks for doing so - I'd suggest adding the same to the template service, as it's expected that ...

We should verify with the requisition service, but it doesn't mean we shouldn't add to all of them. Thanks for doing so - I'd suggest adding the same to the template service, as it's expected that all services have this switched on.

OLMIS-4531: Disabled compressing on reverse proxy because it was added on the application server

1. The ticket states that compression should be tested on initiating a requisition, that's why it is currently added only to the requisition service. 2. I think about disabling compression in nginx...

1. The ticket states that compression should be tested on initiating a requisition, that's why it is currently added only to the requisition service.
2. I think about disabling compression in nginx configuration.

Two questions: *Why was it only added to the requisition service? *It seems that for GET requests we will now be doing double compression (on the application server and on reverse proxy); any way...

Two questions:

  • Why was it only added to the requisition service?
  • It seems that for GET requests we will now be doing double compression (on the application server and on reverse proxy); any way to avoid that?
OLMIS-4531: Added compressing to HTTP POST responses
OLMIS-4531: Added compressing to HTTP POST responses
Okay, all my feedback has been addressed. Marking complete and moving ticket to Done!

Okay, all my feedback has been addressed. Marking complete and moving ticket to Done!

That fixed it!

That fixed it!

I put in a quick comment about tcp_nodelay and a link to an article.

I put in a quick comment about tcp_nodelay and a link to an article.

I was referring to the ones later in the document, but hadn't linked them. They're linked now.

I was referring to the ones later in the document, but hadn't linked them. They're linked now.

Updated. The intent of the sentence was to convey that if nginx.conf was overridden, it still needed to include openlmis.conf (or the wildcard equivalent)

Updated. The intent of the sentence was to convey that if nginx.conf was overridden, it still needed to include openlmis.conf (or the wildcard equivalent)

OLMIS-2957, fix indent for numbering

OLMIS-2957, clarify nginx.conf overriding and env variables.

Thanks, this should be because GH is super finicky about line spaces.

Thanks, this should be because GH is super finicky about line spaces.

OLMIS-2957, fix numbering

We do always want to remove this when the connection is proxied to an upstream so that TCP connections from Nginx to Services aren't inadvertently closed when a client connection to Nginx is closed...

We do always want to remove this when the connection is proxied to an upstream so that TCP connections from Nginx to Services aren't inadvertently closed when a client connection to Nginx is closed:

http://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive

I left a few detailed comments. Overall, this looks good to me, and it mostly makes sense. It appears the main work here is adding Keep-Alive support to speed up NGINX (reduce latency). I also see ...

I left a few detailed comments. Overall, this looks good to me, and it mostly makes sense. It appears the main work here is adding Keep-Alive support to speed up NGINX (reduce latency). I also see that a change was made to allow access log buffering.

I think it's worth putting a sentence into the ticket about improving nginx latency (OLMIS-2957) to explain what changes were done here.

I do not see an easy way to QA test this. I trust the screenshots and the analysis Josh has done to compare before and after to confirm that we have improved performance.

It might be worth providing an example of these environment variables so people know what this is referring to. Is "keepalive 128" one of those variables? I did not see any others, so I was not sur...

It might be worth providing an example of these environment variables so people know what this is referring to. Is "keepalive 128" one of those variables? I did not see any others, so I was not sure what this referred to.

Suggest changing "needs to be included in" to "is included in". The nginx.conf file provided already includes it. Saying "needs to be" sounds like the implementer/sysadmin needs to do something to ...

Suggest changing "needs to be included in" to "is included in". The nginx.conf file provided already includes it. Saying "needs to be" sounds like the implementer/sysadmin needs to do something to include it, but they do not need to take any action.

This "1." numbering does not work right in GitHub. It numbers both sections as 1. (not 1 and 2). I am not sure why, but perhaps moving the docker run command inline might fix it. See https://githu...

This "1." numbering does not work right in GitHub. It numbers both sections as 1. (not 1 and 2). I am not sure why, but perhaps moving the docker run command inline might fix it.

See
https://github.com/OpenLMIS/openlmis-nginx#configuration

Is this a header that we want to always remove? I see some NGINX documentation explains this line is used to "Remove the Connection header if the client sends it, it could be close to close a keep...

Is this a header that we want to always remove?

I see some NGINX documentation explains this line is used to "Remove the Connection header if the client sends it, it could be close to close a keepalive connection":
https://ma.ttias.be/enable-keepalive-connections-in-nginx-upstream-proxy-configurations/

Just wanted to confirm we always want to remove this, and that it won't break anything to do so.

OLMIS-2597, upgrade to nginx 1.12

This screen shot is showing that for the permission string endpoint, the latency introduced at Nginx's access log is about 60ms, whereas before seeing 200ms latency wasn't uncommon.

This screen shot is showing that for the permission string endpoint, the latency introduced at Nginx's access log is about 60ms, whereas before seeing 200ms latency wasn't uncommon.

This diff looks odd as the original file is now openlmis.conf and this is now the basic nginx.conf file of the image.

This diff looks odd as the original file is now openlmis.conf and this is now the basic nginx.conf file of the image.