You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2021/02/05 06:44:48 UTC

[GitHub] [openwhisk] KeonHee opened a new pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

KeonHee opened a new pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031


   <!--- Provide a concise summary of your changes in the Title -->
   ## Description
   
   This pr covers the etcd installation and client implementation. And the etcd will be the coordinator of cluster members and the metadata store in the new scheduler.
   
   
   ## My changes affect the following components
   <!--- Select below all system components are affected by your change. -->
   <!--- Enter an `x` in all applicable boxes. -->
   - [ ] API
   - [ ] Controller
   - [ ] Message Bus (e.g., Kafka)
   - [ ] Loadbalancer
   - [ ] Invoker
   - [ ] Intrinsic actions (e.g., sequences, conductors)
   - [x] Data stores (e.g., CouchDB)
   - [x] Tests
   - [x] Deployment
   - [ ] CLI
   - [ ] General tooling
   - [ ] Documentation
   
   ## Types of changes
   <!--- What types of changes does your code introduce? Use `x` in all the boxes that apply: -->
   - [ ] Bug fix (generally a non-breaking change which closes an issue).
   - [x] Enhancement or new feature (adds new functionality).
   - [ ] Breaking change (a bug fix or enhancement which changes existing behavior).
   
   ## Checklist:
   <!--- Please review the points below which help you make sure you've covered all aspects of the change you're making. -->
   
   - [x] I signed an [Apache CLA](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md).
   - [x] I reviewed the [style guides](https://github.com/apache/openwhisk/wiki/Contributing:-Git-guidelines#code-readiness) and followed the recommendations (Travis CI will check :).
   - [x] I added tests to cover my changes.
   - [ ] My changes require further changes to the documentation.
   - [ ] I updated the documentation where necessary.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] codecov-io commented on pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#issuecomment-773810582


   # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=h1) Report
   > Merging [#5031](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=desc) (ca1e806) into [master](https://codecov.io/gh/apache/openwhisk/commit/66868205b52ee65f28756038c44d8df5b96d2bcc?el=desc) (6686820) will **decrease** coverage by `8.84%`.
   > The diff coverage is `3.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/openwhisk/pull/5031/graphs/tree.svg?width=650&height=150&src=pr&token=l0YmsiSAso)](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #5031      +/-   ##
   ==========================================
   - Coverage   82.52%   73.67%   -8.85%     
   ==========================================
     Files         206      203       -3     
     Lines       10006     9902     -104     
     Branches      445      437       -8     
   ==========================================
   - Hits         8257     7295     -962     
   - Misses       1749     2607     +858     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [.../org/apache/openwhisk/core/entity/CreationId.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0NyZWF0aW9uSWQuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
   | [...la/org/apache/openwhisk/core/etcd/EtcdClient.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZXRjZC9FdGNkQ2xpZW50LnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...ala/org/apache/openwhisk/core/etcd/EtcdUtils.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZXRjZC9FdGNkVXRpbHMuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
   | [...cala/org/apache/openwhisk/http/ErrorResponse.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2h0dHAvRXJyb3JSZXNwb25zZS5zY2FsYQ==) | `87.75% <50.00%> (-2.87%)` | :arrow_down: |
   | [.../scala/org/apache/openwhisk/core/WhiskConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvV2hpc2tDb25maWcuc2NhbGE=) | `95.39% <100.00%> (+0.09%)` | :arrow_up: |
   | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...e/database/cosmosdb/cache/ChangeFeedConsumer.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NoYW5nZUZlZWRDb25zdW1lci5zY2FsYQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0.00% <0.00%> (-95.85%)` | :arrow_down: |
   | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0.00% <0.00%> (-93.90%)` | :arrow_down: |
   | ... and [50 more](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=footer). Last update [6686820...ca1e806](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] bdoyle0182 commented on a change in pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
bdoyle0182 commented on a change in pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#discussion_r539482010



##########
File path: ansible/roles/etcd/tasks/deploy.yml
##########
@@ -0,0 +1,56 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more contributor
+# license agreements; and to You under the Apache License, Version 2.0.
+---
+# This role will install etcd in group 'etcd' in the environment inventory
+
+- name: "Set the name of the etcd node"
+  set_fact:
+    name: "etcd{{ groups['etcd'].index(inventory_hostname) }}"
+
+- name: "set the volume_dir"
+  set_fact:
+    volume_dir: "{{ etcd.dir.data }}/etcd{{ groups['etcd'].index(inventory_hostname) }}:/etcd-data"
+  when: etcd_data_dir is defined
+
+
+- name: "Set the cluster of the etcd cluster"
+  set_fact:
+    cluster: "{% set etcdhosts = [] %}
+              {% for host in groups['etcd'] %}
+                  {{ etcdhosts.append('etcd' + ((loop.index-1)|string) + '=' + 'http://' + hostvars[host].ansible_host + ':' + ((2480+loop.index-1)|string) ) }}
+              {% endfor %}
+              {{ etcdhosts | join(',') }}"
+
+- name: (re)start etcd
+  docker_container:
+    name: etcd{{ groups['etcd'].index(inventory_hostname) }}
+    image: quay.io/coreos/etcd:{{ etcd.version }}
+    state: started
+    recreate: true
+    restart_policy: "{{ docker.restart.policy }}"
+    volumes: "{{volume_dir | default([])}}"
+    command: "/usr/local/bin/etcd \
+               --data-dir=/etcd-data --name '{{ name }}' \
+               --initial-advertise-peer-urls http://{{ ansible_host }}:{{ etcd.server.port + groups['etcd'].index(inventory_hostname) }} \
+               --advertise-client-urls http://{{ ansible_host }}:{{ etcd.client.port + groups['etcd'].index(inventory_hostname) }} \
+               --listen-peer-urls http://0.0.0.0:{{ etcd.server.port + groups['etcd'].index(inventory_hostname) }} \
+               --listen-client-urls http://0.0.0.0:{{ etcd.client.port + groups['etcd'].index(inventory_hostname) }} \
+               --initial-cluster {{ cluster }} \
+               --initial-cluster-state new --initial-cluster-token {{ etcd.cluster.token }} \
+               --quota-backend-bytes {{ etcd.quota_backend_bytes }} \
+               --snapshot-count {{ etcd.snapshot_count }} \
+               --auto-compaction-retention {{ etcd.auto_compaction_retention }} \
+               --auto-compaction-mode {{ etcd.auto_compaction_mode }} \
+               --log-level {{ etcd.loglevel }}"
+    ports:
+      - "{{ etcd.client.port + groups['etcd'].index(inventory_hostname) }}:{{ etcd.client.port + groups['etcd'].index(inventory_hostname) }}"
+      - "{{ etcd.server.port + groups['etcd'].index(inventory_hostname) }}:{{ etcd.server.port + groups['etcd'].index(inventory_hostname) }}"
+    pull: "{{ etcd.pull_etcd | default(true) }}"
+
+- name: wait until etcd in this host is up and running
+  uri:
+    url: "http://{{ ansible_host }}:{{ etcd.client.port + groups['etcd'].index(inventory_hostname) }}/health"
+  register: result
+  until: result.status == 200
+  retries: 12
+  delay: 5

Review comment:
       nit: new line
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] codecov-io edited a comment on pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#issuecomment-773810582






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] KeonHee commented on a change in pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
KeonHee commented on a change in pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#discussion_r570718825



##########
File path: tests/build.gradle
##########
@@ -73,10 +73,11 @@ ext.testSets = [
             "org/apache/openwhisk/core/cli/test/**",
             "org/apache/openwhisk/core/limits/**",
             "org/apache/openwhisk/core/scheduler/queue/test/ElasticSearchDurationCheck*",
+            "org/apache/openwhisk/common/etcd/**",

Review comment:
       Yes. One concern is that if the test is excluded, the code is merged without going through the CI test, so I would like to see if the CI also needs a pipeline for the scheduler.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] style95 commented on a change in pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#discussion_r570713822



##########
File path: tests/build.gradle
##########
@@ -73,10 +73,11 @@ ext.testSets = [
             "org/apache/openwhisk/core/cli/test/**",
             "org/apache/openwhisk/core/limits/**",
             "org/apache/openwhisk/core/scheduler/queue/test/ElasticSearchDurationCheck*",
+            "org/apache/openwhisk/common/etcd/**",

Review comment:
       So we would add tests for the scheduler when all contribution is over?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] KeonHee commented on a change in pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
KeonHee commented on a change in pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#discussion_r570692251



##########
File path: tests/build.gradle
##########
@@ -73,10 +73,11 @@ ext.testSets = [
             "org/apache/openwhisk/core/cli/test/**",
             "org/apache/openwhisk/core/limits/**",
             "org/apache/openwhisk/core/scheduler/queue/test/ElasticSearchDurationCheck*",
+            "org/apache/openwhisk/common/etcd/**",

Review comment:
       Excluded etcd from basic unit tests.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] codecov-io edited a comment on pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#issuecomment-773810582


   # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=h1) Report
   > Merging [#5031](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=desc) (ca1e806) into [master](https://codecov.io/gh/apache/openwhisk/commit/66868205b52ee65f28756038c44d8df5b96d2bcc?el=desc) (6686820) will **decrease** coverage by `7.67%`.
   > The diff coverage is `3.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/openwhisk/pull/5031/graphs/tree.svg?width=650&height=150&src=pr&token=l0YmsiSAso)](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #5031      +/-   ##
   ==========================================
   - Coverage   82.52%   74.85%   -7.68%     
   ==========================================
     Files         206      210       +4     
     Lines       10006    10167     +161     
     Branches      445      440       -5     
   ==========================================
   - Hits         8257     7610     -647     
   - Misses       1749     2557     +808     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [.../org/apache/openwhisk/core/entity/CreationId.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0NyZWF0aW9uSWQuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
   | [...la/org/apache/openwhisk/core/etcd/EtcdClient.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZXRjZC9FdGNkQ2xpZW50LnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...ala/org/apache/openwhisk/core/etcd/EtcdUtils.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZXRjZC9FdGNkVXRpbHMuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
   | [...cala/org/apache/openwhisk/http/ErrorResponse.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2h0dHAvRXJyb3JSZXNwb25zZS5zY2FsYQ==) | `89.79% <50.00%> (-0.83%)` | :arrow_down: |
   | [.../scala/org/apache/openwhisk/core/WhiskConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvV2hpc2tDb25maWcuc2NhbGE=) | `95.39% <100.00%> (+0.09%)` | :arrow_up: |
   | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...e/database/cosmosdb/cache/ChangeFeedConsumer.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NoYW5nZUZlZWRDb25zdW1lci5zY2FsYQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0.00% <0.00%> (-95.85%)` | :arrow_down: |
   | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0.00% <0.00%> (-93.90%)` | :arrow_down: |
   | ... and [28 more](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=footer). Last update [6686820...ca1e806](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] KeonHee commented on a change in pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
KeonHee commented on a change in pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#discussion_r570669969



##########
File path: ansible/roles/controller/tasks/deploy.yml
##########
@@ -272,6 +272,11 @@
 
       "CONFIG_whisk_controller_activation_pollingFromDb": "{{ controller_activation_pollingFromDb | default(true) | lower }}"
 
+      "CONFIG_whisk_etcd_hosts": "{{ etcd_connect_string }}"

Review comment:
       In the case of configuration, it would be good to add only what is needed when etcd is used in the controller & invoker. I will exclude it from this PR.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] codecov-io commented on pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#issuecomment-773810582


   # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=h1) Report
   > Merging [#5031](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=desc) (ca1e806) into [master](https://codecov.io/gh/apache/openwhisk/commit/66868205b52ee65f28756038c44d8df5b96d2bcc?el=desc) (6686820) will **decrease** coverage by `8.84%`.
   > The diff coverage is `3.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/openwhisk/pull/5031/graphs/tree.svg?width=650&height=150&src=pr&token=l0YmsiSAso)](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #5031      +/-   ##
   ==========================================
   - Coverage   82.52%   73.67%   -8.85%     
   ==========================================
     Files         206      203       -3     
     Lines       10006     9902     -104     
     Branches      445      437       -8     
   ==========================================
   - Hits         8257     7295     -962     
   - Misses       1749     2607     +858     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [.../org/apache/openwhisk/core/entity/CreationId.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0NyZWF0aW9uSWQuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
   | [...la/org/apache/openwhisk/core/etcd/EtcdClient.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZXRjZC9FdGNkQ2xpZW50LnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...ala/org/apache/openwhisk/core/etcd/EtcdUtils.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZXRjZC9FdGNkVXRpbHMuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
   | [...cala/org/apache/openwhisk/http/ErrorResponse.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2h0dHAvRXJyb3JSZXNwb25zZS5zY2FsYQ==) | `87.75% <50.00%> (-2.87%)` | :arrow_down: |
   | [.../scala/org/apache/openwhisk/core/WhiskConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvV2hpc2tDb25maWcuc2NhbGE=) | `95.39% <100.00%> (+0.09%)` | :arrow_up: |
   | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...e/database/cosmosdb/cache/ChangeFeedConsumer.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NoYW5nZUZlZWRDb25zdW1lci5zY2FsYQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0.00% <0.00%> (-95.85%)` | :arrow_down: |
   | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0.00% <0.00%> (-93.90%)` | :arrow_down: |
   | ... and [50 more](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=footer). Last update [6686820...ca1e806](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] KeonHee commented on a change in pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
KeonHee commented on a change in pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#discussion_r570669969



##########
File path: ansible/roles/controller/tasks/deploy.yml
##########
@@ -272,6 +272,11 @@
 
       "CONFIG_whisk_controller_activation_pollingFromDb": "{{ controller_activation_pollingFromDb | default(true) | lower }}"
 
+      "CONFIG_whisk_etcd_hosts": "{{ etcd_connect_string }}"

Review comment:
       In the case of configuration, it would be good to add only what is needed when etcd is used in the controller & invoker. I will exclude it from this PR.

##########
File path: tests/build.gradle
##########
@@ -73,10 +73,11 @@ ext.testSets = [
             "org/apache/openwhisk/core/cli/test/**",
             "org/apache/openwhisk/core/limits/**",
             "org/apache/openwhisk/core/scheduler/queue/test/ElasticSearchDurationCheck*",
+            "org/apache/openwhisk/common/etcd/**",

Review comment:
       Excluded etcd from basic unit tests.

##########
File path: tests/build.gradle
##########
@@ -197,6 +198,14 @@ task testUnit(type: Test) {
     exclude couchDbExcludes
 }
 
+task testUnitEtcd(type: Test) {

Review comment:
       The etcd test can be run with `./gradlew tests:testUnitEtcd`.
   

##########
File path: tests/build.gradle
##########
@@ -73,10 +73,11 @@ ext.testSets = [
             "org/apache/openwhisk/core/cli/test/**",
             "org/apache/openwhisk/core/limits/**",
             "org/apache/openwhisk/core/scheduler/queue/test/ElasticSearchDurationCheck*",
+            "org/apache/openwhisk/common/etcd/**",

Review comment:
       Yes. One concern is that if the test is excluded, the code is merged without going through the CI test, so I would like to see if the CI also needs a pipeline for the scheduler.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] JesseStutler commented on a change in pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
JesseStutler commented on a change in pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#discussion_r728965051



##########
File path: common/scala/src/main/scala/org/apache/openwhisk/core/etcd/EtcdUtils.scala
##########
@@ -0,0 +1,250 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.core.etcd
+
+import java.nio.charset.StandardCharsets
+
+import com.google.protobuf.ByteString
+import org.apache.openwhisk.core.ConfigKeys
+import org.apache.openwhisk.core.containerpool.ContainerId
+import org.apache.openwhisk.core.entity.SizeUnits.MB
+import org.apache.openwhisk.core.entity._
+import org.apache.openwhisk.core.entity.size._
+import pureconfig.loadConfigOrThrow
+
+import scala.language.implicitConversions
+import scala.util.Try
+
+case class EtcdConfig(hosts: String)
+
+case class EtcdException(msg: String) extends Exception(msg)
+
+/**
+ * If you import the line below, it implicitly converts ByteString type to Scala Type.
+ *
+ * import org.apache.openwhisk.core.etcd.EtcdType._
+ */
+object EtcdType {
+
+  implicit def stringToByteString(str: String): ByteString = ByteString.copyFromUtf8(str)
+
+  implicit def ByteStringToString(byteString: ByteString): String = byteString.toString(StandardCharsets.UTF_8)
+
+  implicit def ByteStringToInt(byteString: ByteString): Int = byteString.toString(StandardCharsets.UTF_8).toInt
+
+  implicit def IntToByteString(int: Int): ByteString = ByteString.copyFromUtf8(int.toString)
+
+  implicit def ByteStringToLong(byteString: ByteString): Long = byteString.toString(StandardCharsets.UTF_8).toLong
+
+  implicit def LongToByteString(long: Long): ByteString = ByteString.copyFromUtf8(long.toString)
+
+  implicit def ByteStringToBoolean(byteString: ByteString): Boolean =
+    byteString.toString(StandardCharsets.UTF_8).toBoolean
+
+  implicit def BooleanToByteString(bool: Boolean): ByteString = ByteString.copyFromUtf8(bool.toString)
+
+  implicit def ByteStringToByteSize(byteString: ByteString): ByteSize =
+    ByteSize(byteString.toString(StandardCharsets.UTF_8).toLong, MB)
+
+  implicit def ByteSizeToByteString(byteSize: ByteSize): ByteString = ByteString.copyFromUtf8(byteSize.toMB.toString)
+
+}
+
+object EtcdKV {
+
+  val TOP = "\ufff0"
+
+  val clusterName = loadConfigOrThrow[String](ConfigKeys.whiskClusterName)
+
+  object SchedulerKeys {
+    val prefix = s"$clusterName/scheduler"
+
+    val scheduler = s"$prefix"
+
+    /**
+     *  The keys for states of schedulers
+     */
+    def scheduler(instanceId: SchedulerInstanceId) = s"$prefix/${instanceId.asString}"
+
+  }
+
+  object QueueKeys {
+
+    val inProgressPrefix = s"$clusterName/in-progress"
+    val queuePrefix = s"$clusterName/queue"
+
+    /**
+     * The keys for in-progress queue
+     */
+    def inProgressQueue(invocationNamespace: String, fqn: FullyQualifiedEntityName) =
+      s"$inProgressPrefix/queue/$invocationNamespace/${fqn.copy(version = None)}"
+
+    /**
+     * The prefix key for state in the queue
+     */
+    def queuePrefix(invocationNamespace: String, fqn: FullyQualifiedEntityName): String =
+      s"$queuePrefix/$invocationNamespace/${fqn.copy(version = None)}"
+
+    /**
+     * The keys for state in the queue
+     *
+     * Example
+     *  - queue/invocationNs/ns/pkg/act/leader
+     *  - queue/invocationNs/ns/pkg/act/follower/scheduler1
+     *  - queue/invocationNs/ns/pkg/act/follower/scheduler2
+     *
+     */
+    def queue(invocationNamespace: String,

Review comment:
       Hi, sorry to bother you. I'm interested in openwhisk's new scheduler design and try to follow to do some extra customization for my idea. Now I'm trying to read the new scheduler's already merged code and take QueueManger first. I'm scala noob and new to openwhisk, when I read here, I got some questions:
   https://github.com/apache/openwhisk/blob/285f9d4afcc429d63baa1943a69bb64097b059dd/core/scheduler/src/main/scala/org/apache/openwhisk/core/scheduler/queue/QueueManager.scala#L447
   What does getLeaderKey's leader mean?And I have seen a lot QueueKeys.queue, I don't know what is queue method used for. Could you give some advice? Thanks a lot. :)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] bdoyle0182 commented on a change in pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
bdoyle0182 commented on a change in pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#discussion_r528473688



##########
File path: common/scala/src/main/scala/org/apache/openwhisk/core/etcd/EtcdClient.scala
##########
@@ -0,0 +1,262 @@
+package org.apache.openwhisk.core.etcd
+
+import com.google.common.util.concurrent.{FutureCallback, Futures, ListenableFuture}
+import com.ibm.etcd.api._
+import com.ibm.etcd.client.kv.KvClient.Watch
+import com.ibm.etcd.client.kv.{KvClient, WatchUpdate}
+import com.ibm.etcd.client.{EtcdClient => Client}
+import io.grpc.stub.StreamObserver
+import java.util.concurrent.Executors
+
+import org.apache.openwhisk.core.ConfigKeys
+import org.apache.openwhisk.core.etcd.EtcdType._
+import pureconfig.loadConfigOrThrow
+import spray.json.DefaultJsonProtocol
+
+import scala.language.implicitConversions
+import scala.annotation.tailrec
+import scala.concurrent.{ExecutionContextExecutor, Future, Promise}
+
+case class Lease(id: Long, ttl: Long)
+
+object RichListenableFuture {
+  implicit def convertToFuture[T](lf: ListenableFuture[T])(implicit ece: ExecutionContextExecutor): Future[T] = {
+    val p = Promise[T]()
+    Futures.addCallback(lf, new FutureCallback[T] {
+      def onFailure(t: Throwable): Unit = p failure t
+      def onSuccess(result: T): Unit = p success result
+    }, ece)
+    p.future
+  }
+}
+
+object EtcdClient {
+  // hostAndPorts format: {HOST}:{PORT}[,{HOST}:{PORT},{HOST}:{PORT}, ...]
+  def apply(hostAndPorts: String)(implicit ece: ExecutionContextExecutor): EtcdClient = {
+    require(hostAndPorts != null)
+    val client: Client = Client.forEndpoints(hostAndPorts).withPlainText().build()
+    new EtcdClient(client)(ece)
+  }
+
+  def apply(client: Client)(implicit ece: ExecutionContextExecutor): EtcdClient = {
+    new EtcdClient(client)(ece)
+  }
+}
+
+class EtcdClient(val client: Client)(override implicit val ece: ExecutionContextExecutor)
+    extends EtcdKeyValueApi
+    with EtcdLeaseApi
+    with EtcdWatchApi
+    with EtcdLeadershipApi {
+
+  def close() = {
+    client.close()
+  }
+}
+
+trait EtcdKeyValueApi extends KeyValueStore {
+  import RichListenableFuture._
+  protected[etcd] val client: Client
+
+  override def get(key: String): Future[RangeResponse] =
+    client.getKvClient.get(key).async()
+
+  override def getPrefix(prefixKey: String): Future[RangeResponse] = {
+    client.getKvClient.get(prefixKey).asPrefix().async()
+  }
+
+  override def getCount(prefixKey: String): Future[Long] = {
+    client.getKvClient.get(prefixKey).asPrefix().countOnly().async().map(_.getCount)
+  }
+
+  override def put(key: String, value: String): Future[PutResponse] =
+    client.getKvClient.put(key, value).async().recoverWith {
+      case t =>
+        Future.failed[PutResponse](getNestedException(t))
+    }
+
+  override def put(key: String, value: String, leaseId: Long): Future[PutResponse] =
+    client.getKvClient
+      .put(key, value, leaseId)
+      .async()
+      .recoverWith {
+        case t =>
+          Future.failed[PutResponse](getNestedException(t))
+      }
+
+  def put(key: String, value: Boolean): Future[PutResponse] = {
+    put(key, value.toString)
+  }
+
+  def put(key: String, value: Boolean, leaseId: Long): Future[PutResponse] = {
+    put(key, value.toString, leaseId)
+  }
+
+  override def del(key: String): Future[DeleteRangeResponse] =
+    client.getKvClient.delete(key).async().recoverWith {
+      case t =>
+        Future.failed[DeleteRangeResponse](getNestedException(t))
+    }
+
+  override def putTxn[T](key: String, value: T, cmpVersion: Long, leaseId: Long): Future[TxnResponse] = {
+    client.getKvClient
+      .txnIf()
+      .cmpEqual(key)
+      .version(cmpVersion)
+      .`then`()
+      .put(client.getKvClient
+        .put(key, value.toString, leaseId)
+        .asRequest())
+      .async()
+      .recoverWith {
+        case t =>
+          Future.failed[TxnResponse](getNestedException(t))
+      }
+  }
+
+  @tailrec
+  private def getNestedException(t: Throwable): Throwable = {
+    if (t.getCause == null) t
+    else getNestedException(t.getCause)
+  }
+}
+
+trait KeyValueStore {

Review comment:
       Will this be used as an SPI? If so should it be more specific than `KeyValueStore` and related to the scheduler with the function templates




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] style95 commented on pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
style95 commented on pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#issuecomment-774816022


   Many subsequent modules are dependent on this module.
   I would merge this at the end of today if there is no more comment.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] KeonHee commented on a change in pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
KeonHee commented on a change in pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#discussion_r570692382



##########
File path: tests/build.gradle
##########
@@ -197,6 +198,14 @@ task testUnit(type: Test) {
     exclude couchDbExcludes
 }
 
+task testUnitEtcd(type: Test) {

Review comment:
       The etcd test can be run with `./gradlew tests:testUnitEtcd`.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] codecov-io edited a comment on pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#issuecomment-773810582


   # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=h1) Report
   > Merging [#5031](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=desc) (ca1e806) into [master](https://codecov.io/gh/apache/openwhisk/commit/66868205b52ee65f28756038c44d8df5b96d2bcc?el=desc) (6686820) will **decrease** coverage by `7.66%`.
   > The diff coverage is `3.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/openwhisk/pull/5031/graphs/tree.svg?width=650&height=150&src=pr&token=l0YmsiSAso)](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #5031      +/-   ##
   ==========================================
   - Coverage   82.52%   74.85%   -7.67%     
   ==========================================
     Files         206      210       +4     
     Lines       10006    10167     +161     
     Branches      445      440       -5     
   ==========================================
   - Hits         8257     7611     -646     
   - Misses       1749     2556     +807     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [.../org/apache/openwhisk/core/entity/CreationId.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0NyZWF0aW9uSWQuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
   | [...la/org/apache/openwhisk/core/etcd/EtcdClient.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZXRjZC9FdGNkQ2xpZW50LnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...ala/org/apache/openwhisk/core/etcd/EtcdUtils.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZXRjZC9FdGNkVXRpbHMuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
   | [...cala/org/apache/openwhisk/http/ErrorResponse.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2h0dHAvRXJyb3JSZXNwb25zZS5zY2FsYQ==) | `89.79% <50.00%> (-0.83%)` | :arrow_down: |
   | [.../scala/org/apache/openwhisk/core/WhiskConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvV2hpc2tDb25maWcuc2NhbGE=) | `95.39% <100.00%> (+0.09%)` | :arrow_up: |
   | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...e/database/cosmosdb/cache/ChangeFeedConsumer.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NoYW5nZUZlZWRDb25zdW1lci5zY2FsYQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0.00% <0.00%> (-95.85%)` | :arrow_down: |
   | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0.00% <0.00%> (-93.90%)` | :arrow_down: |
   | ... and [28 more](https://codecov.io/gh/apache/openwhisk/pull/5031/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=footer). Last update [6686820...ca1e806](https://codecov.io/gh/apache/openwhisk/pull/5031?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] KeonHee closed pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
KeonHee closed pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] style95 commented on a change in pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#discussion_r569996413



##########
File path: ansible/roles/controller/tasks/deploy.yml
##########
@@ -272,6 +272,11 @@
 
       "CONFIG_whisk_controller_activation_pollingFromDb": "{{ controller_activation_pollingFromDb | default(true) | lower }}"
 
+      "CONFIG_whisk_etcd_hosts": "{{ etcd_connect_string }}"

Review comment:
       Since ShardingContainerPoolBalancer does not use etcd, should we selectively configure these configurations?

##########
File path: tests/src/test/scala/org/apache/openwhisk/common/etcd/EtcdKvTests.scala
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.common.etcd
+
+import org.apache.openwhisk.core.ConfigKeys
+import org.apache.openwhisk.core.entity.InvokerInstanceId
+import org.apache.openwhisk.core.entity.size._
+import org.apache.openwhisk.core.etcd.EtcdKV.InvokerKeys
+import org.junit.runner.RunWith
+import org.scalatest.concurrent.ScalaFutures
+import org.scalatest.junit.JUnitRunner
+import org.scalatest.{FlatSpec, Matchers}
+import pureconfig.loadConfigOrThrow
+
+@RunWith(classOf[JUnitRunner])
+class EtcdKvTests extends FlatSpec with ScalaFutures with Matchers {

Review comment:
       Should we need to selectively run these tests?

##########
File path: common/scala/src/main/scala/org/apache/openwhisk/core/etcd/EtcdClient.scala
##########
@@ -0,0 +1,262 @@
+package org.apache.openwhisk.core.etcd
+
+import com.google.common.util.concurrent.{FutureCallback, Futures, ListenableFuture}
+import com.ibm.etcd.api._
+import com.ibm.etcd.client.kv.KvClient.Watch
+import com.ibm.etcd.client.kv.{KvClient, WatchUpdate}
+import com.ibm.etcd.client.{EtcdClient => Client}
+import io.grpc.stub.StreamObserver
+import java.util.concurrent.Executors
+
+import org.apache.openwhisk.core.ConfigKeys
+import org.apache.openwhisk.core.etcd.EtcdType._
+import pureconfig.loadConfigOrThrow
+import spray.json.DefaultJsonProtocol
+
+import scala.language.implicitConversions
+import scala.annotation.tailrec
+import scala.concurrent.{ExecutionContextExecutor, Future, Promise}
+
+case class Lease(id: Long, ttl: Long)
+
+object RichListenableFuture {
+  implicit def convertToFuture[T](lf: ListenableFuture[T])(implicit ece: ExecutionContextExecutor): Future[T] = {
+    val p = Promise[T]()
+    Futures.addCallback(lf, new FutureCallback[T] {
+      def onFailure(t: Throwable): Unit = p failure t
+      def onSuccess(result: T): Unit = p success result
+    }, ece)
+    p.future
+  }
+}
+
+object EtcdClient {
+  // hostAndPorts format: {HOST}:{PORT}[,{HOST}:{PORT},{HOST}:{PORT}, ...]
+  def apply(hostAndPorts: String)(implicit ece: ExecutionContextExecutor): EtcdClient = {
+    require(hostAndPorts != null)
+    val client: Client = Client.forEndpoints(hostAndPorts).withPlainText().build()
+    new EtcdClient(client)(ece)
+  }
+
+  def apply(client: Client)(implicit ece: ExecutionContextExecutor): EtcdClient = {
+    new EtcdClient(client)(ece)
+  }
+}
+
+class EtcdClient(val client: Client)(override implicit val ece: ExecutionContextExecutor)
+    extends EtcdKeyValueApi
+    with EtcdLeaseApi
+    with EtcdWatchApi
+    with EtcdLeadershipApi {
+
+  def close() = {
+    client.close()
+  }
+}
+
+trait EtcdKeyValueApi extends KeyValueStore {
+  import RichListenableFuture._
+  protected[etcd] val client: Client
+
+  override def get(key: String): Future[RangeResponse] =
+    client.getKvClient.get(key).async()
+
+  override def getPrefix(prefixKey: String): Future[RangeResponse] = {
+    client.getKvClient.get(prefixKey).asPrefix().async()
+  }
+
+  override def getCount(prefixKey: String): Future[Long] = {
+    client.getKvClient.get(prefixKey).asPrefix().countOnly().async().map(_.getCount)
+  }
+
+  override def put(key: String, value: String): Future[PutResponse] =
+    client.getKvClient.put(key, value).async().recoverWith {
+      case t =>
+        Future.failed[PutResponse](getNestedException(t))
+    }
+
+  override def put(key: String, value: String, leaseId: Long): Future[PutResponse] =
+    client.getKvClient
+      .put(key, value, leaseId)
+      .async()
+      .recoverWith {
+        case t =>
+          Future.failed[PutResponse](getNestedException(t))
+      }
+
+  def put(key: String, value: Boolean): Future[PutResponse] = {
+    put(key, value.toString)
+  }
+
+  def put(key: String, value: Boolean, leaseId: Long): Future[PutResponse] = {
+    put(key, value.toString, leaseId)
+  }
+
+  override def del(key: String): Future[DeleteRangeResponse] =
+    client.getKvClient.delete(key).async().recoverWith {
+      case t =>
+        Future.failed[DeleteRangeResponse](getNestedException(t))
+    }
+
+  override def putTxn[T](key: String, value: T, cmpVersion: Long, leaseId: Long): Future[TxnResponse] = {
+    client.getKvClient
+      .txnIf()
+      .cmpEqual(key)
+      .version(cmpVersion)
+      .`then`()
+      .put(client.getKvClient
+        .put(key, value.toString, leaseId)
+        .asRequest())
+      .async()
+      .recoverWith {
+        case t =>
+          Future.failed[TxnResponse](getNestedException(t))
+      }
+  }
+
+  @tailrec
+  private def getNestedException(t: Throwable): Throwable = {
+    if (t.getCause == null) t
+    else getNestedException(t.getCause)
+  }
+}
+
+trait KeyValueStore {

Review comment:
       While this trait can be used to have a new store, but most of the methods and their signatures conform to ETCD.
   I feel like it is highly unlikely that we can replace it with another key-value store as it should support all functionalities such as TTL with lease/keepalive, Transaction, prefix, etc.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] style95 edited a comment on pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
style95 edited a comment on pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#issuecomment-774816022


   Many subsequent modules are dependent on this module.
   I would merge this in 24 hours if there is no more comment.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] style95 merged pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
style95 merged pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] KeonHee closed pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
KeonHee closed pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [openwhisk] style95 commented on a change in pull request #5031: [New Scheduler] Etcd installation & Implements EtcdClient

Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #5031:
URL: https://github.com/apache/openwhisk/pull/5031#discussion_r569996413



##########
File path: ansible/roles/controller/tasks/deploy.yml
##########
@@ -272,6 +272,11 @@
 
       "CONFIG_whisk_controller_activation_pollingFromDb": "{{ controller_activation_pollingFromDb | default(true) | lower }}"
 
+      "CONFIG_whisk_etcd_hosts": "{{ etcd_connect_string }}"

Review comment:
       Since ShardingContainerPoolBalancer does not use etcd, should we selectively configure these configurations?

##########
File path: tests/src/test/scala/org/apache/openwhisk/common/etcd/EtcdKvTests.scala
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.common.etcd
+
+import org.apache.openwhisk.core.ConfigKeys
+import org.apache.openwhisk.core.entity.InvokerInstanceId
+import org.apache.openwhisk.core.entity.size._
+import org.apache.openwhisk.core.etcd.EtcdKV.InvokerKeys
+import org.junit.runner.RunWith
+import org.scalatest.concurrent.ScalaFutures
+import org.scalatest.junit.JUnitRunner
+import org.scalatest.{FlatSpec, Matchers}
+import pureconfig.loadConfigOrThrow
+
+@RunWith(classOf[JUnitRunner])
+class EtcdKvTests extends FlatSpec with ScalaFutures with Matchers {

Review comment:
       Should we need to selectively run these tests?

##########
File path: common/scala/src/main/scala/org/apache/openwhisk/core/etcd/EtcdClient.scala
##########
@@ -0,0 +1,262 @@
+package org.apache.openwhisk.core.etcd
+
+import com.google.common.util.concurrent.{FutureCallback, Futures, ListenableFuture}
+import com.ibm.etcd.api._
+import com.ibm.etcd.client.kv.KvClient.Watch
+import com.ibm.etcd.client.kv.{KvClient, WatchUpdate}
+import com.ibm.etcd.client.{EtcdClient => Client}
+import io.grpc.stub.StreamObserver
+import java.util.concurrent.Executors
+
+import org.apache.openwhisk.core.ConfigKeys
+import org.apache.openwhisk.core.etcd.EtcdType._
+import pureconfig.loadConfigOrThrow
+import spray.json.DefaultJsonProtocol
+
+import scala.language.implicitConversions
+import scala.annotation.tailrec
+import scala.concurrent.{ExecutionContextExecutor, Future, Promise}
+
+case class Lease(id: Long, ttl: Long)
+
+object RichListenableFuture {
+  implicit def convertToFuture[T](lf: ListenableFuture[T])(implicit ece: ExecutionContextExecutor): Future[T] = {
+    val p = Promise[T]()
+    Futures.addCallback(lf, new FutureCallback[T] {
+      def onFailure(t: Throwable): Unit = p failure t
+      def onSuccess(result: T): Unit = p success result
+    }, ece)
+    p.future
+  }
+}
+
+object EtcdClient {
+  // hostAndPorts format: {HOST}:{PORT}[,{HOST}:{PORT},{HOST}:{PORT}, ...]
+  def apply(hostAndPorts: String)(implicit ece: ExecutionContextExecutor): EtcdClient = {
+    require(hostAndPorts != null)
+    val client: Client = Client.forEndpoints(hostAndPorts).withPlainText().build()
+    new EtcdClient(client)(ece)
+  }
+
+  def apply(client: Client)(implicit ece: ExecutionContextExecutor): EtcdClient = {
+    new EtcdClient(client)(ece)
+  }
+}
+
+class EtcdClient(val client: Client)(override implicit val ece: ExecutionContextExecutor)
+    extends EtcdKeyValueApi
+    with EtcdLeaseApi
+    with EtcdWatchApi
+    with EtcdLeadershipApi {
+
+  def close() = {
+    client.close()
+  }
+}
+
+trait EtcdKeyValueApi extends KeyValueStore {
+  import RichListenableFuture._
+  protected[etcd] val client: Client
+
+  override def get(key: String): Future[RangeResponse] =
+    client.getKvClient.get(key).async()
+
+  override def getPrefix(prefixKey: String): Future[RangeResponse] = {
+    client.getKvClient.get(prefixKey).asPrefix().async()
+  }
+
+  override def getCount(prefixKey: String): Future[Long] = {
+    client.getKvClient.get(prefixKey).asPrefix().countOnly().async().map(_.getCount)
+  }
+
+  override def put(key: String, value: String): Future[PutResponse] =
+    client.getKvClient.put(key, value).async().recoverWith {
+      case t =>
+        Future.failed[PutResponse](getNestedException(t))
+    }
+
+  override def put(key: String, value: String, leaseId: Long): Future[PutResponse] =
+    client.getKvClient
+      .put(key, value, leaseId)
+      .async()
+      .recoverWith {
+        case t =>
+          Future.failed[PutResponse](getNestedException(t))
+      }
+
+  def put(key: String, value: Boolean): Future[PutResponse] = {
+    put(key, value.toString)
+  }
+
+  def put(key: String, value: Boolean, leaseId: Long): Future[PutResponse] = {
+    put(key, value.toString, leaseId)
+  }
+
+  override def del(key: String): Future[DeleteRangeResponse] =
+    client.getKvClient.delete(key).async().recoverWith {
+      case t =>
+        Future.failed[DeleteRangeResponse](getNestedException(t))
+    }
+
+  override def putTxn[T](key: String, value: T, cmpVersion: Long, leaseId: Long): Future[TxnResponse] = {
+    client.getKvClient
+      .txnIf()
+      .cmpEqual(key)
+      .version(cmpVersion)
+      .`then`()
+      .put(client.getKvClient
+        .put(key, value.toString, leaseId)
+        .asRequest())
+      .async()
+      .recoverWith {
+        case t =>
+          Future.failed[TxnResponse](getNestedException(t))
+      }
+  }
+
+  @tailrec
+  private def getNestedException(t: Throwable): Throwable = {
+    if (t.getCause == null) t
+    else getNestedException(t.getCause)
+  }
+}
+
+trait KeyValueStore {

Review comment:
       While this trait can be used to have a new store, but most of the methods and their signatures conform to ETCD.
   I feel like it is highly unlikely that we can replace it with another key-value store as it should support all functionalities such as TTL with lease/keepalive, Transaction, prefix, etc.

##########
File path: tests/build.gradle
##########
@@ -73,10 +73,11 @@ ext.testSets = [
             "org/apache/openwhisk/core/cli/test/**",
             "org/apache/openwhisk/core/limits/**",
             "org/apache/openwhisk/core/scheduler/queue/test/ElasticSearchDurationCheck*",
+            "org/apache/openwhisk/common/etcd/**",

Review comment:
       So we would add tests for the scheduler when all contribution is over?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org