8000 [New Scheduler]Add CreationJobManager by jiangpengcheng · Pull Request #5116 · apache/openwhisk · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[New Scheduler]Add CreationJobManager #5116

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

8000
Merged
merged 6 commits into from
Jun 7, 2021

Conversation

jiangpengcheng
Copy link
Contributor

Description

This module records container creation using a CreationId, when a new container creation request is created, one entry with an unique CreationId will be created and saved in to ETCD, and will be deleted when creation is finished(or timeout waiting for creation complete ack).

So that a MemoryQueue can query how many containers are being created from ETCD and then do containers scheduling based on this value.

It can also help to trace/debug the procedure of a container creation.

Related issue and scope

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

My changes affect the following components

  • API
  • Controller
  • Message Bus (e.g., Kafka)
  • Loadbalancer
  • Scheduler
  • Invoker
  • Intrinsic actions (e.g., sequences, conductors)
  • Data stores (e.g., CouchDB)
  • Tests
  • Deployment
  • CLI
  • General tooling
  • Documentation

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.

@jiangpengcheng jiangpengcheng force-pushed the add_creation_job_manager branch from 12e8fff to 1700c8d Compare May 24, 2021 09:59
@style95 style95 requested a review from KeonHee May 25, 2021 01:59
@codecov-commenter
Copy link
codecov-commenter commented May 25, 2021

Codecov Report

Merging #5116 (58d6fb9) into master (f1829e1) will decrease coverage by 6.11%.
The diff coverage is 83.58%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5116      +/-   ##
==========================================
- Coverage   81.86%   75.75%   -6.12%     
==========================================
  Files         225      227       +2     
  Lines       11423    11528     +105     
  Branches      501      497       -4     
==========================================
- Hits         9352     8733     -619     
- Misses       2071     2795     +724     
Impacted Files Coverage Δ
...sk/core/scheduler/container/ContainerManager.scala 88.71% <ø> (+1.95%) ⬆️
.../core/scheduler/container/CreationJobManager.scala 83.33% <83.33%> (ø)
.../scala/org/apache/openwhisk/core/WhiskConfig.scala 95.59% <100.00%> (+0.02%) ⬆️
...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%) ⬇️
...enwhisk/connector/kafka/KamonMetricsReporter.scala 0.00% <0.00%> (-83.34%) ⬇️
... and 24 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 f1829e1...58d6fb9. Read the comment docs.


private val topicPrefix = loadConfigOrThrow[String](ConfigKeys.kafkaTopicsPrefix)
private val topic = s"${topicPrefix}creationAck${schedulerInstanceId.asString}"
private val maxActiveAcksPerPoll = 128
Copy link
Contributor

Choose a reason for hiding this comment

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

In Scheduler.scala, this value is configurable.
Can we make it configurable as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it seems other places are also hardcoded

dataManagementService: ActorRef)(implicit actorSystem: ActorSystem, logging: Logging)
extends Actor {
private implicit val ec: ExecutionContext = actorSystem.dispatcher
private val baseTimeout = loadConfigOrThrow[Int](ConfigKeys.schedulerInProgressJobRetentionSecond).seconds
Copy link
Contributor

Choose a reason for hiding this comment

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

For time configuration, it is better to change it to loadConfigOrThrow[FiniteDuration]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great catch, I will update

@ningyougang
Copy link
Contributor

LGTM with some small suggestions

@jiangpengcheng jiangpengcheng force-pushed the add_creation_job_manager branch from 732e6e2 to 219c069 Compare May 27, 2021 06:51
@ningyougang ningyougang merged commit 71585f1 into apache:master Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0