[GitHub] markusthoemmes closed pull request #3266: Use always all available controllers.

[GitBox <gi...@apache.org>]

markusthoemmes closed pull request #3266: Use always all available controllers.
URL: https://github.com/apache/incubator-openwhisk/pull/3266
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/ansible/group_vars/all b/ansible/group_vars/all
index 8f14ea60d3..23eeeac2c6 100644
--- a/ansible/group_vars/all
+++ b/ansible/group_vars/all
@@ -18,7 +18,7 @@ exclude_logs_from: []
 #   whisk_api_localhost_name_default (last)
 whisk_api_localhost_name_default: "localhost"
 
-hosts_dir: "{{ inventory_dir|?default(env_hosts_dir) }}" 
+hosts_dir: "{{ inventory_dir|?default(env_hosts_dir) }}"
 
 whisk:
   version:
@@ -54,8 +54,6 @@ controller:
       bindPort: 2551
       # at this moment all controllers are seed nodes
       seedNodes: "{{ groups['controllers'] | map('extract', hostvars, 'ansible_host') | list }}"
-  # We recommend to enable HA for the controllers only, if bookkeeping data are shared too. (localBookkeeping: false)
-  ha: "{{ controller_enable_ha | default(True) and groups['controllers'] | length > 1 }}"
   loadbalancer:
     spi: "{{ controller_loadbalancer_spi | default('') }}"
   loglevel: "{{ controller_loglevel | default(whisk_loglevel) | default('INFO') }}"
diff --git a/ansible/roles/controller/tasks/deploy.yml b/ansible/roles/controller/tasks/deploy.yml
index 3fa07b5802..f07566400e 100644
--- a/ansible/roles/controller/tasks/deploy.yml
+++ b/ansible/roles/controller/tasks/deploy.yml
@@ -140,7 +140,6 @@
 
       "CONTROLLER_LOCALBOOKKEEPING": "{{ controller.localBookkeeping }}"
       "AKKA_CLUSTER_SEED_NODES": "{{seed_nodes_list | join(' ') }}"
-      "CONTROLLER_HA": "{{ controller.ha }}"
 
       "METRICS_KAMON": "{{ metrics.kamon.enabled }}"
       "METRICS_LOG": "{{ metrics.log.enabled }}"
diff --git a/ansible/roles/nginx/templates/nginx.conf.j2 b/ansible/roles/nginx/templates/nginx.conf.j2
index e8e447bfe3..c31026cc66 100644
--- a/ansible/roles/nginx/templates/nginx.conf.j2
+++ b/ansible/roles/nginx/templates/nginx.conf.j2
@@ -29,11 +29,8 @@ http {
         # Otherwise, nginx would dispatch requests when the container is up, but the backend in the container not.
         # From the docs:
         # "normally, requests with a non-idempotent method (POST, LOCK, PATCH) are not passed to the next server if a request has been sent to an upstream server"
-        server {{ hostvars[groups['controllers'] | first].ansible_host }}:{{ controller.basePort }} fail_timeout=60s;
-{% for ip in groups['controllers'] %}
-{% if groups['controllers'].index(ip) > 0 %}
-        server {{ hostvars[ip].ansible_host }}:{{ controller.basePort + groups['controllers'].index(ip) }} {% if controller.ha %}fail_timeout=60s{% else %}backup{% endif %};
-{% endif %}
+{% for c in groups['controllers'] %}
+        server {{ hostvars[c].ansible_host }}:{{ controller.basePort + groups['controllers'].index(c) }} fail_timeout=60s;
 {% endfor %}
         keepalive 512;
     }
diff --git a/ansible/templates/whisk.properties.j2 b/ansible/templates/whisk.properties.j2
index 9096b9eaa5..f8d6a1bdde 100644
--- a/ansible/templates/whisk.properties.j2
+++ b/ansible/templates/whisk.properties.j2
@@ -56,7 +56,6 @@ invoker.hosts.basePort={{ invoker.port }}
 controller.hosts={{ groups["controllers"] | map('extract', hostvars, 'ansible_host') | list | join(",") }}
 controller.host.basePort={{ controller.basePort }}
 controller.instances={{ controller.instances }}
-controller.ha={{ controller.ha }}
 
 invoker.container.network=bridge
 invoker.container.policy={{ invoker_container_policy_name | default()}}
