Skip to content

Conversation

@yashLadha
Copy link
Contributor

@yashLadhayashLadha commented Mar 12, 2022

This is a WIP PR for adding custom scheduler to cluster.

Will add the doc changes later after the finalisation of structure in PR.

@nodejs-github-botnodejs-github-bot added cluster Issues and PRs related to the cluster subsystem. needs-ci PRs that need a full CI run. labels Mar 12, 2022
@yashLadhayashLadhaforce-pushed the add_customer_scheduler branch from 286733a to 645b540CompareMarch 12, 2022 06:47
@yashLadhayashLadha added the needs-benchmark-ci PR that need a benchmark CI run. label Mar 12, 2022
@yashLadhayashLadhaforce-pushed the add_customer_scheduler branch 5 times, most recently from cbe6731 to 21d893dCompareMarch 13, 2022 10:20
@nodejs-github-bot

This comment was marked as duplicate.

@yashLadhayashLadhaforce-pushed the add_customer_scheduler branch from 21d893d to 581a209CompareMarch 13, 2022 10:28
@nodejs-github-bot

This comment was marked as outdated.

@yashLadhayashLadhaforce-pushed the add_customer_scheduler branch from 581a209 to 87bc466CompareMarch 13, 2022 11:39
@nodejs-github-bot

This comment was marked as outdated.

@yashLadhayashLadhaforce-pushed the add_customer_scheduler branch from 87bc466 to 9dec344CompareMarch 13, 2022 11:51
@nodejs-github-bot

This comment was marked as outdated.

@yashLadhayashLadhaforce-pushed the add_customer_scheduler branch from 9dec344 to e3980ffCompareMarch 13, 2022 12:20
@nodejs-github-bot
Copy link
Collaborator

@yashLadha
Copy link
ContributorAuthor

yashLadha commented Apr 1, 2022

Opening this for initial review. Will add the docs later after the finalisation.

@yashLadhayashLadha marked this pull request as ready for review April 1, 2022 02:02
@yashLadhayashLadhaforce-pushed the add_customer_scheduler branch from e3980ff to 40302e5CompareApril 2, 2022 11:14
functionvalidateAndReturnScheduler(scheduler,schedulingPolicy){
if(scheduler!==undefined){
if(typeofscheduler.execute!=='function'){
thrownewERR_CLUSTER_INVALID_SCHEDULER('scheduler.execute');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this warrants a new error code. ERR_INVALID_ARG_TYPE would probably still work here.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INVALID_ARG_TYPE seems very broad and that's why i made the choice of ERR_CLUSTER_INVALID_SCHEDULER. Will update the use to INVALID_ARG_TYPE.

@yashLadhayashLadhaforce-pushed the add_customer_scheduler branch from 40302e5 to 8328c98CompareApril 4, 2022 13:51
@cjihrig
Copy link
Contributor

It's nice to see this picked back up. Just a word of caution @yashLadha - I don't know how popular cluster is in userland anymore, but it is very sparsely maintained in core. When people inevitably have problems with their custom schedulers, you're going to be the main person asked to help debug it. Are you sure you want to sign up for that 😄 (that's why I originally decided not to pursue this)?

@yashLadha
Copy link
ContributorAuthor

@cjihrig Sure, reason why i wanted this being i also faced similar issue at one the projects and though if this is part of core cluster then that would be awesome.

scheduler let's use a custom scheduler function for scheduling workers.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clusterIssues and PRs related to the cluster subsystem.needs-benchmark-ciPR that need a benchmark CI run.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@yashLadha@nodejs-github-bot@cjihrig@jasnell