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

Finer object composition for the controller class #84

Closed
wlandau opened this issue May 29, 2023 · 7 comments
Closed

Finer object composition for the controller class #84

wlandau opened this issue May 29, 2023 · 7 comments

Comments

@wlandau
Copy link
Owner

wlandau commented May 29, 2023

In crew, every R6 controller object contains 2 smaller R6 objects:

  1. A router to keep track of mirai::daemons() and the mirai dispatcher, and
  2. A launcher that knows how to create, monitor, and terminate mirai servers on the user's platform of choice. A launcher plugin is an R6 subclass that inherits from the abstract launcher class.

That leaves a lot for the controller to do: namely,

A. Manage mirai task objects (e.g. controller$collect()), and
B. Auto-scale mirai servers (`controller$scale()).

Both A and B are highly nontrivial, and if @shikokuchuo's promising idea from shikokuchuo/mirai#60 (comment) comes to fruition, then each will run on different processes. So from crew's side, it makes sense to create R6 classes for each: a "schedule" for A and a "regulator" for B. The regulator will contain the launcher, and it will be shipped to the dispatcher process in shikokuchuo/mirai#60 (comment) to do auto-scaling given a very simple set of inputs.

Currently, the object composition structure looks like this:

controller
├── launcher
└── router

My goal for the structure is:

controller
├── schedule
├── regulator
│   └── launcher
└── router
@wlandau
Copy link
Owner Author

wlandau commented Jun 12, 2023

One nice simplification would be to have the launcher call mirai::saisei(). If auto-scaling becomes possible on the mirai dispatcher, a dispatcher-side version of saisei() may be needed, as the current saisei() seems to be designed for the client only.

@wlandau
Copy link
Owner Author

wlandau commented Jun 12, 2023

Launcher-side saisei() and poll() work great and simplify things. In fact, I don't think crew even needs a "regulator" class. The launcher itself can auto-scale if I calculate demand differently.

Also, I am thinking of a couple more changes (unfortunately breaking ones for launcher plugins, but crew is still new enough for that):

  1. Rename "router" to "client".
  2. Condense the launcher, worker, and instance arguments of launch_worker() down to a single name argument with the worker name already fully formed.

@wlandau
Copy link
Owner Author

wlandau commented Jun 12, 2023

Also should bring back seconds_interval to launchers to prepare for integration with the mirai dispatcher.

@wlandau
Copy link
Owner Author

wlandau commented Jun 12, 2023

In branch 84, I simplified the internals a whole lot. Whether or not crew will be able to integrate with the mirai dispatcher (which will require a little more work on my end, but nothing painful at this point), I am happy with all the cleanup in this heavy refactor.

Now I need to debug and test:

  • Rd examples
  • README examples
  • Vignette examples
  • Launcher tests
  • interactive tests
  • Unit tests
  • throughput tests
  • crew.cluster integration
  • targets integration

@wlandau
Copy link
Owner Author

wlandau commented Jun 13, 2023

Condense the launcher, worker, and instance arguments of launch_worker() down to a single name argument with the worker name already fully formed.

No, this didn't work. crew.cluster relies on having access to each of these pieces separately. Users can construct job names on their own.

@wlandau
Copy link
Owner Author

wlandau commented Jun 13, 2023

Best of both worlds: supply both the job name and the pieces.

@wlandau wlandau mentioned this issue Jun 13, 2023
2 tasks
@wlandau
Copy link
Owner Author

wlandau commented Jun 15, 2023

#87 caused #88 and #89, but it at least fixed #84.

@wlandau wlandau closed this as completed Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant