Phase 1 of Torqestration #540

jamielamont · 2022-12-05T09:50:46Z

Created new orchestrator process
Setup configurable lower and upper limits
Implemented scale up and scale down functions
Function to auto scale up upon startup to meet lower limit set in config
Ability to call orchestrator to scale up and down from gateway

…cess

…alingdetails table plus added orchestrator to list of connections in gateway settings

…yond or under the configured upper and lower limits

… freed up after proc removal

…it tests for scaleup and scaledown funcs

drgdavies · 2022-12-05T09:53:35Z

addproc.sh

@@ -0,0 +1,66 @@
+#!/bin/bash


Could addproc and remove proc be consolidated into a single bash script? Perhaps using flags for up and down? Would be nicer to have a scaleproc.sh script that handles both scenarios

Yeah this would be a lot tidier. I'll get working on this.

drgdavies · 2022-12-05T09:54:25Z

code/processes/gateway.q

 // Join active .gw.servers to .servers.SERVERS table
 activeservers:{lj[select from .gw.servers where active;`handle xcol `w xkey .servers.SERVERS]}

+/function to tell orchestrator to scale up


The code here is duplicated across both. Could you have a generic function to cover both scenarios and either use params or projections?

We were still working on adjusting the gateway code with differences between scaling up and down and this made it simpler. It can be a generic function with these changes added, I'll give it a look.

drgdavies · 2022-12-05T09:56:04Z

code/processes/orchestrator.q

+scalingdetails:([] time:`timestamp$(); procname:`$(); dir:`$(); instancecreated:`$(); instanceremoved:`$(); totalnumofinstances:`int$(); lowerlimit:`int$(); upperlimit:`int$());	/table for tracking scaling
+
+processlimitscsv:hsym first .proc.getconfigfile"processlimits.csv";	/location of csv file
+limits:1!("SII";enlist ",")0:processlimitscsv;	/table of scalable processes and the max number of instances allowed for each


What is the key of the file? 1! doesn't give an indication, I think I'd prefer use of xkey here given we don't know the file.

2 other points - is there already csv loading functionality in TorQ? Could it be leveraged? For example what happens if file doesn't exist here? OR if the file load fails? We need to handle those scenarios.

Ok cool I will have a look for this functionality

drgdavies · 2022-12-05T09:56:40Z

code/processes/orchestrator.q

