sysapps / tcp-udp-sockets

Raw sockets API
85 stars 25 forks source link

Clarifying send() #46

Closed ClaesNilsson closed 10 years ago

ClaesNilsson commented 11 years ago
  • I guess I'm also concerned about TCPSocket.send().. the definition of it says that if it exceeds an internal buffer of unknowable size it must close the socket and throw an error. How can an application use that safely if it doesn't know what value will overrun the socket and trigger the exception and a close?

Rather than the true/false semantic being used as a return value here (which requires the whole send be buffered) it would be traditional to let the send accept 0->N bytes of the N bytes being sent and have that (0->N value) be the return code. Partial sends are part and parcel of stream APIs. That way if I have 4MB to send but you've only got 1MB of buffers I don't have to magically guess that - I do a 4MB write, 1MB gets buffered - 1MB is returned and I come back later to try and write the next 3MB. (either immediately which probably returns 0 or after an ondrain event).

[Ke-Fong]:

The closing on buffer overflow is too much, and should be changed to a (transient) error instead. At previous F2F meeting, this matter has been discussed and it was agreed that the buffer should have an implementation defined size. I agree with that, except that spec should probably specify a minimum.

The goal was to make the API easier to use. Partial sends (and reads) are indeed everyday's life with stream API. Yet, this is a JavaScript API, not low-level C. The kernel is buffering the send anyway, send() rather means "queue data to socket's kernel-side write buffer" than actual "send and return me control when you've done it". So it's better to let the API do the buffering, especially when you'll have to handle it with ArrayBuffer otherwise. JavaScript is not that good for dealing with raw binary data. Also JavaScript functions are not to be blocking.

NodeJS's network API works in a somewhat similar way, and it has a proven track record.

For all these reasons, I believe letting the API do the buffering + ondrain when buffer is flushed, is the good way to proceed. The current specification is perhaps a little vague and the ondrain and can be enhanced.

[Jonas] One of the design principals for Web APIs is to make things easier for developers, at the cost of taking more complexity in the platform.

Forgetting to check the return value seems too easy to do and it will work most of the time, which will lead to hard to track down bugs.

I agree that throwing is the wrong thing to do here. I'd rather think that the platform should be required to buffer any data that is sent. Handing out-of-memory should be done the same way here as we do everywhere else in the web platform, I.e. as a quality-of-implementation issue.

[Claes] I don't have so much to add in addition to what Jonas and Ke-Fong says but as editor I am happy for tangible proposals for wording to clarify vague stuff.

ClaesNilsson commented 11 years ago

At the Toronto SysApps F2F meeting we had a small group (Josh, Dave R, Marcos and me) session on the send() method. The rationale for having a web developer friendly API and handle buffering of outgoing data in the API implementation is well understood. On the other hand this is a low level SysApps API and using it requires some skill of the developer. A concern was raised with the fact the drain event is not issued until the data has been written to the network and the output buffer is empty again. This could mean performance issues, for example when sending large files, as the application has to wait to fill the output buffer with more data until the buffer is totally empty. So an application could look something like this:

