Skip to content
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

[New Scheduler] Manage container creation #5074

Merged
merged 12 commits into from May 26, 2021
Merged

[New Scheduler] Manage container creation #5074

merged 12 commits into from May 26, 2021

Conversation

KeonHee
Copy link
Member

@KeonHee KeonHee commented Mar 2, 2021

Description

  • This module determines which invoker to create the container from.

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.

@codecov-io
Copy link

codecov-io commented Mar 2, 2021

Codecov Report

Merging #5074 (9f4708d) into master (cd6fded) will decrease coverage by 6.07%.
The diff coverage is 74.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5074      +/-   ##
==========================================
- Coverage   81.25%   75.17%   -6.08%     
==========================================
  Files         218      222       +4     
  Lines       10700    11010     +310     
  Branches      450      484      +34     
==========================================
- Hits         8694     8277     -417     
- Misses       2006     2733     +727     
Impacted Files Coverage Δ
.../apache/openwhisk/core/controller/Controller.scala 54.80% <ø> (ø)
...enwhisk/core/loadBalancer/InvokerSupervision.scala 88.81% <ø> (-0.46%) ⬇️
...che/openwhisk/core/loadBalancer/LoadBalancer.scala 90.00% <ø> (+13.07%) ⬆️
...e/loadBalancer/ShardingContainerPoolBalancer.scala 80.79% <ø> (ø)
...k/core/containerpool/v2/InvokerHealthManager.scala 75.00% <ø> (-0.38%) ⬇️
.../org/apache/openwhisk/core/connector/Message.scala 64.76% <23.63%> (-14.22%) ⬇️
.../main/scala/org/apache/openwhisk/core/WarmUp.scala 64.28% <64.28%> (ø)
...la/org/apache/openwhisk/common/TransactionId.scala 94.25% <75.00%> (+0.13%) ⬆️
...in/scala/org/apache/openwhisk/common/Message.scala 77.77% <77.77%> (ø)
...sk/core/scheduler/container/ContainerManager.scala 86.76% <86.76%> (ø)
... and 36 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...9f4708d. Read the comment docs.

@KeonHee KeonHee changed the title [WIP] [New Scheduler] Manage container creation [New Scheduler] Manage container creation Mar 22, 2021
@KeonHee KeonHee changed the title [New Scheduler] Manage container creation [WIP] [New Scheduler] Manage container creation Mar 23, 2021
@KeonHee KeonHee changed the title [WIP] [New Scheduler] Manage container creation [New Scheduler] Manage container creation Mar 23, 2021
@KeonHee
Copy link
Member Author

KeonHee commented Apr 6, 2021

The standalone test continues to fail because an error occurred while pulling openwhisk/action-nodejs-v10:nightly.

$ docker pull openwhisk/action-nodejs-v10:nightly
Error response from daemon: Get https://registry-1.docker.io/v2/: net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)

@style95
Copy link
Member

style95 commented Apr 7, 2021

The standalone test continues to fail because an error occurred while pulling openwhisk/action-nodejs-v10:nightly.

$ docker pull openwhisk/action-nodejs-v10:nightly
Error response from daemon: Get https://registry-1.docker.io/v2/: net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)

I worked around this by pulling such kind images in advance before starting tests.

result = prime * result + tags.hashCode()
result = prime * result + dedicatedNamespaces.hashCode()
result
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Regarding InvokerResourceMessage's equals and hashCode, it seems we have no need to override these 2 methods.
image

@@ -139,6 +139,7 @@ registry:
confdir: "{{ config_root_dir }}/registry"

kafka:
topicsPrefix: "{{ kafka_topics_prefix | default('') }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Add prefix for kafka topic feature is already implemented in this pr: #5062

Copy link
Member

@style95 style95 left a comment

Choose a reason for hiding this comment

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

LGTM
@KeonHee

Can we add some design documents for this component especially regarding the container creation/deletion process and warmup process here?
https://cwiki.apache.org/confluence/display/OPENWHISK/Component+Design

@KeonHee
Copy link
Member Author

KeonHee commented Apr 29, 2021

@style95 Sure. I will upload it ASAP

@KeonHee
Copy link
Member Author

KeonHee commented May 6, 2021

I wrote a document: https://cwiki.apache.org/confluence/display/OPENWHISK/ContainerManager

@style95
Copy link
Member

style95 commented May 6, 2021

I wrote a document: https://cwiki.apache.org/confluence/display/OPENWHISK/ContainerManager

Can we add a section about warmup?

@codecov-commenter
Copy link

codecov-commenter commented May 7, 2021

Codecov Report

Merging #5074 (d4a43d6) into master (9c445f3) will decrease coverage by 5.56%.
The diff coverage is 84.49%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5074      +/-   ##
==========================================
- Coverage   81.11%   75.54%   -5.57%     
==========================================
  Files         214      225      +11     
  Lines       10913    11423     +510     
  Branches      469      501      +32     
==========================================
- Hits         8852     8630     -222     
- Misses       2061     2793     +732     
Impacted Files Coverage Δ
.../apache/openwhisk/core/controller/Controller.scala 55.66% <ø> (ø)
...enwhisk/core/loadBalancer/InvokerSupervision.scala 88.81% <ø> (-0.46%) ⬇️
...che/openwhisk/core/loadBalancer/LoadBalancer.scala 90.00% <ø> (+13.07%) ⬆️
...e/loadBalancer/ShardingContainerPoolBalancer.scala 80.79% <ø> (ø)
...k/core/containerpool/v2/InvokerHealthManager.scala 75.00% <ø> (-0.38%) ⬇️
.../main/scala/org/apache/openwhisk/core/WarmUp.scala 64.28% <64.28%> (ø)
...la/org/apache/openwhisk/common/TransactionId.scala 94.25% <75.00%> (+0.13%) ⬆️
...in/scala/org/apache/openwhisk/common/Message.scala 77.77% <77.77%> (ø)
...sk/core/scheduler/container/ContainerManager.scala 86.76% <86.76%> (ø)
...in/scala/org/apache/openwhisk/common/Logging.scala 77.22% <100.00%> (-8.35%) ⬇️
... and 42 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 9c445f3...d4a43d6. Read the comment docs.

@style95 style95 merged commit f1829e1 into apache:master May 26, 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.

None yet

5 participants