+/initialises connection to discovery process and creates keyed table containing the number of instances of each scalable process
+getscaleprocsinstances:{[] 
+	.servers.startup[];
+	`.orch.procs set string@/:exec procname from .servers.procstab;


Why are these stringed?

In the next line (Line 17) there is a lambda which gets the count of instances for each scalable process. This is done by checking which of the strings in the .orch.procs list are like the scalable process. E.g. {sum procs like "rdb1*"} will recognize all instances of rdb1 i.e. rdb1.2 rdb1.3 and so on

what if there's lots of rdbs and we have an rdb10, would that be detected as a replicated instance of rdb1?

I also don't think you need the @/: here?

Yes currently it would be detected so will need fixed and yes it's not needed, I see now 'like' accepts string or symbol in left operand. Thanks

drgdavies · 2022-12-05T09:57:41Z

code/processes/orchestrator.q

+getscaleprocsinstances[];
+
+/function to scale up a process
+scaleup:{[procname]


Again scaleup and scaledown here there looks to be a lot of duplication in the code. Potentially parameterize or project.

drgdavies · 2022-12-05T09:58:02Z

code/processes/orchestrator.q

+        ];
+        }
+
+initialscaling@/:scaleprocslist;


What is this doing?

Calls the intialscaling function for each of the scalable processes to ensure they have been scaled up to their lower limits on startup

jonathonmcmurray · 2022-12-05T15:06:34Z

code/processes/orchestrator.q

+/initialises connection to discovery process and creates keyed table containing the number of instances of each scalable process
+getscaleprocsinstances:{[] 
+	.servers.startup[];
+	`.orch.procs set string@/:exec procname from .servers.procstab;


what if there's lots of rdbs and we have an rdb10, would that be detected as a replicated instance of rdb1?

I also don't think you need the @/: here?

jonathonmcmurray · 2022-12-05T15:08:48Z

code/processes/orchestrator.q

+	$[scaleprocsinstances[procname;`instances]<limits[procname;`upper];
+		[system "bash ${TORQHOME}/addproc.sh ",string procname;
+		/update number of process instances
+		getscaleprocsinstances[];
+		/update table with record for scaling up
+		`.orch.scalingdetails upsert (.z.p;procname;`up;`$(last procs where procs like string[procname],"*");`;scaleprocsinstances[procname;`instances];limits[procname;`lower];limits[procname;`upper])];
+		.lg.o[`scale;"upper limit hit for ",string procname]
+	];


Multi-line $ with blocks in them are confusing IMO. I'd reverse the condition & early exit in an if e.g.

Suggested change

$[scaleprocsinstances[procname;`instances]<limits[procname;`upper];

[system "bash ${TORQHOME}/addproc.sh ",string procname;

/update number of process instances

getscaleprocsinstances[];

/update table with record for scaling up

`.orch.scalingdetails upsert (.z.p;procname;`up;`$(last procs where procs like string[procname],"*");`;scaleprocsinstances[procname;`instances];limits[procname;`lower];limits[procname;`upper])];

.lg.o[`scale;"upper limit hit for ",string procname]

];

if[scaleprocsinstances[procname;`instances]>=limits[procname;`upper];

.lg.o[`scale;"upper limit hit for ",string procname];

:();

];

system "bash ${TORQHOME}/addproc.sh ",string procname;

/update number of process instances

getscaleprocsinstances[];

/update table with record for scaling up

`.orch.scalingdetails upsert (.z.p;procname;`up;`$(last procs where procs like string[procname],"*");`;scaleprocsinstances[procname;`instances];limits[procname;`lower];limits[procname;`upper]);

jonathonmcmurray · 2022-12-05T15:12:07Z

code/processes/gateway.q


+/function to tell orchestrator to scale up
+scaleup:{[procname]
+handle:first exec w from .servers.SERVERS where proctype=`orchestrator;


I think you could/should probably use .servers.gethandlebytype here

Suggested change

handle:first exec w from .servers.SERVERS where proctype=`orchestrator;

handle:.servers.gethandlebytype[`orchestrator;`any];

(this will mean that if for whatever reason connection to orchestrator has been lost, it will attempt to re-open it first)

jonathonmcmurray · 2022-12-05T15:14:29Z

code/processes/orchestrator.q

+/function to ensure all processes have been scaled up to meet lower limit
+initialscaling:{[procname]
+        if[scaleprocsinstances[procname;`instances]<limits[procname;`lower];
+                reqinstances:limits[procname;`lower]-scaleprocsinstances[procname;`instances]; 
+		do[reqinstances;scaleup[procname]];
+        ];
+        }


there's some indenting with spaces here, rest of file appears to be using tabs - make it consistent please

simondalzell-aquaq and others added 10 commits November 25, 2022 14:53

Included addproc.sh

ab17c9e

basic template created for new orchestrator process

266b583

Added function to obtain the number of instances of each scalable pro…

34c9652

…cess

Updated scaleup and scaledown funcs to upsert record of scaling to sc…

a5d5daf

…alingdetails table plus added orchestrator to list of connections in gateway settings

Script for scaling down individual processes

07a2005

Added checks to scaleup and scaledown funcs so instances cannot go be…

e4a642c

…yond or under the configured upper and lower limits

Adjusting order of things slightly, and add in line to ensure port is…

abb461f

… freed up after proc removal

Added function to scale processes to lower limit upon startup plus un…

d6bd8c6

…it tests for scaleup and scaledown funcs

Working test.csv ready for review

bade307

Functions added to gateway to call orchestator to scale up/down

e4baca4

jamielamont requested review from drgdavies and picoDoc December 5, 2022 09:50

jamielamont assigned jamielamont, meganamorelli and simondalzell-aquaq Dec 5, 2022

drgdavies reviewed Dec 5, 2022

View reviewed changes

jonathonmcmurray reviewed Dec 5, 2022

View reviewed changes

jamielamont and others added 5 commits December 6, 2022 14:54

refactored code based on review

512b9c5

remove comments from debugging

3978e97

Delete addproc.sh

5512451

Delete removeproc.sh

59848f4

Update process.csv

bf7ce3f

	handle:first exec w from .servers.SERVERS where proctype=`orchestrator;
	handle:.servers.gethandlebytype[`orchestrator;`any];

Phase 1 of Torqestration #540

Are you sure you want to change the base?

Phase 1 of Torqestration #540

Uh oh!

Conversation

jamielamont commented Dec 5, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamielamont Dec 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jamielamont Dec 5, 2022 •

edited

Loading