-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Adjust user memory via api #4917
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,7 @@ package org.apache.openwhisk.core.containerpool | |
|
|
||
| import akka.actor.{Actor, ActorRef, ActorRefFactory, Props} | ||
| import org.apache.openwhisk.common.{Logging, LoggingMarkers, MetricEmitter, TransactionId} | ||
| import org.apache.openwhisk.core.connector.MessageFeed | ||
| import org.apache.openwhisk.core.connector.{MessageFeed, UserMemoryMessage} | ||
| import org.apache.openwhisk.core.entity.ExecManifest.ReactivePrewarmingConfig | ||
| import org.apache.openwhisk.core.entity._ | ||
| import org.apache.openwhisk.core.entity.size._ | ||
|
|
@@ -31,6 +31,8 @@ import scala.util.{Random, Try} | |
|
|
||
| case class ColdStartKey(kind: String, memory: ByteSize) | ||
|
|
||
| object UserMemoryQuery | ||
|
|
||
| case object EmitMetrics | ||
|
|
||
| case object AdjustPrewarmedContainer | ||
|
|
@@ -68,6 +70,7 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef, | |
| var busyPool = immutable.Map.empty[ActorRef, ContainerData] | ||
| var prewarmedPool = immutable.Map.empty[ActorRef, PreWarmedData] | ||
| var prewarmStartingPool = immutable.Map.empty[ActorRef, (String, ByteSize)] | ||
| var latestUserMemory = poolConfig.userMemory | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hope we can avoid using
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm.. seems the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can take advantage of |
||
| // If all memory slots are occupied and if there is currently no container to be removed, than the actions will be | ||
| // buffered here to keep order of computation. | ||
| // Otherwise actions with small memory-limits could block actions with large memory limits. | ||
|
|
@@ -209,7 +212,7 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef, | |
| s"Rescheduling Run message, too many message in the pool, " + | ||
| s"freePoolSize: ${freePool.size} containers and ${memoryConsumptionOf(freePool)} MB, " + | ||
| s"busyPoolSize: ${busyPool.size} containers and ${memoryConsumptionOf(busyPool)} MB, " + | ||
| s"maxContainersMemory ${poolConfig.userMemory.toMB} MB, " + | ||
| s"maxContainersMemory ${latestUserMemory.toMB} MB, " + | ||
| s"userNamespace: ${r.msg.user.namespace.name}, action: ${r.action}, " + | ||
| s"needed memory: ${r.action.limits.memory.megabytes} MB, " + | ||
| s"waiting messages: ${runBuffer.size}")(r.msg.transid) | ||
|
|
@@ -297,6 +300,13 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef, | |
| case RescheduleJob => | ||
| freePool = freePool - sender() | ||
| busyPool = busyPool - sender() | ||
| case userMemoryMessage: UserMemoryMessage => | ||
| logging.info( | ||
| this, | ||
| s"user memory is reconfigured from ${latestUserMemory.toString} to ${userMemoryMessage.userMemory.toString}") | ||
| latestUserMemory = userMemoryMessage.userMemory | ||
| case UserMemoryQuery => | ||
| sender() ! latestUserMemory.toString | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about sending the intact obejct directly and handle the type conversion in the InvokerServer layer?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm.. if send
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok since this is just a |
||
| case EmitMetrics => | ||
| emitMetrics() | ||
|
|
||
|
|
@@ -444,7 +454,7 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef, | |
| def hasPoolSpaceFor[A](pool: Map[A, ContainerData], | ||
| prewarmStartingPool: Map[A, (String, ByteSize)], | ||
| memory: ByteSize): Boolean = { | ||
| memoryConsumptionOf(pool) + prewarmStartingPool.map(_._2._2.toMB).sum + memory.toMB <= poolConfig.userMemory.toMB | ||
| memoryConsumptionOf(pool) + prewarmStartingPool.map(_._2._2.toMB).sum + memory.toMB <= latestUserMemory.toMB | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| /* | ||
| * 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.invoker | ||
|
|
||
| import akka.actor.ActorSystem | ||
| import akka.http.scaladsl.model.StatusCodes | ||
| import akka.http.scaladsl.model.headers.BasicHttpCredentials | ||
| import akka.http.scaladsl.server.Route | ||
| import org.apache.openwhisk.common.{Logging, TransactionId} | ||
| import org.apache.openwhisk.core.ConfigKeys | ||
| import org.apache.openwhisk.http.BasicRasService | ||
| import org.apache.openwhisk.http.ErrorResponse.terminate | ||
| import pureconfig._ | ||
| import spray.json.PrettyPrinter | ||
|
|
||
| import scala.concurrent.ExecutionContext | ||
|
|
||
| /** | ||
| * Implements web server to handle certain REST API calls. | ||
| */ | ||
| class DefaultInvokerServer(val invoker: InvokerCore)(implicit val ec: ExecutionContext, | ||
| val actorSystem: ActorSystem, | ||
| val logger: Logging) | ||
| extends BasicRasService { | ||
|
|
||
| val invokerUsername = loadConfigOrThrow[String](ConfigKeys.whiskInvokerUsername) | ||
| val invokerPassword = loadConfigOrThrow[String](ConfigKeys.whiskInvokerPassword) | ||
|
|
||
| override def routes(implicit transid: TransactionId): Route = { | ||
| super.routes ~ extractCredentials { | ||
| case Some(BasicHttpCredentials(username, password)) | ||
| if username == invokerUsername && password == invokerPassword => | ||
| (path("config" / "memory") & get) { | ||
| invoker.getUserMemory() | ||
| } | ||
| case _ => | ||
| implicit val jsonPrettyResponsePrinter = PrettyPrinter | ||
| terminate(StatusCodes.Unauthorized) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| object DefaultInvokerServer extends InvokerServerProvider { | ||
| override def instance( | ||
| invoker: InvokerCore)(implicit ec: ExecutionContext, actorSystem: ActorSystem, logger: Logging): BasicRasService = | ||
| new DefaultInvokerServer(invoker) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ package org.apache.openwhisk.core.invoker | |
|
|
||
| import akka.Done | ||
| import akka.actor.{ActorSystem, CoordinatedShutdown} | ||
| import akka.http.scaladsl.server.Route | ||
| import akka.stream.ActorMaterializer | ||
| import com.typesafe.config.ConfigValueFactory | ||
| import kamon.Kamon | ||
|
|
@@ -217,7 +218,9 @@ trait InvokerProvider extends Spi { | |
| } | ||
|
|
||
| // this trait can be used to add common implementation | ||
| trait InvokerCore {} | ||
| trait InvokerCore { | ||
| def getUserMemory(): Route | ||
| } | ||
|
|
||
| /** | ||
| * An Spi for providing RestAPI implementation for invoker. | ||
|
|
@@ -227,9 +230,3 @@ trait InvokerServerProvider extends Spi { | |
| def instance( | ||
| invoker: InvokerCore)(implicit ec: ExecutionContext, actorSystem: ActorSystem, logger: Logging): BasicRasService | ||
| } | ||
|
|
||
| object DefaultInvokerServer extends InvokerServerProvider { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this removed?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved it to core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/DefaultInvokerServer.scala |
||
| override def instance( | ||
| invoker: InvokerCore)(implicit ec: ExecutionContext, actorSystem: ActorSystem, logger: Logging): BasicRasService = | ||
| new BasicRasService {} | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we directly convert this to
ConfigMemoryList?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, updated.