[New Scheduler]Implement PFCInvokerServer by ningyougang · Pull Request #5098 · apache/openwhisk

ningyougang · 2021-03-30T09:36:57Z

Description

Manage New scheduler's invokerServer. provide basic disable/enable for the invoker.

These http endpoints are for deployment or get the invoker's current situation, nomally, when do zerodowntime deployment,
need to send /disable request to invoker, make the invoker's status to down, then, the new activations will not be sent to the invoker, we can wait the invoker executed all flying activations, then, we can stop the invoker safely and deploy the invoker .

/disable
e.g. curl -X POST -u xxx:xxx http://$invoker:12001/disable, disabled the invoker, the invoker status will be changed to unhealthy firstly and changed todown finally after several seconds.
/enable
e.g. curl -X POST -u xxx:xxx http://$invoker:12001/enable, enable the invoker, the invoker status will be changed to up again.
/memory (this is for FPCInvokerServer only)
e.g. curl -X GET -u xxx:xxx http://$invoker:12001/memory, if the result >0, the invoker is busy, otherwise, it is idle.

Due to simple, no document.

Related issue and scope

I opened an issue to propose and discuss this change (#????)

My changes affect the following components

Types of changes

Bug fix (generally a non-breaking change which closes an issue).
Enhancement or new feature (adds new functionality).
Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

I signed an Apache CLA.
I reviewed the style guides and followed the recommendations (Travis CI will check :).
I added tests to cover my changes.
My changes require further changes to the documentation.
I updated the documentation where necessary.

codecov-io · 2021-03-30T10:21:01Z

Codecov Report

Merging #5098 (86f5531) into master (cd6fded) will decrease coverage by 6.63%.
The diff coverage is 79.16%.

@@            Coverage Diff             @@
##           master    #5098      +/-   ##
==========================================
- Coverage   81.25%   74.61%   -6.64%     
==========================================
  Files         218      220       +2     
  Lines       10700    10723      +23     
  Branches      450      456       +6     
==========================================
- Hits         8694     8001     -693     
- Misses       2006     2722     +716

Impacted Files	Coverage Δ
...la/org/apache/openwhisk/core/invoker/Invoker.scala	`69.35% <ø> (-0.49%)`	⬇️
...pache/openwhisk/core/invoker/InvokerReactive.scala	`58.11% <0.00%> (-1.89%)`	⬇️
...ache/openwhisk/core/invoker/FPCInvokerServer.scala	`72.72% <72.72%> (ø)`
.../openwhisk/core/invoker/DefaultInvokerServer.scala	`100.00% <100.00%> (ø)`
...core/database/cosmosdb/RxObservableImplicits.scala	`0.00% <0.00%> (-100.00%)`	⬇️
...ore/database/cosmosdb/cache/CacheInvalidator.scala	`0.00% <0.00%> (-100.00%)`	⬇️
...e/database/cosmosdb/cache/ChangeFeedConsumer.scala	`0.00% <0.00%> (-100.00%)`	⬇️
...core/database/cosmosdb/CosmosDBArtifactStore.scala	`0.00% <0.00%> (-95.85%)`	⬇️
...sk/core/database/cosmosdb/CosmosDBViewMapper.scala	`0.00% <0.00%> (-93.90%)`	⬇️
...tabase/cosmosdb/cache/CacheInvalidatorConfig.scala	`0.00% <0.00%> (-92.31%)`	⬇️
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd6fded...86f5531. Read the comment docs.

ningyougang · 2021-03-31T01:59:14Z

    invoker: InvokerCore)(implicit ec: ExecutionContext, actorSystem: ActorSystem, logger: Logging): BasicRasService
 }