function getData() { var data;

// do stuff that will retrieve data return data; }

function pushData() { var data;

do { data = getData(); } while (data != null && socket.send(data)); }

// Each time the buffer is flushed // we continue to send data. socket.ondrain = pushData;

// Start sending data. pushData();

Note that both node.js write() (http://nodejs.org/api/net.html#net_socket_write_data_encoding_callback) and Mozilla Web API send() (https://developer.mozilla.org/en-US/docs/WebAPI/TCP_Socket) are based on this design paradigm.

I would like to hear your view on this issue based on experiences with sockets, node.js,WebAPI etc. We may also approach node.js and ask on the implementation of write() in relation to http://linux.die.net/man/2/send.

ClaesNilsson commented 11 years ago

Dave Raggett (copied from https://github.com/sysapps/raw-sockets/issues/22, which has been closed):

Who are the target users for the raw socket API? My expectation is that people wanting to use raw sockets will be system programmers familiar with the principles of UDP and TCP/IP, and the POSIX sockets interface. I anticipate the development of libraries using raw sockets to implement specific protocols and for specific use cases. A higher level sockets API could make it harder for library developers to do a good job.

One example is whether we should expose EWOULDBLOCK which allows you to determine when a call to send will need to block. The Node.js socket.write method doesn't appear to expose this feature, and instead supports a call back when a synchronous send completes. It would be good to contact the developers of the Node net module to get their perspective and their rationale for their API with respect to POSIX sockets.

Some specific cases to look at include sending very large amounts of data, and trying to send at a constant rate, where the app needs to adjust this rate depending upon a means to estimate the effective bandwidth of a connection. We should consider providing example code for these cases.

ClaesNilsson commented 11 years ago

Thinking more about this issue I am not sure that there is a significant performance problem with the current design. Event though an application has to wait for the drain event before it can fill the send buffer with more data the time it takes to do this is probably not significant compared to the time it takes to transmit the contents of the send buffer on the network.

A more complicated design could provide the possible to continously fill the buffer with data while transmission is in progress but will that give a significant improvement in performance? (Well, the current design provides the possible to continously fill the buffer with data while transmission is in progress but if transmission is slow and the buffer gets filled up the application has to wait for the buffer to be completey emptied)

However, once again, it would be interesting to get feedback from the node.js network team.

mcmanus commented 11 years ago

Wait - is the ondrain event fired when any amount of data has been moved to kernel space, or when all the buffered data has been moved to kernel space?

at first read, I think I believed the latter and https://github.com/sysapps/raw-sockets/issues/46#issuecomment-23511425 seems to also suggest that.. but on rereading I think maybe it is the former (when any data has been sent and therefore a send() of size 1 can complete)

I'm thinking of : "In the process of sending TCP data, upon a detection that previously-buffered data has been written to the network and it is possible to buffer more data received from the application, the user agent MUST:

queue a task to fire an event named drain at the TCPSocket object. "

The way it is written above (i.e. to notify when any send would succeed even if there is still buffered data) is better imo.. though the naming of "drain" is probably misleading at that point and probably why I misread it).

requiring a full draining of the buffers before notification effectively requires large buffering to smooth things over. That's a problem in a bunch of cases: 1] kernel buffers can be a scarce resource, 2] large amounts of buffering breaks any prioritization, and 3] rate discovery via observation becomes very hard.

They are all problems. Of those - I think 2 is really the most interesting.. this comes up again and again in media protocols and transport protocols such as spdy and websockets mux.

perhaps a compromise is to allow an attribute to specify the watermark for onDrain? But honestly most BSD style notification APIs (select, poll, kevents) just tell you when it won't block (i.e. 1 byte would succeed at the kernel level) and that's been sufficient.

also, an option to control the amount of buffering (even if it is just a normal vs low-latency boolean attribute) would probably be a good addition. There's a reason the BSD API has all this stuff :)

ClaesNilsson commented 11 years ago

For node.js net module the documentation of the write () method (http://nodejs.org/api/net.html#net_socket_write_data_encoding_callback) states: "Returns true if the entire data was flushed successfully to the kernel buffer. Returns false if all or part of the data was queued in user memory. 'drain' will be emitted when the buffer is again free." For the drain event (http://nodejs.org/api/net.html#net_event_drain) it is stated: "Emitted when the write buffer becomes empty. Can be used to throttle uploads."

So in node ity seems that the drain event is emitted when all the buffered data has been moved to kernel space.

The documentation of the Mozilla socket API (http://mxr.mozilla.org/mozilla-central/source/dom/network/interfaces/nsIDOMTCPSocket.idl#29) states: "ondrain will be called once the previously-buffered data has been written to the network, at which point the client can resume calling send again."

This is a bit vague- So Patrick and Jonas, can we get more informtion on the Mozilla API and the experiences from the implementation?

Coming back to your proposals Patrick on an attribute to specify the watermark for onDrain. Do you have a suggestion for a default value for this watermark? And if we want to specify the amount of buffering how would that look like? Default value?

anotherlin commented 11 years ago

With buffering, the idea is to make simpler for the end developer and performance by keeping the socket's kernel side write buffer filled.

send() returns true if the data can be copied entirely inside the socket's kernel side write buffer (write(numberBytes) == numberBytes). Otherwise, it returns false, and our API is in charge of buffering. bufferedAmount attribute is increased by the number of bytes left to flush. Essentially the boolean returned by send() is just a hint telling if we're buffering user side. Make the user side buffer minimum size (in particular, specify a minimal requirement in spec ?) big enough so it is very difficult for a "normal" application (one not transmitting huge amount of data) to overflow it. This is to make the end developer's work easier, "simpler" (just sending small commands and replies) applications will then just consider overflow as an error condition and quit.

The ondrain event is to be fired when the user side buffer is flushed to kernel side but it should be "worded" carefully in the spec so as to allow implementation some liberty. Let's say we've got a 200k (typical for linux) kernel side write buffer, if we flush the user side buffer and we end-up with 199k full kernel side buffer. There's only 1k left before we're buffering again. "ondrain" should be fired somewhat later, let's say at 100k. So as to allow filling of the kernel side write buffer. But still, that can be a difficult question, as fired events are put in event queue and that queue can be filled with other events. There can be already some elapsed time before the callback from "ondrain" is called and new send() issued.

anotherlin commented 11 years ago

The "ondrain" event is only of use for bandwidth hungry (transferring huge amount of data) application. If you were to write an SMTP client, exchanging commands won't need to use "ondrain" feature, this is only needed when sending the content of the email (especially if it contains lot of attachments). An IRC client would probably never have a use for it.

ClaesNilsson commented 11 years ago

I think that it is time for updating the specification for send() and drain event. My suggestion is:

WDYT?

Could Patrick or Jonas provide any feedback based on experiences in using the Mozilla socket API that is based on the same design paradigm?

anotherlin commented 11 years ago

About the drain I would suggest a mix of the 3 last options:

'drain' will be emitted when the kernel buffer is partly free, watermark is in the [0, kernel buffer size] interval.

The default watermark is implementation dependent. This is to allow implementers to optimize for their target devices. Or on the contrary to make implementation simpler : put the watermark at kernel buffer size. The spec can provide a suggested value or formula (for example, fire 'drain' when 3/4 of kernel size is free).

The user may change the watermark, however the implementation may choose not to honor the user demand (can make thing simpler for implementers and some implementation may want to restrict that). Note that if user can change set watermark, and set it to zero, it amounts to the first option. And for TCP that is interesting as it means that all data has been received and acknowledged by peer.

ClaesNilsson commented 11 years ago

Ok, thanks Ke-Fong. So I will do the following changes:

OK?

ClaesNilsson commented 10 years ago

This is not applicable any more as the API has been rewritten to be based on Streams.