[New Scheduler] Add ActivationService #5070

upgle · 2021-02-19T07:31:00Z

Description

ActivationService is a GRPC service implementation for the scheduler that communicates with the container proxy.

Please refer to this document:
https://cwiki.apache.org/confluence/display/OPENWHISK/ActivationServiceImpl

Related issue and scope

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

My changes affect the following components

Scheduler

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.

bdoyle0182 · 2021-02-22T23:21:42Z

core/scheduler/src/main/java/Empty.java

+public class Empty {
+    // Workaround for this issue https://github.com/akka/akka-grpc/issues/289
+    // Gradle complains about no java sources.
+    // Note. Openwhisk is using a lower gradle version, so the latest akka-grpc version cannot be used.


Can we fix this? The issue has been fixed

I've already tried it, but It requires a higher Gradle version to use the latest akka-grpc.

bdoyle0182 · 2021-02-22T23:23:13Z

core/scheduler/src/main/protobuf/activation.proto

+
+//#services
+service ActivationService {
+


nit: remove extra lines

bdoyle0182 · 2021-02-22T23:28:02Z

...cheduler/src/main/scala/org/apache/openwhisk/core/scheduler/grpc/ActivationServiceImpl.scala

+import scala.util.Try
+
+class ActivationServiceImpl()(implicit actorSystem: ActorSystem, logging: Logging) extends ActivationService {
+  implicit val requestTimeout: Timeout = Timeout(50.seconds)


should this not be hardcoded?

bdoyle0182 · 2021-02-22T23:28:31Z

...cheduler/src/main/scala/org/apache/openwhisk/core/scheduler/grpc/ActivationServiceImpl.scala

+import scala.util.Try
+
+class ActivationServiceImpl()(implicit actorSystem: ActorSystem, logging: Logging) extends ActivationService {
+  implicit val requestTimeout: Timeout = Timeout(50.seconds)


should this not be hardcoded?

bdoyle0182 · 2021-02-22T23:28:49Z

...cheduler/src/main/scala/org/apache/openwhisk/core/scheduler/grpc/ActivationServiceImpl.scala

+        QueuePool.get(MemoryQueueKey(request.invocationNamespace, key)) match {
+          case Some(queueValue) =>
+            // enqueue activation message to reschedule
+            logging.info(


Should this be debug

bdoyle0182 · 2021-02-22T23:29:21Z

...cheduler/src/main/scala/org/apache/openwhisk/core/scheduler/grpc/ActivationServiceImpl.scala

+  implicit val ec: ExecutionContextExecutor = actorSystem.dispatcher
+
+  override def rescheduleActivation(request: RescheduleRequest): Future[RescheduleResponse] = {
+    logging.info(this, s"Try to reschedule activation ${request.invocationNamespace} ${request.fqn} ${request.rev}")


I think this should be debug statement

Rescheduling is a special case that happens very occasionally. I think we can keep this log with the info level.

bdoyle0182 · 2021-02-22T23:46:01Z

core/scheduler/src/main/scala/org/apache/openwhisk/core/scheduler/queue/QueueManager.scala

+  implicit val serdes = jsonFormat(ActionMismatch.apply _, "actionMisMatch")
+}
+
+object QueuePool {


Is this safe as a global static object being accessed from futures?

The only field _queuePool(TrieMap[MemoryQueueKey, MemoryQueueValue]) which is included in this object is a thread-safe.

ningyougang · 2021-02-26T06:20:56Z

core/scheduler/src/main/protobuf/activation.proto

+
+    rpc FetchActivation (FetchRequest) returns (FetchResponse) {}
+
+    rpc RescheduleActivation (RescheduleRequest) returns (RescheduleResponse) {}


For the design document, RescheduleActivation rpc is for rescheduling when the container proxy cannot process messages, can give some example here? i think just describe it is enough.

codecov-io · 2021-03-04T09:05:41Z

Codecov Report

Merging #5070 (4d9c7c6) into master (4a13303) will decrease coverage by 43.52%.
The diff coverage is 75.82%.

@@             Coverage Diff             @@
##           master    #5070       +/-   ##
===========================================
- Coverage   81.95%   38.42%   -43.53%     
===========================================
  Files         210      218        +8     
  Lines       10167    10700      +533     
  Branches      440      450       +10     
===========================================
- Hits         8332     4112     -4220     
- Misses       1835     6588     +4753

Impacted Files	Coverage Δ
.../openwhisk/core/scheduler/queue/QueueManager.scala	`56.75% <56.75%> (ø)`
...sk/core/scheduler/grpc/ActivationServiceImpl.scala	`88.00% <88.00%> (ø)`
...ala/org/apache/openwhisk/core/entity/DocInfo.scala	`81.81% <100.00%> (-5.28%)`	⬇️
...enwhisk/core/entity/FullyQualifiedEntityName.scala	`72.97% <100.00%> (-18.46%)`	⬇️
...a/org/apache/openwhisk/common/ConfigMapValue.scala	`0.00% <0.00%> (-100.00%)`	⬇️
.../apache/openwhisk/core/controller/Namespaces.scala	`0.00% <0.00%> (-100.00%)`	⬇️
...pache/openwhisk/core/controller/CorsSettings.scala	`0.00% <0.00%> (-100.00%)`	⬇️
...che/openwhisk/core/entitlement/RateThrottler.scala	`0.00% <0.00%> (-100.00%)`	⬇️
...he/openwhisk/core/entitlement/KindRestrictor.scala	`0.00% <0.00%> (-100.00%)`	⬇️
...penwhisk/core/database/cosmosdb/CosmosDBUtil.scala	`0.00% <0.00%> (-100.00%)`	⬇️
... and 149 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 4a13303...4d9c7c6. Read the comment docs.

style95 · 2021-03-18T23:25:49Z

Seems there are some formatting errors.

[/home/travis/build/apache/openwhisk/tools/travis/../../core/scheduler/build/generated/source/proto/main/akkaGrpc/org/apache/openwhisk/grpc/ActivationService.scala]:
       1: file does not include required license header.
       9: line has trailing whitespace.
      10: line has trailing whitespace.
      12: line has trailing whitespace.
      13: line has trailing whitespace.
      15: line has trailing whitespace.
      23: line has trailing whitespace.
      25: line has trailing whitespace.
      27: line has trailing whitespace.
      29: line has trailing whitespace.
      31: line has trailing whitespace.
  [/home/travis/build/apache/openwhisk/tools/travis/../../core/scheduler/build/generated/source/proto/main/akkaGrpc/org/apache/openwhisk/grpc/ActivationServiceClient.scala]:
       1: file does not include required license header.
      37: line has trailing whitespace.
      39: line has trailing whitespace.
      41: line has trailing whitespace.
      43: line has trailing whitespace.

upgle added 2 commits February 19, 2021 16:07

Add ActivationService for scheduler

1cd9853

Add annotation

31085d3

upgle added the scheduler label Feb 19, 2021

Add license header

1941ffb

bdoyle0182 reviewed Feb 22, 2021

View reviewed changes

Reformat activation.proto

6ec11c4

ningyougang reviewed Feb 26, 2021

View reviewed changes

Reduce request timeout

83f8313

Add license header

2a4c69e

upgle mentioned this pull request Apr 6, 2021

Exclude auto-generated protobuf files apache/openwhisk-utilities#81

Closed

Scan code before compiling the code

4d9c7c6

style95 approved these changes Apr 13, 2021

View reviewed changes

style95 merged commit cd6fded into apache:master Apr 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[New Scheduler] Add ActivationService #5070

[New Scheduler] Add ActivationService #5070

upgle commented Feb 19, 2021 •

edited

Loading

bdoyle0182 Feb 22, 2021

upgle Feb 26, 2021

bdoyle0182 Feb 22, 2021

bdoyle0182 Feb 22, 2021

bdoyle0182 Feb 22, 2021

bdoyle0182 Feb 22, 2021

bdoyle0182 Feb 22, 2021

upgle Feb 26, 2021

bdoyle0182 Feb 22, 2021

upgle Mar 4, 2021

ningyougang Feb 26, 2021

codecov-io commented Mar 4, 2021 •

edited

Loading

style95 commented Mar 18, 2021


		rpc FetchActivation (FetchRequest) returns (FetchResponse) {}

		rpc RescheduleActivation (RescheduleRequest) returns (RescheduleResponse) {}

[New Scheduler] Add ActivationService #5070

[New Scheduler] Add ActivationService #5070

Conversation

upgle commented Feb 19, 2021 • edited Loading

Description

Related issue and scope

My changes affect the following components

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Mar 4, 2021 • edited Loading

Codecov Report

style95 commented Mar 18, 2021

upgle commented Feb 19, 2021 •

edited

Loading

codecov-io commented Mar 4, 2021 •

edited

Loading