emit "error" or a different EventEmitter event when the max buffer size is exceeded by send

[benderTheCrime]

Hi All,

I'm using zeromq.js@4.6.0, "push/pull," and I'm using an IPC connection to a spawned NodeJS sub-process. Additionally, I've manually set the size of the underlying buffer using socket.setsockopt which is difficult to validate (without getsockopt) given this issue:

I'm not sure if it is expected that the socket emit "error" when the IPC buffer is saturated, but currently, it does not appear to be emitting when the buffer is overflown via the socket.send method (with async flag). This results in OOMs at very high volumes.

Currently, it does not look like the Nan error handler inside the bindings.cc throws unless there is an actual issue in the process of sending, specifically with batched messages.

An example of this can be seen here:

A "producer.js" file:

const cp = require('child_process');

const zmq = require('zeromq');

const child = cp.fork(CHILD_PROCESS_MODULE_PATH); // worker.js
const socket = zmq.socket('push');
const str = 'a'.repeat(1E3);
let async = true;

socket.setsockopt(zmq.ZMQ_SNDBUF, 500); // Arbitrarily low, for the spirit of the test
socket.bindSync(`ipc:///tmp/logger.${child.pid}.${version}.sock`);
socket.on('error', function() { // This will never get called
    async = false;
    setImmediate(() => async = true); // This can be more robust, but it's demonstratory
});

setInterval(function() {
    let i = 0;

    while (i < 1E4) { // this should OOM regardless of buffer size
        if (async) {
            socket.send(str, 1);
        } else {
            console.log(str);
        }
    }
}, 10);

A "worker.js" file:

const zmq = require('zeromq');

const socket = zmq.socket('pull');
const socketAddress = `ipc:///tmp/logger.${process.pid}.${version}.sock`;

socket.connect(socketAddress);
socket.on('message', console.log.bind(console));

It's threadbare, but that should do it!

Alternatively, if it is not expected behavior that the socket emit when the buffer is overflown, it would be nice if there was an explicit event emitted when this happens so that I can do something to compensate for exceptionally high volumes.

Thanks.

[rolftimmermans]

This seems to be a shortcoming of the design of the internal queueing mechanism of the JavaScript code, which exists on top of zmq's internal queueing. It seems related to #152.

I am currently investigating what it would take to rewrite the internals so this library behaves more similarly to the zmq native bindings, especially with regards to queueing, timeouts, and the high water mark settings. Do you have any sort of example code that demonstrates the issue you experience? That would be tremendously helpful.

[benderTheCrime]

Thanks for getting back to me!

Sure, will do. I'll add a basic example of what I'm trying to accomplish in the original issue.

I'm curious to see where you would add a check for this in the bindings. From digging through it looks to be non-trivial, although I could be mistaken (I have very little experience working with this module).

[rolftimmermans]

From digging through it looks to be non-trivial

I agree. The JS queueing mechanism needs to be removed.

[benderTheCrime]

@rolftimmermans I'd like to get a sense of what the damage would be here and what I can do to help. I neither have a whole lot of time to contribute right now, nor would I know what the best approach was to remove the batching mechanism without infringing on the principles of zeromq (if indeed the batching was used to prevent copies of messages as well).

Do you have any idea how large a scope this change would be and if this could be something that I could expect to be fixed by a contributor in the short term?

[rolftimmermans]

@benderTheCrime I am currently working on a proof of concept. Unfortunately it's a massive change because to truly solve this the API needs to be different.

Example: the receive HWM should to overflow if an application cannot handle all messages fast enough.

However, the EventEmitter pattern causes messages to be read even if the application is not able to deal with the message at that time. The currently API offers no option to not read a message.

This is just an example, there are other problems too...

Edit: I expect to publish more details this week, it would be helpful to get your feedback on it.

[benderTheCrime]

Thanks again for getting back to me!

It's unfortunate to see that it's such a large change, but I'm eager to see what the POC looks like!

[rolftimmermans]

On Linux/macOS you can try zeromq-ng which addresses this issue. It currently requires Node.js 8.6+, but should be possible to backport in the future once N-API stabilises.

[rolftimmermans]

The latest 6.0 beta is based on zeromq-ng and fully supports features like this one. One way this has been addressed is to change the API so that sending/receiving returns a promise which will be resolve only after a message was successfully queued on the ZMQ socket. Settings such as timeouts and high water marks may cause queueing to fail and the promise to be rejected. With the new API all features of ZMQ should work as expected in Node.js. See #189 for more of the reasoning behind the new API.

If you run into any problems feel free to report it here or in a new issue.