-
-object DefaultInvokerServer extends InvokerServerProvider {


I moved DefaultInvokerServer to a separate file, keep the same as FPCInvokerServer.
This is mainly to facilitate management

ningyougang · 2021-03-31T02:01:48Z

+            "health",
+            PingMessage(
+              InvokerInstanceId(
+                -1 - instance.instance,


Use the same topic name health to send unhealthy message via negative number of invoker.
In controller side(e.g. InvokerSupervision), get the message from kafka and make the invoker to unhealthy and down finally.

ningyougang · 2021-03-31T02:10:30Z

+          invoker.enable()
+        } ~ (path("disable") & post) {
+          invoker.disable()
+        } ~ (path("memory") & get) {


Regarding /memory, this is for FPCInvokerServer only, this is used for judge whether the invoker is idle, if the result(busyMemory + inProgressMemory) > 0, it shows the invoker is executing activations, so it is busy, if the result == 0, it shows the invoker is idle.

For DefaultInvokerServer, can't use /memory to judge whether the invoker is idle, this is another topic for this. btw, we judge it like this, get every controller slot of the invoker, and do sum. if the result > 0, the invoker is busy, otherwise, it is idle.

ningyougang · 2021-04-14T05:19:26Z

Have any comment ?

style95 · 2021-04-14T05:31:00Z

  val healthActionNamePrefix = "invokerHealthTestAction"
  val bufferSize = 10
  val bufferErrorTolerance = 3
+  var useMemory = 0l


Why do we need this mutable variable?

There has a get used memory API: http://$invoker:$port/memory,

complete(InvokerHealthManager.useMemory.toString)

The FunctionPullingContainerPool actor(which will be contributed) sends the pool's used memory info(busyMemory + inprogressMemory) to InvokerHealthyManager actor, so use a mutable variable to store the value.

I know that's there only one instance of the invoker health manager per instance / jvm, but it still seems risky to be mutating a var in an object like this especially when it's the source of truth on whether the invoker is idle or not.

@style95 @bdoyle0182 , yes, not a good idea of use mutable variable
It is better to do like below

functionPullingContainerPool.ask(UsedMemory)(Timeout(5.seconds)).mapTo[Int]

But functionPullingContainerPool is implementing now, so i removed /memory api in this pr: 86f5531
and i will add /memory api in [New Scheduler] FunctionPullingContainerPool pr.

bdoyle0182 · 2021-04-14T22:39:37Z

+
+object DefaultInvokerServer extends InvokerServerProvider {
+
+  // TODO: TBD, after FPCInvokerReactive is ready, can read the credentials from pureconfig


Not a problem for this mr, but I don't think that's a secure way of setting admin credentials

Yes, after FPCInvokerReactive is ready, can read the credentials from pureconfig like other components.

bdoyle0182 · 2021-04-14T22:42:46Z

  val healthActionNamePrefix = "invokerHealthTestAction"
  val bufferSize = 10
  val bufferErrorTolerance = 3
+  var useMemory = 0l


nit: usedMemory

Ok, good catch, as i said above, i removed /memory api in this pr. i will add /memory api in another pr: [New Scheduler] FunctionPullingContainerPool with functionPullingContainerPool.ask(UsedMemory)(Timeout(5.seconds)).mapTo[Int] solution.

bdoyle0182 · 2021-04-14T22:46:47Z

LGTM other than the discussion on the mutable variable

ningyougang · 2021-04-19T00:59:44Z

Have any more comment?

style95 · 2021-04-19T01:30:29Z

    }
  })

+  override def enable(): Route = ???


I haven't tried it but would it be ok to leave these methods empty to run it?

Sorry, you mean implement it like below?

override def enable(): Route = { complete("todo or pong") }

Due to InvokerReactive extends InvokerCore which has enabled/disabled,
so need to implement enable/disabled

when deploy using InvokerReactive and send enable/disable requests to invoker, what will happen?

codecov-commenter · 2021-04-21T02:59:38Z

Codecov Report

Attention: Patch coverage is 79.16667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 75.16%. Comparing base (cd6fded) to head (10b2471).
Report is 212 commits behind head on master.

Files with missing lines	Patch %	Lines
...ache/openwhisk/core/invoker/FPCInvokerServer.scala	72.72%	3 Missing ⚠️
...pache/openwhisk/core/invoker/InvokerReactive.scala	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #5098       +/-   ##
===========================================
+ Coverage   32.52%   75.16%   +42.64%     
===========================================
  Files         211      220        +9     
  Lines       10435    10723      +288     
  Branches      447      460       +13     
===========================================
+ Hits         3394     8060     +4666     
+ Misses       7041     2663     -4378

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ningyougang · 2021-04-26T01:35:06Z

One of the unit test case is failed all the time.

I pull above images in my local successfully, don't know the concrete reason why in travis is failed.

ningyougang added the wip label Mar 30, 2021

ningyougang force-pushed the FPCInvokerServer branch from faa7bab to 43caa23 Compare March 31, 2021 01:56

ningyougang removed the wip label Mar 31, 2021

ningyougang commented Mar 31, 2021

View reviewed changes

ningyougang force-pushed the FPCInvokerServer branch from 43caa23 to 241d26a Compare March 31, 2021 07:38

ningyougang closed this Apr 2, 2021

ningyougang reopened this Apr 2, 2021

Implement PFCInvokerServer

c2ea992

ningyougang force-pushed the FPCInvokerServer branch from d58eed9 to c2ea992 Compare April 14, 2021 05:17

style95 added the scheduler label Apr 14, 2021

style95 reviewed Apr 14, 2021

View reviewed changes

bdoyle0182 reviewed Apr 14, 2021

View reviewed changes

Remove /memory api

86f5531

style95 reviewed Apr 19, 2021

View reviewed changes

bdoyle0182 approved these changes Apr 19, 2021

View reviewed changes

style95 approved these changes Apr 20, 2021

View reviewed changes

Default implement enable/disable

10b2471

ningyougang closed this Apr 21, 2021

ningyougang reopened this Apr 21, 2021

jiangpengcheng approved these changes May 6, 2021

View reviewed changes

jiangpengcheng merged commit e036fc9 into apache:master May 6, 2021


		object DefaultInvokerServer extends InvokerServerProvider {

		// TODO: TBD, after FPCInvokerReactive is ready, can read the credentials from pureconfig

Uh oh!

Conversation

ningyougang commented Mar 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issue and scope

My changes affect the following components

Types of changes

Checklist:

Uh oh!

codecov-io commented Mar 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ningyougang Mar 31, 2021 • 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!

ningyougang Mar 31, 2021 • 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!

ningyougang Mar 31, 2021 • 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!

ningyougang commented Apr 14, 2021

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!

ningyougang Apr 15, 2021 • 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!

ningyougang Apr 15, 2021 • 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!

bdoyle0182 commented Apr 14, 2021

Uh oh!

ningyougang commented Apr 19, 2021

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!

codecov-commenter commented Apr 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ningyougang commented Apr 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ningyougang commented Mar 30, 2021 •

edited

Loading

codecov-io commented Mar 30, 2021 •

edited

Loading

ningyougang Mar 31, 2021 •

edited

Loading

ningyougang Mar 31, 2021 •

edited

Loading

ningyougang Mar 31, 2021 •

edited

Loading

ningyougang Apr 15, 2021 •

edited

Loading

ningyougang Apr 15, 2021 •

edited

Loading

codecov-commenter commented Apr 21, 2021 •

edited

Loading