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 #15675: Leak in Cache of Node Compliance and NodeInfo and perfs improvement #2648

Closed
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
3af575c
Fixes #15675: Leak in Cache of Node Compliance and NodeInfo and perfs…
ncharles Dec 3, 2019
e6aeb1a
fixup! fixes #15675: fix memory usage
ncharles Dec 3, 2019
f80eee0
fixup! fixup! fixes #15675: fix memory usage
ncharles Dec 3, 2019
32ecee9
fixup
ncharles Dec 9, 2019
b5e4710
fixup
ncharles Dec 9, 2019
9b35e71
Try to prevent locking in yourkit
ncharles Dec 13, 2019
9598aaa
memory usage computation
ncharles Dec 13, 2019
38611b3
Revert "memory usage computation"
ncharles Dec 13, 2019
b530809
adding memory usage computation
ncharles Dec 14, 2019
f70ce20
allow instrumentaton
ncharles Dec 15, 2019
3ed9ec0
exclude jar to allow instrumentation to load
ncharles Dec 15, 2019
87c09cf
adding measure pour nodecontexts
ncharles Dec 16, 2019
78cdb29
originalVars is unused
ncharles Dec 16, 2019
74e664f
measuring policies
ncharles Dec 16, 2019
312da22
test wrapping inside for yield
ncharles Dec 18, 2019
1f92658
typo
ncharles Dec 18, 2019
16eafe2
remove unessary call to trim
ncharles Dec 18, 2019
18af296
more for yield, it's like violence
ncharles Dec 20, 2019
c56c638
lower number of created objects + fasten compliance computation
ncharles Dec 20, 2019
9bce1c0
try to help hotpot by adding all variables at once
ncharles Dec 22, 2019
9e2c74e
try to help hotpot by adding all variables at once - edge case
ncharles Dec 22, 2019
e916e3e
for yield, for yield for everyone, for you, for them
ncharles Dec 22, 2019
962c7a0
convert val to def to save memory and measure impact
ncharles Dec 30, 2019
78eab1e
randomize templates to write
ncharles Dec 31, 2019
36b0eea
adding timing info
ncharles Jan 1, 2020
e3001fa
aggregate timings
ncharles Jan 1, 2020
535e32e
improve timing
ncharles Jan 1, 2020
2001015
fix type
ncharles Jan 1, 2020
97c9c0b
fix type2
ncharles Jan 1, 2020
fbfcc9c
fix type2
ncharles Jan 1, 2020
355fbeb
fix compilation
ncharles Jan 4, 2020
9176763
remove some mapValues
ncharles Jan 6, 2020
7fe1bc6
Revert "remove some mapValues"
ncharles Jan 9, 2020
a0c1757
removing extra map of nodeconfiguration in the hope to save memory
ncharles Jan 9, 2020
5a9d3ea
attempt to lower pressure on memory usage
ncharles Jan 11, 2020
6225312
fix type
ncharles Jan 11, 2020
e680f67
fix type
ncharles Jan 11, 2020
33f4ca6
clean some usage
ncharles Jan 12, 2020
96ceafb
correct type
ncharles Jan 12, 2020
a93b67e
test to remove spike
ncharles Jan 12, 2020
236a035
try to lower memory pressur whil fetching reports
ncharles Jan 13, 2020
9ff671a
fix type
ncharles Jan 13, 2020
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
9 changes: 8 additions & 1 deletion webapp/sources/rudder/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,14 @@ along with Rudder. If not, see <http://www.gnu.org/licenses/>.
<!-- fill-in rule templates -->
</modules>

<dependencies/>
<dependencies>
<dependency>
<groupId>com.github.jbellis</groupId>
<artifactId>jamm</artifactId>
<version>0.3.3</version>
<scope>provided</scope>
</dependency>
</dependencies>

<repositories>
<repository>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class CachedReportsExecutionRepository(
cache = Map()
}

