Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cluster: worker.send returning false, but message is sent #26937

Closed
gustavomassa opened this issue Mar 26, 2019 · 3 comments
Closed

Cluster: worker.send returning false, but message is sent #26937

gustavomassa opened this issue Mar 26, 2019 · 3 comments
Labels
cluster Issues and PRs related to the cluster subsystem. invalid Issues and PRs that are invalid.

Comments

@gustavomassa
Copy link

gustavomassa commented Mar 26, 2019

Node 10.14.0/10.15.3
Windows 10 Pro 64/CentOS 7.6.18.10

Example of code:

    public static sendMasterRequest(data: ClusterMasterRequest): void {
        if (!cluster.isMaster) throw new Error('Only Master can use sendMasterRequest!');

        try {
            const message: ClusterMessage = {
                workerId: data.worker.id,
                workerPid: data.worker.process.pid,
                workerName: data.worker['name'],
                event: data.event,
                data: data
            };
            const sent = data.worker.send(message);
            if (!sent) throw new Error(`Failed to send master request to worker ${data.worker.id}`);

        } catch (ex) {
            WBA.logError(new Error(ex));
        }
    }

data.worker is a reference of cluster.worker object. Sometimes the send function returns false, but the message is received by the worker.
Normally when sending message I use process.send, but in that case I need to send to that specific worker, so worker.send on that case, process.send return void, worker.send return boolean, why?

return proc.send(message, handle);

I don't think it is returning here, since the message is delivered correctly... so the problem must be the return of proc.send.

if (!proc.connected)
    return false;

Here is an example with the worker info:
image

@BridgeAR BridgeAR added the cluster Issues and PRs related to the cluster subsystem. label Mar 27, 2019
@bnoordhuis
Copy link
Member

false means the message had to be queued for transmission, not that it wasn't sent. It's an indicator that you're sending messages faster than they can be transmitted, i.e., that you need to back off.

process.send return void, worker.send return boolean

They both return booleans. I'm not sure where you got the idea that process.send() doesn't but it does.

I'm going to close this out because I see nothing that suggests a bug in Node.js. If you have follow-up questions, please post them to the nodejs/help repo.

@bnoordhuis bnoordhuis added the invalid Issues and PRs that are invalid. label Mar 29, 2019
@gustavomassa
Copy link
Author

gustavomassa commented Mar 29, 2019

@bnoordhuis

false means the message had to be queued for transmission, not that it wasn't sent. It's an indicator that you're sending messages faster than they can be transmitted, i.e., that you need to back off.

This should be documented, maybe boolean is not the best type to return process.send values... If the message is not queued and not sent, what is the return value?
Please could you update these docs:
https://nodejs.org/api/cluster.html#cluster_worker_send_message_sendhandle_callback
https://nodejs.org/api/process.html#process_process_send_message_sendhandle_options_callback

They both return booleans. I'm not sure where you got the idea that process.send() doesn't but it does.

The problem is the node types package @types/node, there the process.send interface returns void instead of boolean...
image
DefinitelyTyped/DefinitelyTyped#34330

@sam-github
Copy link
Contributor

https://nodejs.org/api/child_process.html#child_process_subprocess_send_message_sendhandle_options_callback describes the meaning of the boolean return.

It should be described in https://nodejs.org/api/cluster.html#cluster_worker_send_message_sendhandle_callback and
https://nodejs.org/api/process.html#process_process_send_message_sendhandle_options_callback but is not.

As for @types/node, node.js doesn't maintain that, you should report the bug in the type def to the maintainer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cluster Issues and PRs related to the cluster subsystem. invalid Issues and PRs that are invalid.
Projects
None yet
Development

No branches or pull requests

4 participants