Use EventEmitter for remote client connections #11

New Issue

coolaj86 · 2018-09-16T08:24:00Z

coolaj86 commented

2018-09-16 08:24:00 +00:00

First, read the full README of proxy-packer for context: https://git.coolaj86.com/coolaj86/proxy-packer.js

All of the packerHandlers (see near https://git.coolaj86.com/coolaj86/telebit.js/src/branch/master/lib/remote.js#L267), especially onconnection, onmessage, and onerror (see https://git.coolaj86.com/coolaj86/telebit.js/src/branch/master/lib/remote.js#L343), but excluding oncontrol (which is a relay server message, not a client message), should be converted to select from a map of EventEmmiters.

We need to be using on('readable') and read() instead of on('data').

However, the pause and resume events cannot directly apply backpressure - because there is a single TCP stream and multiple clients that may need individual backpressure applied, as if they had their own TCP stream. There will need to be a buffer of a certain number of bytes (let's start with 16kb as it's about 10 * 1500 - assuming non-jumbo packets). If the data raises above the "waterline" the client should fire the pause event. When it goes some amount, say 50%, below the waterline we'll want to fire the resume event.

The actual numbers on that may need to be way different. In fact, I'd say it'll probably be best to base it on the amount of memory available and the number of clients connected - the goal being to avoid the latency introduced by pausing and resuming, when reasonable.

That area of code definitely needs a refactor, but converting to event emitters without changing anything else that isn't necessary is a good start.

This could be developed in isolation by creating a test based on https://git.coolaj86.com/coolaj86/proxy-packer.js/src/branch/master/test/parse.js but I think it's probably best just to wing it and then do a manual test by restarting telebit and uploading and downloading a few files of several megabytes (i.e. create an app to run on port 3000 and then run telebit http 3000)

First, read the full README of proxy-packer for context: https://git.coolaj86.com/coolaj86/proxy-packer.js All of the packerHandlers (see near https://git.coolaj86.com/coolaj86/telebit.js/src/branch/master/lib/remote.js#L267), especially `onconnection`, `onmessage`, and `onerror` (see https://git.coolaj86.com/coolaj86/telebit.js/src/branch/master/lib/remote.js#L343), but excluding `oncontrol` (which is a relay server message, not a client message), should be converted to select from a map of EventEmmiters. We need to be using `on('readable')` and `read()` instead of `on('data')`. However, the pause and resume events cannot directly apply backpressure - because there is a single TCP stream and multiple clients that may need individual backpressure applied, as if they had their own TCP stream. There will need to be a buffer of a certain number of bytes (let's start with 16kb as it's about 10 * 1500 - assuming non-jumbo packets). If the data raises above the "waterline" the client should fire the pause event. When it goes some amount, say 50%, below the waterline we'll want to fire the resume event. The actual numbers on that may need to be way different. In fact, I'd say it'll probably be best to base it on the amount of memory available and the number of clients connected - the goal being to avoid the latency introduced by pausing and resuming, when reasonable. That area of code definitely needs a refactor, but converting to event emitters without changing anything else that isn't necessary is a good start. This could be developed in isolation by creating a test based on https://git.coolaj86.com/coolaj86/proxy-packer.js/src/branch/master/test/parse.js but I think it's probably best just to wing it and then do a manual test by restarting telebit and uploading and downloading a few files of several megabytes (i.e. create an app to run on port 3000 and then run `telebit http 3000`)

Sign in to join this conversation.

No Label

feature

No Milestone

No Assignees

1 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: coolaj86/telebit.js#11