Using a custom http agent

[amirt-microsoft]

Hello,
on release 2.7.0, pull request 368
you added the usage of forever agent by default.
by doing it, you are using the forever agent for every http request.

Before the change, all requests were using the app global http agent, which is configurable by the app.
after the change it's using forever agent with its default values.
We are encountering socket issues when running an app on azure app service, and making many requests to storage.

Is it possible to add an option to disable the forever agent, or maybe to be able to provide a custom configuration for it?

[jiacfan]

@amirt-microsoft

Sorry for the latency, and thanks for providing suggestions!

For the pull request 368, we use forever agent by default, as it show good result in internal stress testing. At same time, I think your suggestion "customizable agent" is quite reasonable and valuable.

For the "socket issues" you mentioned, would you please share more details? And would you share the common agent settings you might use to mitigate the "socket issues" you mentioned.

Thanks,
Jiachen

[amirt-microsoft]

The issue we are experiencing while using the latest version on app service is running out of sockets.
We were able to achive excellent results by using the following package:
https://www.npmjs.com/package/agentkeepalive
more specifically, we are using the following configuration:
const agentConfig = {maxSockets: 10, maxFreeSockets: 10, timeout: 60000, freeSocketKeepAliveTimeout: 30000};
const http = require('http');
const https = require('https');
const HttpAgent = require('agentkeepalive');
const HttpsAgent = require('agentkeepalive').HttpsAgent;
http.globalAgent = new HttpAgent(agentConfig);
https.globalAgent = new HttpsAgent(agentConfig);

[jiacfan]

@amirt-microsoft

Thanks for the reply, the suggestion is valuable, I'll discuss with team internally and see if we can provide a option switch to disable forever agent, and so users can set their customized agent as their wish when forever agent is not set. And we'll also evaluate to provide a custom configuration.

Could you work around this issue in your side temporarily?We will give more details in this thread later.

Thanks,
Jiachen

[amirt-microsoft]

Thanks for looking into it!
I can't think of any workaround I can do on my side, and will be glad to hear if you have any suggestions.
At the moment, We have downgraded and using version 2.6.0
looking forward to get updates on the issue.
Thanks!

[jiacfan]

@amirt-microsoft

If version 2.6.0 is good for your scenario, please use it temporarily. And thanks for the efforts! Will update in this thread on the further operations on this item.

Thanks,
Jiachen

[tony-gutierrez]

This is definitely needed if anyone plans on using this module on Azure App Services. They have a pathetic 160 port limit, so every node library used has to be evaluated for proper keep-alive strategy.

Also I would recommend that users use native node keep-alive as opposed to agentkeepalive.

The documentdb, service bus, and notification hub sdks allow you to overwrite the agent.

[jiacfan]

@tony-gutierrez Thanks for the suggestion! Yes, the ability to do customization is valuable. This request is on track. At same time, appreciate any contribution, Thanks!

[amirt-microsoft]

hi @jiacfan it's been over a month now, are you going to add this support anytime soon?
Let me remind you that this is a blocker for us. should we look for other solutions?

[jiacfan]

Hi, @amirt-microsoft

Thanks for reminding! And sorry for keep you waiting. I think the support will be a good optimization, and will add the code change to dev branch. Is it ok for you to try with the dev branch? At same time, we'll do validation and prepare for the release. The publish could be in 1 or 2 weeks.

Thanks,
Jiachen

[jiacfan]

Hi, @amirt-microsoft and all:

Here is the propose for "supporting customizable HTTP agent", and I wish to hear everyone's feedback if it can meet your requirements:

The propose is to add a new option for StorageServiceClient(And will be inherited by BlobService FileService and etc) naming enableGlobalHttpAgent.

By default, node.js client library still uses forever agent, which enables keep-alive and has good effect in most cases. And if advanced user wish to use customized Global http agent, please init Blob Service, and do settings:
blobService.enableGlobalHttpAgent= true;
And set global HTTP agent accordingly.

This would not be a breaking change, and only optimized for advanced users who wish to use their customized global http agent.

Would this meet your requirements?

Thanks,
Jiachen

[amirt-microsoft]

Thanks @jiacfan
I would also suggest to add an option to provide a custom agent upon service initialization, but your suggestion should be enough as a first step.

[tony-gutierrez]

Definitely allow for a custom agent swap in. The cosmos sdk, and the service bus sdk allow for this. Also, I'd like to retract my previous recommendation against https://github.com/node-modules/agentkeepalive and suggest that it is used as the agent to swap in for most of these SDKs. The reason being that node's native keep-alive does not close sockets on timeout, it just sends a timeout event that the developer must catch and handle port closing. Agentkeepalive has this functionality built in.

