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

Tzachi share node2 #1352

Closed
wants to merge 12 commits into from
Closed

Conversation

tzachi-dar
Copy link
Contributor

This pr adds code to run on shared node.

Solution overview:

  1. A shared node is created to accept different requests.
  2. js modules have been changed to return their values based on a function call.
  3. "Clients" use a unix socket to talk with shared node.
    More details:
    Keeping shared node alive:
  4. Shared node is started every time that it is killed.
  5. Every minute, ping is sent to the shared node. If it does not answer it, it is killed.
  6. On my testing, I did not see it never failing.

Return value of shared node:
The result of the node consist of 3 things:

  1. The exit code of the function.
  2. The output to stdout of the js code.
  3. The output to stderr of the js code (was not still used).

Testing:
Test rig has been looping for a long time without any problem. It was able to complete pump loops within 5 minutes except where communication with pump was problematic.
All input to the shared node is captured, and copied to a shared directory.
Later, the old code and the new code have been running on this input. No difference was found between the two runs.
With this change the rigs become much less loaded (and very likely this is also a needed step in reducing battery consumption).

@Dune-jr
Copy link

Dune-jr commented Jan 26, 2020

@tzachi-dar Shouldn't this PR be for a merge on dev instead of master?

@tzachi-dar tzachi-dar changed the base branch from master to dev January 26, 2020 22:51
@tzachi-dar
Copy link
Contributor Author

@tzachi-dar Shouldn't this PR be for a merge on dev instead of master?

Sure, I have fixed this now.

@scottleibrand
Copy link
Contributor

Is there any special installation required to use this? I'm getting failures using run_remote_command ns-status. It could also be due to merge conflicts with #1346 that I resolved incorrectly. Could you merge those two branches properly and push the results somewhere I can test them?

@tzachi-dar
Copy link
Contributor Author

I have merged both branches into #1359

@scottleibrand scottleibrand mentioned this pull request Feb 14, 2020
@scottleibrand
Copy link
Contributor

Is this still needed, or is it superceded by #1370 ?

@scottleibrand
Copy link
Contributor

I'm going to close this on the assumption it's no longer needed after #1370. If that's incorrect, we can reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants