Merge pull request #461 from bmpotter/timeouts-and-msgs

Issues: 455, 456, 458 - Timeouts and msgs

Makefile

@@ -81,15 +81,15 @@ docker: .docker-exec
 	sbt docker:publishLocal
 	@touch $@
 
-# config.json is renamed to exchange-api.tmpl to overwrite the provided file of the same name in the Docker image. Prevents the container from attempting to overwrite a bind-mounted config.json with read-only permissions.
+# config.json is mounted into the container as exchange-api.tmpl to overwrite the provided file of the same name in the Docker image. Prevents the container from attempting to overwrite a bind-mounted config.json with read-only permissions.
 .docker-exec-run: .docker-exec .docker-network
 	@if [[ ! -f "$(EXCHANGE_HOST_KEYSTORE_DIR)/keystore" || ! -f "$(EXCHANGE_HOST_KEYSTORE_DIR)/keypassword" ]]; then echo "Error: keystore and keypassword do not exist in $(EXCHANGE_HOST_KEYSTORE_DIR). You must first copy them there or run 'make gen-key'"; false; fi
 	- docker rm -f $(DOCKER_NAME) 2> /dev/null || :
 	docker run --name $(DOCKER_NAME) --network $(DOCKER_NETWORK) -d -t -p $(EXCHANGE_API_PORT):$(EXCHANGE_API_PORT) -p $(EXCHANGE_API_HTTPS_PORT):$(EXCHANGE_API_HTTPS_PORT) -e "JAVA_OPTS=$(JAVA_OPTS)" -e "ICP_EXTERNAL_MGMT_INGRESS=$$ICP_EXTERNAL_MGMT_INGRESS" -v $(EXCHANGE_HOST_CONFIG_DIR)/config.json:$(EXCHANGE_CONFIG_DIR)/exchange-api.tmpl:ro -v $(EXCHANGE_HOST_ICP_CERT_FILE):$(EXCHANGE_ICP_CERT_FILE) -v $(EXCHANGE_HOST_KEYSTORE_DIR):$(EXCHANGE_CONTAINER_KEYSTORE_DIR):ro -v $(EXCHANGE_HOST_POSTGRES_CERT_FILE):$(EXCHANGE_CONTAINER_POSTGRES_CERT_FILE) $(image-string):$(DOCKER_TAG)
 	@touch $@
 
 # Note: this target is used by travis as part of testing
-# config.json is renamed to exchange-api.tmpl to overwrite the provided file of the same name in the Docker image. Prevents the container from attempting to overwrite a bind-mounted config.json with read-only permissions.
+# config.json is mounted into the container as exchange-api.tmpl to overwrite the provided file of the same name in the Docker image. Prevents the container from attempting to overwrite a bind-mounted config.json with read-only permissions.
 .docker-exec-run-no-https: .docker-exec .docker-network
 	- docker rm -f $(DOCKER_NAME) 2> /dev/null || :
 	docker run --name $(DOCKER_NAME) --network $(DOCKER_NETWORK) -d -t -p $(EXCHANGE_API_PORT):$(EXCHANGE_API_PORT) -e "JAVA_OPTS=$(JAVA_OPTS)" -v $(EXCHANGE_HOST_CONFIG_DIR)/config.json:$(EXCHANGE_CONFIG_DIR)/exchange-api.tmpl $(image-string):$(DOCKER_TAG)

README.md

@@ -256,6 +256,12 @@ Now you can disable root by setting `api.root.enabled` to `false` in `/etc/horiz
     - detect if a pattern is updated with service that has userInput w/o default values, and give warning
     - Consider changing all creates to POST, and update (via put/patch) return codes to 200
 
+## Changes in 2.60.0
+
+- Issue 456: Change config value `akka.http.server.request-timeout` to `45s`
+- Issue 455: Avoid DB reads if `maxMessagesInMailbox` and `maxAgreements` are `0` which is default and means unlimited
+- Issue 458: Lower default values of resourceChanges ttl and cleanupInterval
+
 ## Changes in 2.59.0
 
 - Issue 429: add `noheartbeat=true` option to APIs `PUT /orgs/{orgid}/nodes/{nodeid}` and `PUT /orgs/{orgid}/nodes/{nodeid}/agreements/<agreement-id>`

build.sbt

@@ -138,7 +138,7 @@ lazy val root = (project in file("."))
                                         Cmd("USER", "1001:1001"), 
                                         /*
                                          * If bind-mounting your own config.json rename the configuration file in the container's filesystem to exchange-api.tmpl. This will overwrite the 
-                                         * provided exchange-api.tmpl provided in this docker image and prevent cases where a bind-mount config.json is set with read-only permissions. 
+                                         * exchange-api.tmpl provided in this docker image and prevent cases where a bind-mount config.json is set with read-only permissions. 
                                          * Any mounted config.json can choose to use variables to take advantage of the substitution below.
                                          */
                                         Cmd("ENTRYPOINT", "/usr/bin/envsubst $ENVSUBST_CONFIG < /etc/horizon/exchange/exchange-api.tmpl > /etc/horizon/exchange/config.json && /opt/docker/bin/" ++ name.value),

src/main/resources/config.json

@@ -34,9 +34,9 @@
 			"type": "guava"    // currently guava is the only option
 		},
 		"resourceChanges": {
-			"ttl": 604800,    // number of seconds to keep the history records of resource changes - 604800 is 1 week
+			"ttl": 14400,    // number of seconds to keep the history records of resource changes (14400 is 4 hours). When agents miss 1 or more heartbeats, they reset querying the /changes route, so they do not need very old entries.
 			"maxRecordsCap": 10000, // maximum number of records the notification framework route will return
-			"cleanupInterval": 86400	 // number of seconds between pruning the resourcechanges table in the db of expired changes - 86400 is 24 hours
+			"cleanupInterval": 3600	 // number of seconds between pruning the resourcechanges table in the db of expired changes - 3600 is 1 hour
 		},
 		"db": {
       "acquireIncrement": 1,
@@ -79,7 +79,8 @@
 			"Agbot": ["READ_MYSELF", "WRITE_MYSELF", "DATA_HEARTBEAT_MY_AGBOTS", "READ_MY_AGBOTS", "READ_ALL_AGBOTS", "READ_ALL_NODES", "SEND_MSG_TO_NODE", "READ_ALL_SERVICES", "READ_ALL_PATTERNS", "READ_ALL_BUSINESS", "READ_MY_ORG", "MAXCHANGEID"]
 		},
 		"akka": {
-			// add entries like: "akka.http.server.max-connections": 512
+			"akka.http.server.request-timeout": "45s"
+			// add additional entries like: "akka.http.server.max-connections": 512
 			// see: https://doc.akka.io/docs/akka-http/current/configuration.html
 			// and see: https://doc.akka.io/docs/akka/current/general/configuration-reference.html
 			// can use this to see all of the akka config: "akka.log-config-on-start": "on"

src/main/resources/version.txt

@@ -1 +1 @@
-2.59.0
+2.60.0

src/main/scala/com/horizon/exchangeapi/AgbotsRoutes.scala

@@ -1291,13 +1291,13 @@ trait AgbotsRoutes extends JacksonSupport with AuthenticationSupport {
     exchAuth(TAgbot(compositeId),Access.WRITE) { _ =>
       validateWithMsg(reqBody.getAnyProblem) {
         complete({
-          db.run(AgbotAgreementsTQ.getNumOwned(compositeId).result.flatMap({ xs =>
-            logger.debug("PUT /orgs/"+orgid+"/agbots/"+id+"/agreements/"+agrId+" num owned: "+xs)
+          val maxAgreements = ExchConfig.getInt("api.limits.maxAgreements")
+          val getNumOwnedDbio = if (maxAgreements == 0) DBIO.successful(0) else AgbotAgreementsTQ.getNumOwned(compositeId).result // avoid DB read for this if there is no max
+          db.run(getNumOwnedDbio.flatMap({ xs =>
+            if (maxAgreements != 0) logger.debug("PUT /orgs/"+orgid+"/agbots/"+id+"/agreements/"+agrId+" num owned: "+xs)
             val numOwned = xs
-            val maxAgreements = ExchConfig.getInt("api.limits.maxAgreements")
-            if (maxAgreements == 0 || numOwned <= maxAgreements) {    // we are not sure if this is create or update, but if they are already over the limit, stop them anyway
-              reqBody.toAgbotAgreementRow(compositeId, agrId).upsert.asTry
-            }
+            // we are not sure if this is create or update, but if they are already over the limit, stop them anyway
+            if (maxAgreements == 0 || numOwned <= maxAgreements) reqBody.toAgbotAgreementRow(compositeId, agrId).upsert.asTry
             else DBIO.failed(new DBProcessingError(HttpCode.ACCESS_DENIED, ApiRespType.ACCESS_DENIED, ExchMsg.translate("over.max.limit.of.agreements", maxAgreements) )).asTry
           }).flatMap({
             case Success(v) =>
@@ -1575,11 +1575,12 @@ trait AgbotsRoutes extends JacksonSupport with AuthenticationSupport {
       complete({
         val nodeId = ident.creds.id      //somday: handle the case where the acls allow users to send msgs
         var msgNum = ""
+        val maxMessagesInMailbox = ExchConfig.getInt("api.limits.maxMessagesInMailbox")
+        val getNumOwnedDbio = if (maxMessagesInMailbox == 0) DBIO.successful(0) else AgbotMsgsTQ.getNumOwned(compositeId).result // avoid DB read for this if there is no max
         // Remove msgs whose TTL is past, then check the mailbox is not full, then get the node publicKey, then write the agbotmsgs row, all in the same db.run thread
-        db.run(AgbotMsgsTQ.getNumOwned(compositeId).result.flatMap({ xs =>
-          logger.debug("POST /orgs/"+orgid+"/agbots/"+id+"/msgs mailbox size: "+xs)
+        db.run(getNumOwnedDbio.flatMap({ xs =>
+          if (maxMessagesInMailbox != 0) logger.debug("POST /orgs/"+orgid+"/agbots/"+id+"/msgs mailbox size: "+xs)
           val mailboxSize = xs
-          val maxMessagesInMailbox = ExchConfig.getInt("api.limits.maxMessagesInMailbox")
           if (maxMessagesInMailbox == 0 || mailboxSize < maxMessagesInMailbox) NodesTQ.getPublicKey(nodeId).result.asTry
           else DBIO.failed(new DBProcessingError(HttpCode.BAD_GW, ApiRespType.BAD_GW, ExchMsg.translate("agbot.mailbox.full", compositeId, maxMessagesInMailbox) )).asTry
         }).flatMap({

src/main/scala/com/horizon/exchangeapi/NodesRoutes.scala

@@ -1971,13 +1971,13 @@ trait NodesRoutes extends JacksonSupport with AuthenticationSupport {
       validateWithMsg(reqBody.getAnyProblem) {
         complete({
           val noHB = if (noheartbeat.isEmpty) false else if (noheartbeat.get.toLowerCase == "true") true else false
-          db.run(NodeAgreementsTQ.getNumOwned(compositeId).result.flatMap({ xs =>
-            logger.debug("PUT /orgs/"+orgid+"/nodes/"+id+"/agreements/"+agrId+" num owned: "+xs)
+          val maxAgreements: Int = ExchConfig.getInt("api.limits.maxAgreements")
+          val getNumOwnedDbio = if (maxAgreements == 0) DBIO.successful(0) else NodeAgreementsTQ.getNumOwned(compositeId).result // avoid DB read for this if there is no max
+          db.run(getNumOwnedDbio.flatMap({ xs =>
+            if (maxAgreements != 0) logger.debug("PUT /orgs/"+orgid+"/nodes/"+id+"/agreements/"+agrId+" num owned: "+xs)
             val numOwned: Int = xs
-            val maxAgreements: Int = ExchConfig.getInt("api.limits.maxAgreements")
-            if (maxAgreements == 0 || numOwned <= maxAgreements) {    // we are not sure if this is create or update, but if they are already over the limit, stop them anyway
-              reqBody.toNodeAgreementRow(compositeId, agrId).upsert.asTry
-            }
+            // we are not sure if this is create or update, but if they are already over the limit, stop them anyway
+            if (maxAgreements == 0 || numOwned <= maxAgreements) reqBody.toNodeAgreementRow(compositeId, agrId).upsert.asTry
             else DBIO.failed(new DBProcessingError(HttpCode.ACCESS_DENIED, ApiRespType.ACCESS_DENIED, ExchMsg.translate("over.limit.of.agreements.for.node", maxAgreements) )).asTry
           }).flatMap({
             case Success(v) =>
@@ -2185,11 +2185,12 @@ trait NodesRoutes extends JacksonSupport with AuthenticationSupport {
       complete({
         val agbotId: String = ident.creds.id      //someday: handle the case where the acls allow users to send msgs
         var msgNum = ""
+        val maxMessagesInMailbox: Int = ExchConfig.getInt("api.limits.maxMessagesInMailbox")
+        val getNumOwnedDbio = if (maxMessagesInMailbox == 0) DBIO.successful(0) else NodeMsgsTQ.getNumOwned(compositeId).result // avoid DB read for this if there is no max
         // Remove msgs whose TTL is past, then check the mailbox is not full, then get the agbot publicKey, then write the nodemsgs row, all in the same db.run thread
-        db.run(NodeMsgsTQ.getNumOwned(compositeId).result.flatMap({ xs =>
-          logger.debug("POST /orgs/"+orgid+"/nodes/"+id+"/msgs mailbox size: "+xs)
+        db.run(getNumOwnedDbio.flatMap({ xs =>
+          if (maxMessagesInMailbox != 0) logger.debug("POST /orgs/"+orgid+"/nodes/"+id+"/msgs mailbox size: "+xs)
           val mailboxSize: Int = xs
-          val maxMessagesInMailbox: Int = ExchConfig.getInt("api.limits.maxMessagesInMailbox")
           if (maxMessagesInMailbox == 0 || mailboxSize < maxMessagesInMailbox) AgbotsTQ.getPublicKey(agbotId).result.asTry
           else DBIO.failed(new DBProcessingError(HttpCode.BAD_GW, ApiRespType.BAD_GW, ExchMsg.translate("node.mailbox.full", compositeId, maxMessagesInMailbox) )).asTry
         }).flatMap({