node-modules/agentkeepalive#62

[jiacfan]

Hi, @amirt-microsoft and @tony-gutierrez

This is a great discussion to go on. @tony-gutierrez I see you mentioned about cosmos sdk, and service bus sdk, and they didn't open interface for customizing custom agent in top level when constructing client.

Personally, I wish to use the enableGlobalHttpAgent approach, and disable global http agent by default in this phase, and not exposing http agent to swap-in in top level, this helps to control the quality of bits released, and open option to customization for advanced user who knows how to configure http agent well in their environment. (in case of establish a concept, customizing http agent is encouraged all the time. )

Happy to discuss further, and welcome any suggestions!

Thanks,
Jiachen

[tony-gutierrez]

They do not have the option in the constructor, but the agent can simply be overwritten.

[jiacfan]

Hi, @tony-gutierrez

Yes, similar target wants to be achieved here(overwrite will be do with global http agent), from this aspect, is enableGlobalHttpAgent acceptable for you in the coming release? We can further optimize it if there is further suggestions on Cons of this approach.

Thanks,
Jiachen

[amirt-microsoft]

@jiacfan @tony-gutierrez
global agent is the agent node uses by default.
so using it can be helpful in some cases, but in other cases, the global agent configuration might not be suitable for azure-storage.
enabling usage of a custom agent, seems like a more generic approach, and just as simple to implement.

[jiacfan]

Hi, @amirt-microsoft and @tony-gutierrez

My concerning here is the possibility of using the custom agent parameter incorrectly.

Node.js has http.globalAgent and https.globalAgent, when advanced user changed any of it, it's ok, as it indicates the advanced user knows what that will cause.

Azure storage client library depends on request module, which allows setting only single "agent instance to use". When it's not set(i.e. using enableGlobalHttpAgent=true), it reads from global http/https according to the protocol of request URL(or proxy), and it's safe to use.

While if user set the single agent instance, e.g. not working well with https, and the protocol of url to use is https, then it would be not good. (The behavior could be undefined).

So in this phase, I prefer to enableGlobalHttpAgent approach, where customization is only for advanced user, and prevent misuse(global agent is by default disabled). And prevent the misuse of agent only support http in https scenarios.

At same time, FYI, there is new architecture typescript client library under developing, which has concept of pipeline, where user can customize HttpClient and related settings. As typescript has strong type validation, it would have compile time HttpClient and Agent validation, and can help to prevent mis-type agent injection.

@amirt-microsoft I see the Pro you mentioned, and wishing to collect the feedback further. Would it be a typical scenario to apply two agent in one App? To achieve good performance for an app, would it be commonly using a single typical setting on the same environment? If the later one is much common scenario, I would propose to get enableGlobalHttpAgent approach online first.

Welcome discussions, and appreciate great ideas!

Thanks,
Jiachen

[jiacfan]

Hi, @amirt-microsoft, @tony-gutierrez and All:

Please let me if there is any more feedback, and if it's ok, I'll start the implementation of enableGlobalHttpAgent mentioned.

Thanks,
Jiachen

[jiacfan]

Hi, @amirt-microsoft

FYI. The changed has been merged into dev branch: https://github.com/Azure/azure-storage-node/commits/dev, welcome early try.

Thanks,
Jiachen

[tony-gutierrez]

I've reverted to 2.6.0 and overridden https.globalAgent. Please update this issue when there is a release with the new "enableGlobalHttpAgent" flag.

[jiacfan]

@tony-gutierrez

Sure, the release 2.10.1 involve this change is under releasing process, will update this thread when it's available.

Best Regards,
Jiachen

[jiacfan]

@tony-gutierrez

FYI. Node.js library 2.10.1 has been released.

Welcome try.

Thanks,
Jiachen

[tony-gutierrez]

Is the new config flag documented anywhere?

[tony-gutierrez]

Is there a way to set this option in a constructor or is this correct:

this.tableService = azureStorage.createTableService(account, key);

//keepalive and port limit
this.tableService.enableGlobalHttpAgent = true;

[jiacfan]

@tony-gutierrez

Yes, Your code is correct. Please check the latest document, there are doc for every service API, e.g. for table service:

enableGlobalHttpAgent Determines whether global HTTP(s) agent is enabled; true to use Global HTTP(s) agent; otherwise, false to use http(s).Agent({keepAlive:true}).

Best Regards,
Jiachen