diff --git a/common/scala/src/main/scala/whisk/core/WhiskConfig.scala b/common/scala/src/main/scala/whisk/core/WhiskConfig.scala
index c538d6ec56..cf66627ed0 100644
--- a/common/scala/src/main/scala/whisk/core/WhiskConfig.scala
+++ b/common/scala/src/main/scala/whisk/core/WhiskConfig.scala
@@ -97,7 +97,6 @@ class WhiskConfig(requiredProperties: Map[String, String],
   val actionSequenceLimit = this(WhiskConfig.actionSequenceMaxLimit)
   val controllerSeedNodes = this(WhiskConfig.controllerSeedNodes)
   val controllerLocalBookkeeping = getAsBoolean(WhiskConfig.controllerLocalBookkeeping, false)
-  val controllerHighAvailability = getAsBoolean(WhiskConfig.controllerHighAvailability, false)
 }
 
 object WhiskConfig {
@@ -231,7 +230,6 @@ object WhiskConfig {
   val triggerFirePerMinuteLimit = "limits.triggers.fires.perMinute"
   val controllerSeedNodes = "akka.cluster.seed.nodes"
   val controllerLocalBookkeeping = "controller.localBookkeeping"
-  val controllerHighAvailability = "controller.ha"
 }
 
 object ConfigKeys {
diff --git a/core/controller/src/main/scala/whisk/core/entitlement/Entitlement.scala b/core/controller/src/main/scala/whisk/core/entitlement/Entitlement.scala
index d148eb78fd..1aa6e3c341 100644
--- a/core/controller/src/main/scala/whisk/core/entitlement/Entitlement.scala
+++ b/core/controller/src/main/scala/whisk/core/entitlement/Entitlement.scala
@@ -67,8 +67,7 @@ protected[core] object EntitlementProvider {
     WhiskConfig.actionInvokeConcurrentLimit -> null,
     WhiskConfig.triggerFirePerMinuteLimit -> null,
     WhiskConfig.actionInvokeSystemOverloadLimit -> null,
-    WhiskConfig.controllerInstances -> null,
-    WhiskConfig.controllerHighAvailability -> null)
+    WhiskConfig.controllerInstances -> null)
 }
 
 /**
@@ -85,7 +84,7 @@ protected[core] abstract class EntitlementProvider(config: WhiskConfig, loadBala
    * Allows 20% of additional requests on top of the limit to mitigate possible unfair round-robin loadbalancing between
    * controllers
    */
-  private val overcommit = if (config.controllerHighAvailability) 1.2 else 1
+  private val overcommit = if (config.controllerInstances.toInt > 1) 1.2 else 1
   private def dilateLimit(limit: Int): Int = Math.ceil(limit.toDouble * overcommit).toInt
 
   /**
diff --git a/core/controller/src/main/scala/whisk/core/loadBalancer/ContainerPoolBalancer.scala b/core/controller/src/main/scala/whisk/core/loadBalancer/ContainerPoolBalancer.scala
index f3c2f0d875..76e523f754 100644
--- a/core/controller/src/main/scala/whisk/core/loadBalancer/ContainerPoolBalancer.scala
+++ b/core/controller/src/main/scala/whisk/core/loadBalancer/ContainerPoolBalancer.scala
@@ -92,7 +92,7 @@ class ContainerPoolBalancer(config: WhiskConfig, controllerInstance: InstanceId)
 
   override def totalActiveActivations = loadBalancerData.totalActivationCount
 
-  override def clusterSize = if (config.controllerHighAvailability) config.controllerInstances.toInt else 1
+  override def clusterSize = config.controllerInstances.toInt
 
   /**
    * Tries to fill in the result slot (i.e., complete the promise) when a completion message arrives.
diff --git a/tests/src/test/scala/limits/ThrottleTests.scala b/tests/src/test/scala/limits/ThrottleTests.scala
index bfbc3f69b3..9aee3fb6f1 100644
--- a/tests/src/test/scala/limits/ThrottleTests.scala
+++ b/tests/src/test/scala/limits/ThrottleTests.scala
@@ -44,7 +44,6 @@ import common.WskProps
 import common.WskTestHelpers
 import spray.json._
 import spray.json.DefaultJsonProtocol._
-import whisk.core.WhiskConfig
 import whisk.http.Messages._
 import whisk.utils.ExecutionContextFactory
 import whisk.utils.retry
@@ -74,7 +73,7 @@ class ThrottleTests
   val throttleWindow = 1.minute
 
   // Due to the overhead of the per minute limit in the controller, we add this overhead here as well.
-  val overhead = if (WhiskProperties.getProperty(WhiskConfig.controllerHighAvailability).toBoolean) 1.2 else 1.0
+  val overhead = if (WhiskProperties.getControllerHosts.split(",").length > 1) 1.2 else 1.0
   val maximumInvokesPerMinute = math.ceil(getLimit("limits.actions.invokes.perMinute") * overhead).toInt
   val maximumFiringsPerMinute = math.ceil(getLimit("limits.triggers.fires.perMinute") * overhead).toInt
   val maximumConcurrentInvokes = getLimit("limits.actions.invokes.concurrent")


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services