Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #15775: Performance problems with ZIO #2529

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,11 @@ object JsDirectiveParamLogger extends Logger {
override protected def _logger = LoggerFactory.getLogger("jsDirectiveParam")
}

object JsDirectiveParamLoggerPure extends NamedZioLogger {
def loggerName = "jsDirectiveParam"
}


object GenerationLoggerPure extends NamedZioLogger {
def loggerName = "policy-generation"
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

package com.normation.rudder.domain.logger

import com.normation.NamedZioLogger
import org.slf4j.LoggerFactory
import net.liftweb.common.Logger

Expand All @@ -45,6 +46,22 @@ import net.liftweb.common.Logger
*/
object TimingDebugLogger extends Logger {
override protected def _logger = LoggerFactory.getLogger("debug_timing")
object PolicyGeneration extends Logger {
override protected def _logger = LoggerFactory.getLogger("debug_timing.generation")
object BuildNodeConfig extends Logger {
override protected def _logger = LoggerFactory.getLogger("debug_timing.generation.buildNodeConfig")
}
}
}

object TimingDebugLoggerPure extends NamedZioLogger {
override def loggerName: String = "debug_timing"
object PolicyGeneration extends NamedZioLogger {
override def loggerName: String = "debug_timing.generation"
object BuildNodeConfig extends NamedZioLogger {
override def loggerName: String = "debug_timing.generation.buildNodeConfig"
}
}
}


Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import com.normation.cfclerk.domain.Variable
import com.normation.rudder.domain.nodes.NodeInfo
import com.normation.rudder.domain.parameters.ParameterName
import com.normation.rudder.domain.policies.{DirectiveId, GlobalPolicyMode, PolicyMode, RuleId}
import net.liftweb.common.Box

import scala.collection.immutable.TreeMap
import org.joda.time.DateTime
Expand All @@ -64,6 +63,8 @@ import com.normation.cfclerk.domain.AgentConfig
import com.normation.cfclerk.domain.TechniqueGenerationMode
import com.normation.cfclerk.domain.TechniqueVersion

import com.normation.errors._

/*
* This file contains all the specific data structures used during policy generation.
* It introduce a set of new concepts:
Expand Down Expand Up @@ -134,7 +135,7 @@ final case class InterpolationContext(
, nodeContext : TreeMap[String, Variable]
// parameters for this node
//must be a case SENSITIVE Map !!!!
, parameters : Map[ParameterName, InterpolationContext => Box[String]]
, parameters : Map[ParameterName, InterpolationContext => IOResult[String]]
//the depth of the interpolation context evaluation
//used as a lazy, trivial, mostly broken way to detect cycle in interpretation
//for ex: param a => param b => param c => ..... => param a
Expand All @@ -156,7 +157,7 @@ object InterpolationContext {
, nodeContext : Map[String, Variable]
// parameters for this node
//must be a case SENSITIVE Map !!!!
, parameters : Map[ParameterName, InterpolationContext => Box[String]]
, parameters : Map[ParameterName, InterpolationContext => IOResult[String]]
//the depth of the interpolation context evaluation
//used as a lazy, trivial, mostly broken way to detect cycle in interpretation
//for ex: param a => param b => param c => ..... => param a
Expand Down Expand Up @@ -440,7 +441,7 @@ final case class ParsedPolicyDraft(
, isSystem : Boolean
, policyMode : Option[PolicyMode]
, trackerVariable : TrackerVariable
, variables : InterpolationContext => Box[Map[String, Variable]]
, variables : InterpolationContext => IOResult[Map[String, Variable]]
, originalVariables: Map[String, Variable] // the original variable, unexpanded
, ruleOrder : BundleOrder
, directiveOrder : BundleOrder
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ import com.normation.rudder.domain.parameters.ParameterName
import net.liftweb.json.JsonAST.JValue
import com.normation.inventory.domain.AgentType
import com.normation.rudder.domain.policies.PolicyModeOverrides
import com.normation.errors._
import zio._
import zio.syntax._

/**
* A parser that handle parameterized value of
Expand Down Expand Up @@ -125,7 +128,7 @@ trait InterpolatedValueCompiler {
* Return a Box, where Full denotes a successful
* parsing of all values, and EmptyBox. an error.
*/
def compile(value: String): Box[InterpolationContext => Box[String]]
def compile(value: String): IOResult[InterpolationContext => IOResult[String]]

/**
*
Expand Down Expand Up @@ -201,10 +204,10 @@ class InterpolatedValueCompilerImpl extends RegexParsers with InterpolatedValueC
* just call the parser on a value, and in case of successful parsing, interprete
* the resulting AST (seq of token)
*/
def compile(value: String): Box[InterpolationContext => Box[String]] = {
override def compile(value: String): IOResult[InterpolationContext => IOResult[String]] = {
parseAll(all, value) match {
case NoSuccess(msg, remaining) => FailedBox(s"""Error when parsing value "${value}", error message is: ${msg}""")
case Success(tokens, remaining) => Full(parseToken(tokens))
case NoSuccess(msg, remaining) => Unexpected(s"""Error when parsing value "${value}", error message is: ${msg}""").fail
case Success(tokens, remaining) => parseToken(tokens).succeed
}
}

Expand All @@ -219,15 +222,11 @@ class InterpolatedValueCompilerImpl extends RegexParsers with InterpolatedValueC
* The funny part that for each token add the interpretation of the token
* by composing interpretation function.
*/
def parseToken(tokens:List[Token]): InterpolationContext => Box[String] = {
def parseToken(tokens:List[Token]): InterpolationContext => IOResult[String] = {
def build(context: InterpolationContext) = {
val init: Box[String] = Full("")
tokens.foldLeft(init){
case (eb:EmptyBox, _ ) => eb
case (Full(str), token) => analyse(context, token) match {
case eb:EmptyBox => eb
case Full(s) => Full(str + s)
}
val init = ""
ZIO.foldLeft(tokens)(init) {
(str, token) => analyse(context, token).map(s => (str + s))
}
}

Expand All @@ -240,25 +239,26 @@ class InterpolatedValueCompilerImpl extends RegexParsers with InterpolatedValueC
* the final string (that may not succeed at run time, because of
* unknown parameter, etc)
*/
def analyse(context: InterpolationContext, token:Token): Box[String] = {
def analyse(context: InterpolationContext, token:Token): IOResult[String] = {
token match {
case CharSeq(s) => Full(s)
case CharSeq(s) => s.succeed
case NodeAccessor(path) => getNodeAccessorTarget(context, path)
case Param(name) => getRudderGlobalParam(context, ParameterName(name))
case Property(path, opt) => opt match {
case None =>
getNodeProperty(context, path)
case Some(InterpreteOnNode) =>
//in that case, we want to exactly output the agent-compatible string. For now, easy, only one string
Full("${node.properties[" + path.mkString("][") + "]}")
("${node.properties[" + path.mkString("][") + "]}").succeed
case Some(DefaultValue(optionTokens)) =>
//in that case, we want to find the default value.
//we authorize to have default value = ${node.properties[bla][bla][bla]|node},
//because we may want to use prop1 and if node set, prop2 at run time.
for {
default <- parseToken(optionTokens)(context)
prop <- getNodeProperty(context, path).foldM(_ => default.succeed, s => s.succeed)
} yield {
getNodeProperty(context, path).openOr(default)
prop
}
}
}
Expand All @@ -282,41 +282,41 @@ class InterpolatedValueCompilerImpl extends RegexParsers with InterpolatedValueC
/**
* Retrieve the global parameter from the node context.
*/
def getRudderGlobalParam(context: InterpolationContext, paramName: ParameterName): Box[String] = {
def getRudderGlobalParam(context: InterpolationContext, paramName: ParameterName): IOResult[String] = {
context.parameters.get(paramName) match {
case Some(value) =>
if(context.depth >= maxEvaluationDepth) {
FailedBox(s"""Can not evaluted global parameter "${paramName.value}" because it uses an interpolation variable that depends upon """
+ s"""other interpolated variables in a stack more than ${maxEvaluationDepth} in depth. We fear it's a circular dependancy.""")
Unexpected(s"""Can not evaluted global parameter "${paramName.value}" because it uses an interpolation variable that depends upon """
+ s"""other interpolated variables in a stack more than ${maxEvaluationDepth} in depth. We fear it's a circular dependancy.""").fail
} else value(context.copy(depth = context.depth+1))
case _ => FailedBox(s"Error when trying to interpolate a variable: Rudder parameter not found: '${paramName.value}'")
case _ => Unexpected(s"Error when trying to interpolate a variable: Rudder parameter not found: '${paramName.value}'").fail
}
}

/**
* Get the targeted accessed node information, checking that it exists.
*/
def getNodeAccessorTarget(context: InterpolationContext, path: List[String]): Box[String] = {
val error = FailedBox(s"Unknow interpolated variable $${node.${path.mkString(".")}}" )
def getNodeAccessorTarget(context: InterpolationContext, path: List[String]): IOResult[String] = {
val error = Unexpected(s"Unknow interpolated variable $${node.${path.mkString(".")}}" ).fail
path match {
case Nil => FailedBox("In node interpolated variable, at least one accessor must be provided")
case Nil => Unexpected("In node interpolated variable, at least one accessor must be provided").fail
case access :: tail => access.toLowerCase :: tail match {
case "id" :: Nil => Full(context.nodeInfo.id.value)
case "hostname" :: Nil => Full(context.nodeInfo.hostname)
case "admin" :: Nil => Full(context.nodeInfo.localAdministratorAccountName)
case "state" :: Nil => Full(context.nodeInfo.state.name)
case "id" :: Nil => context.nodeInfo.id.value.succeed
case "hostname" :: Nil => context.nodeInfo.hostname.succeed
case "admin" :: Nil => context.nodeInfo.localAdministratorAccountName.succeed
case "state" :: Nil => context.nodeInfo.state.name.succeed
case "policymode" :: Nil =>
val effectivePolicyMode = context.globalPolicyMode.overridable match {
case PolicyModeOverrides.Unoverridable =>
context.globalPolicyMode.mode.name
case PolicyModeOverrides.Always =>
context.nodeInfo.policyMode.getOrElse(context.globalPolicyMode.mode).name
}
Full(effectivePolicyMode)
effectivePolicyMode.succeed
case "policyserver" :: tail2 => tail2 match {
case "id" :: Nil => Full(context.policyServerInfo.id.value)
case "hostname" :: Nil => Full(context.policyServerInfo.hostname)
case "admin" :: Nil => Full(context.policyServerInfo.localAdministratorAccountName)
case "id" :: Nil => context.policyServerInfo.id.value.succeed
case "hostname" :: Nil => context.policyServerInfo.hostname.succeed
case "admin" :: Nil => context.policyServerInfo.localAdministratorAccountName.succeed
case _ => error
}
case seq => error
Expand All @@ -331,23 +331,23 @@ class InterpolatedValueCompilerImpl extends RegexParsers with InterpolatedValueC
* If the path length is more than one, try to parse the string has a
* json value and access the remaining part as a json path.
*/
def getNodeProperty(context: InterpolationContext, path: List[String]): Box[String] = {
def getNodeProperty(context: InterpolationContext, path: List[String]): IOResult[String] = {
val errmsg = s"Missing property '$${node.properties[${path.mkString("][")}]}' on node '${context.nodeInfo.hostname}' [${context.nodeInfo.id.value}]"
path match {
//we should not reach that case since we enforce at leat one match of [...] in the parser
case Nil => FailedBox(s"The syntax $${node.properties} is invalid, only $${node.properties[propertyname]} is accepted")
case Nil => Unexpected(s"The syntax $${node.properties} is invalid, only $${node.properties[propertyname]} is accepted").fail
case h :: tail => context.nodeInfo.properties.find(p => p.name == h) match {
case None => FailedBox(errmsg)
case None => Unexpected(errmsg).fail
case Some(prop) => tail match {
case Nil => Full(prop.renderValue)
case Nil => prop.renderValue.succeed
//here, we need to parse the value in json and try to find the asked path
case subpath => getJsonProperty(subpath, prop.value) ?~! errmsg
case subpath => getJsonProperty(subpath, prop.value).chainError(errmsg)
}
}
}
}

def getJsonProperty(path: List[String], json: JValue): Box[String] = {
def getJsonProperty(path: List[String], json: JValue): IOResult[String] = {
import net.liftweb.json._

@scala.annotation.tailrec
Expand All @@ -358,9 +358,9 @@ class InterpolatedValueCompilerImpl extends RegexParsers with InterpolatedValueC

for {
prop <- access(json, path) match {
case JNothing => FailedBox(s"Can not find property in JSON '${compactRender(json)}'")
case JString(s) => Full(s) //needed to special case to not have '\"' everywhere
case x => Full(compactRender(x))
case JNothing => Unexpected(s"Can not find property in JSON '${compactRender(json)}'").fail
case JString(s) => s.succeed //needed to special case to not have '\"' everywhere
case x => compactRender(x).succeed
}
} yield {
prop
Expand Down
Loading