denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
98.04k stars 5.39k forks source link

APIs to facilitate connection pools #13636

Open ignoramous opened 2 years ago

ignoramous commented 2 years ago

I am unsure if Deno.connect requires clients to implement connection pooling, because the current API surface that Deno.Conn exposes makes it tricky to implement one (though, folks indeed have impl poolers atop it using try..catch semantics).

In comparing with NodeJS net, I believe equivalent of these two APIs are needed in Deno.Conn (TCP):

  1. socket.readyState tells if conn is open or half-open.
  2. socket.destroyed tells if conn is open at all.
  3. An adjustable socket.keepalive.

(Here's how we use those APIs to determine health of a tcp-conn).

NodeJS net also exposes socket.resume and socket.pause, though those are not really required to build a pooler.

From NodeJS dgram, for connected UDP sockets, Event.close and Event.error come in handy to remove failed conns preemptively from the pool.

(Here's how we use those events to evict udp-conns from the pool).

Of course, if Deno doesn't require client-code to pool (TCP/UDP) conns at all (like for HTTP), then that's even more awesome. From this github discussion, it is confirmed that tokio doesn't pool conns, but that tower does... but I am not fluent in Rust to deduce anything from Deno's use of tower.

cc: @bartlomieju

bartlomieju commented 2 years ago

Deno.Conn doesn't implement connection pooling. I feel like it's something that should be left to the 3rd party modules.

If there are missing APIs that are required to implement pooling then I'll be happy to accept a PR that adds relevant APIs.

ignoramous commented 2 years ago

Thanks. Does Deno impl connection pools of its own for HTTP/S?

bartlomieju commented 2 years ago

Thanks. Does Deno impl connection pools of its own for HTTP/S?

For fetch we use reqwest which provides connection pooling, but for HTTP server bindings Deno.serveHttp() this is up to the user to pool connections.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

ignoramous commented 1 year ago

Hi @crowlKats

I had opened this bug is for APIs to facilitate creating connection pools for TCP/UDP conns, while the linked PR, I believe, addresses pooling for http-server?

crowlKats commented 1 year ago

@ignoramous You are correct; I'll reopen this issue