override def getNodesLastRun(nodeIds: Set[NodeId]): Box[Map[NodeId, Option[AgentRunWithNodeConfig]]] = this.synchronized {
override def getNodesLastRun(nodeIds: Set[NodeId]): Box[Map[NodeId, Option[AgentRunWithNodeConfig]]] = scala.concurrent.blocking { this.synchronized {
val n1 = System.currentTimeMillis
(for {
runs <- readBackend.getNodesLastRun(nodeIds.diff(cache.keySet))
Expand All @@ -127,7 +127,7 @@ class CachedReportsExecutionRepository(
cache = cache ++ runs
cache.filterKeys { x => nodeIds.contains(x) }
}) ?~! s"Error when trying to update the cache of Agent Runs informations"
}
} }

override def updateExecutions(executions : Seq[AgentRun]) : Seq[Box[AgentRun]] = this.synchronized {
logger.trace(s"Update runs for nodes [${executions.map( _.agentRunId.nodeId.value ).mkString(", ")}]")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ final case class Policy(
// == .toList (keep order) ==> List[List[Variable]]
// == flatten (keep order) ==> List[Variable]
val expandedVars = Policy.mergeVars(policyVars.map( _.expandedVars.values).toList.flatten)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm really tempted to make a def out of this one, as it is used only in two different place

val originalVars = Policy.mergeVars(policyVars.map( _.originalVars.values).toList.flatten)
//val originalVars = Policy.mergeVars(policyVars.map( _.originalVars.values).toList.flatten)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one seems to have a lot of impact
Size deepof nodeConfigs is 2 668 075 072
Size deepof ruleVals is 376 459 952

vs
Size deepof nodeConfigs is 3 474 356 896
Size deepof ruleVals is 748 776 080

but we need to validate as there is a lot of variability in ruleval (from 748 487 288 to 2 123 635 544)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nodeconfig is stable at about 2 668 075 048 with this line vs 3 474 356 896 without this line

val trackerVariable = policyVars.head.trackerVariable.spec.cloneSetMultivalued.toVariable(policyVars.map(_.trackerVariable.values).toList.flatten)
}

Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ trait NodeConfigurationLogger {

def log(nodeConfiguration: Seq[NodeConfiguration]): Box[Set[NodeId]]

def isDebugEnabled: Boolean

}

/**
Expand All @@ -70,6 +72,8 @@ class NodeConfigurationLoggerImpl(

val logger = LoggerFactory.getLogger("rudder.debug.nodeconfiguration")

def isDebugEnabled: Boolean = logger.isDebugEnabled

{
val p = new File(path)
p.mkdirs()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import com.normation.inventory.domain.AgentType.CfeEnterprise
import com.normation.inventory.domain.NodeId
import com.normation.rudder.domain.Constants
import com.normation.rudder.domain.licenses.CfeEnterpriseLicense
import com.normation.rudder.domain.nodes.NodeProperty
import com.normation.rudder.domain.nodes.{NodeInfo, NodeProperty}
import com.normation.rudder.domain.policies.GlobalPolicyMode
import com.normation.rudder.domain.reports.NodeConfigId
import com.normation.rudder.hooks.HookEnvPairs
Expand Down Expand Up @@ -97,15 +97,16 @@ trait PolicyWriterService {
*
*/
def writeTemplate(
rootNodeId : NodeId
, nodesToWrite : Set[NodeId]
, allNodeConfigs: Map[NodeId, NodeConfiguration]
, versions : Map[NodeId, NodeConfigId]
, allLicenses : Map[NodeId, CfeEnterpriseLicense]
, globalPolicyMode: GlobalPolicyMode
, generationTime : DateTime
, parallelism : Parallelism
) : Box[Seq[NodeConfiguration]]
rootNodeId : NodeId
, nodesToWrite : Set[NodeId]
, updatedNodeConfigs: Map[NodeId, NodeConfiguration]
, allNodeInfos : Map[NodeId, NodeInfo]
, versions : Map[NodeId, NodeConfigId]
, allLicenses : Map[NodeId, CfeEnterpriseLicense]
, globalPolicyMode : GlobalPolicyMode
, generationTime : DateTime
, parallelism : Parallelism
) : Box[Seq[NodeId]]
}

class PolicyWriterServiceImpl(
Expand Down Expand Up @@ -147,7 +148,7 @@ class PolicyWriterServiceImpl(
val message = s"could not write ${fileName} file, cause is: ${e.getMessage}"
Failure(message)
case Success(_) =>
Full(agentNodeConfig)
Full(agentNodeConfig.config.nodeInfo.id)
}
}

Expand All @@ -170,7 +171,7 @@ class PolicyWriterServiceImpl(
val message = s"could not write ${fileName} file, cause is: ${e.getMessage}"
Failure(message)
case Success(_) =>
Full(agentNodeConfig)
Full(agentNodeConfig.config.nodeInfo.id)
}
}

Expand Down Expand Up @@ -212,29 +213,31 @@ class PolicyWriterServiceImpl(
*
*/
override def writeTemplate(
rootNodeId : NodeId
, nodesToWrite : Set[NodeId]
, allNodeConfigs : Map[NodeId, NodeConfiguration]
, versions : Map[NodeId, NodeConfigId]
, allLicenses : Map[NodeId, CfeEnterpriseLicense]
, globalPolicyMode: GlobalPolicyMode
, generationTime : DateTime
, parallelism : Parallelism
) : Box[Seq[NodeConfiguration]] = {

val nodeConfigsToWrite = allNodeConfigs.filterKeys(nodesToWrite.contains(_))
val interestingNodeConfigs = allNodeConfigs.filterKeys(k => nodeConfigsToWrite.exists{ case(x, _) => x == k }).values.toSeq
val techniqueIds = interestingNodeConfigs.flatMap( _.getTechniqueIds ).toSet

//debug - but don't fails for debugging !
logNodeConfig.log(nodeConfigsToWrite.values.toSeq) match {
case eb:EmptyBox =>
val e = eb ?~! "Error when trying to write node configurations for debugging"
logger.error(e)
e.rootExceptionCause.foreach { ex =>
logger.error("Root exception cause was:", ex)
rootNodeId : NodeId
, nodesToWrite : Set[NodeId]
, updatedNodeConfigs: Map[NodeId, NodeConfiguration]
, allNodeInfos : Map[NodeId, NodeInfo]
, versions : Map[NodeId, NodeConfigId]
, allLicenses : Map[NodeId, CfeEnterpriseLicense]
, globalPolicyMode : GlobalPolicyMode
, generationTime : DateTime
, parallelism : Parallelism
) : Box[Seq[NodeId]] = {

val interestingNodeConfigs = updatedNodeConfigs.values.toSeq
val techniqueIds = interestingNodeConfigs.flatMap(_.getTechniqueIds).toSet

if (logNodeConfig.isDebugEnabled) {
//debug - but don't fails for debugging !
logNodeConfig.log(interestingNodeConfigs) match {
case eb: EmptyBox =>
val e = eb ?~! "Error when trying to write node configurations for debugging"
logger.error(e)
e.rootExceptionCause.foreach { ex =>
logger.error("Root exception cause was:", ex)
}
case _ => //nothing to do
}
case _ => //nothing to do
}

/*
Expand Down Expand Up @@ -265,7 +268,7 @@ class PolicyWriterServiceImpl(

val readTemplateTime1 = System.currentTimeMillis
for {
configAndPaths <- calculatePathsForNodeConfigurations(interestingNodeConfigs, rootNodeId, allNodeConfigs, newPostfix, backupPostfix)
configAndPaths <- calculatePathsForNodeConfigurations(interestingNodeConfigs, rootNodeId, allNodeInfos, newPostfix, backupPostfix)
pathsInfo = configAndPaths.map { _.paths }
templates <- readTemplateFromFileSystem(techniqueIds)
resources <- readResourcesFromFileSystem(techniqueIds)
Expand All @@ -278,10 +281,9 @@ class PolicyWriterServiceImpl(
//////////
// nothing agent specific before that
//////////

preparedPromises <- ParallelSequence.traverse(configAndPaths) { case agentNodeConfig =>
val nodeConfigId = versions(agentNodeConfig.config.nodeInfo.id)
prepareTemplate.prepareTemplateForAgentNodeConfiguration(agentNodeConfig, nodeConfigId, rootNodeId, templates, allNodeConfigs, Policy.TAG_OF_RUDDER_ID, globalPolicyMode, generationTime) ?~!
prepareTemplate.prepareTemplateForAgentNodeConfiguration(agentNodeConfig, nodeConfigId, rootNodeId, templates, Policy.TAG_OF_RUDDER_ID, globalPolicyMode, generationTime) ?~!
s"Error when calculating configuration for node '${agentNodeConfig.config.nodeInfo.hostname}' (${agentNodeConfig.config.nodeInfo.id.value})"
}
preparedPromisesTime = System.currentTimeMillis
Expand All @@ -306,7 +308,7 @@ class PolicyWriterServiceImpl(
// nothing agent specific after that
//////////

propertiesWritten <- ParallelSequence.traverse(configAndPaths) { case agentNodeConfig =>
_ <- ParallelSequence.traverse(configAndPaths) { case agentNodeConfig =>
writeNodePropertiesFile(agentNodeConfig) ?~!
s"An error occured while writing property file for Node ${agentNodeConfig.config.nodeInfo.hostname} (id: ${agentNodeConfig.config.nodeInfo.id.value}"
}
Expand All @@ -315,7 +317,7 @@ class PolicyWriterServiceImpl(
propertiesWrittenDur = propertiesWrittenTime - promiseWrittenTime
_ = policyLogger.debug(s"Properties written in ${propertiesWrittenDur} ms")

parametersWritten <- ParallelSequence.traverse(configAndPaths) { case agentNodeConfig =>
_ <- ParallelSequence.traverse(configAndPaths) { case agentNodeConfig =>
writeRudderParameterFile(agentNodeConfig) ?~!
s"An error occured while writing parameter file for Node ${agentNodeConfig.config.nodeInfo.hostname} (id: ${agentNodeConfig.config.nodeInfo.id.value}"
}
Expand All @@ -324,12 +326,14 @@ class PolicyWriterServiceImpl(
parametersWrittenDur = parametersWrittenTime - propertiesWrittenTime
_ = policyLogger.debug(s"Parameters written in ${parametersWrittenDur} ms")

licensesCopied <- copyLicenses(configAndPaths, allLicenses)
_ <- copyLicenses(configAndPaths, allLicenses)

licensesCopiedTime = System.currentTimeMillis
licensesCopiedDur = licensesCopiedTime - parametersWrittenTime
_ = policyLogger.debug(s"Licenses copied in ${licensesCopiedDur} ms")



_ = fillTemplates.clearCache
/// perhaps that should be a post-hook somehow ?
// and perhaps we should have an AgentSpecific global pre/post write
Expand Down Expand Up @@ -397,8 +401,7 @@ class PolicyWriterServiceImpl(
postMvHooksTime2 = System.currentTimeMillis
_ = policyLogger.debug(s"Hooks for policy-generation-node-finished executed in ${postMvHooksTime2 - movedPromisesTime2} ms")
} yield {
val ids = movedPromises.map { _.nodeId }.toSet
allNodeConfigs.filterKeys { id => ids.contains(id) }.values.toSeq
movedPromises.map { _.nodeId }
}

}
Expand All @@ -415,7 +418,7 @@ class PolicyWriterServiceImpl(
private[this] def calculatePathsForNodeConfigurations(
configs : Seq[NodeConfiguration]
, rootNodeConfigId : NodeId
, allNodeConfigs : Map[NodeId, NodeConfiguration]
, allNodeInfos : Map[NodeId, NodeInfo]
, newsFileExtension : String
, backupFileExtension: String
): Box[Seq[AgentNodeConfiguration]] = {
Expand All @@ -438,7 +441,7 @@ class PolicyWriterServiceImpl(
, pathComputer.getRootPath(agentType) + backupFileExtension
))
} else {
pathComputer.computeBaseNodePath(config.nodeInfo.id, rootNodeConfigId, allNodeConfigs.mapValues(_.nodeInfo)).map { case NodePromisesPaths(id, base, news, backup) =>
pathComputer.computeBaseNodePath(config.nodeInfo.id, rootNodeConfigId, allNodeInfos).map { case NodePromisesPaths(id, base, news, backup) =>
val postfix = agentType.toRulesPath
NodePromisesPaths(id, base + postfix, news + postfix, backup + postfix)
}
Expand Down Expand Up @@ -760,8 +763,6 @@ class PolicyWriterServiceImpl(

/**
* Move the machine promises folder to the backup folder
* @param machineFolder
* @param backupFolder
*/
private[this] def backupNodeFolder(nodeFolder: String, backupFolder: String): Unit = {
val src = new File(nodeFolder)
Expand All @@ -777,8 +778,6 @@ class PolicyWriterServiceImpl(

/**
* Move the newly created folder to the final location
* @param newFolder : where the promises have been written
* @param nodeFolder : where the promises will be
*/
private[this] def moveNewNodeFolder(sourceFolder: String, destinationFolder: String): Unit = {
val src = new File(sourceFolder)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ trait PrepareTemplateVariables {
, nodeConfigVersion: NodeConfigId
, rootNodeConfigId : NodeId
, templates : Map[(TechniqueResourceId, AgentType), TechniqueTemplateCopyInfo]
, allNodeConfigs : Map[NodeId, NodeConfiguration]
, rudderIdCsvTag : String
, globalPolicyMode : GlobalPolicyMode
, generationTime : DateTime
Expand All @@ -97,7 +96,6 @@ class PrepareTemplateVariablesImpl(
, nodeConfigVersion: NodeConfigId
, rootNodeConfigId : NodeId
, templates : Map[(TechniqueResourceId, AgentType), TechniqueTemplateCopyInfo]
, allNodeConfigs : Map[NodeId, NodeConfiguration]
, rudderIdCsvTag : String
, globalPolicyMode : GlobalPolicyMode
, generationTime : DateTime
Expand Down
6 changes: 6 additions & 0 deletions webapp/sources/rudder/rudder-web/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ along with Rudder. If not, see <http://www.gnu.org/licenses/>.
</includes>
</resource>
</webResources>

<!-- this is needed to get instrumentation working, jar need to be out of web-inf -->
<packagingExcludes>
WEB-INF/lib/jamm-*.jar
</packagingExcludes>

</configuration>
</plugin>
<plugin>
Expand Down