From ef05efe49209862373b1374af908ec6a1516aa65 Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Thu, 3 Feb 2022 22:06:16 +0100 Subject: [PATCH 01/14] Fixes #20716: Improve dynamic group computation speed --- .../rudder/domain/queries/CmdbQuery.scala | 25 ++++++++++ .../rudder/domain/queries/DitQueryData.scala | 2 +- .../services/nodes/NodeInfoService.scala | 4 ++ .../services/queries/LdapQueryProcessor.scala | 49 +++++++++++++------ 4 files changed, 65 insertions(+), 15 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala index b8724fd7461..8bf7a5ed108 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala @@ -222,6 +222,31 @@ final case object NodeStateComparator extends NodeCriterionType { } +final case object NodeStringComparator extends NodeCriterionType { + override val comparators = BaseComparators.comparators + + override protected def validateSubCase(v: String, comparator: CriterionComparator) = { + if(null == v || v.isEmpty) Left(Inconsistency("Empty string not allowed")) else { + comparator match { + case Regex | NotRegex => validateRegex(v) + case x => Right(v) + } + } + } + + override def matches(comparator: CriterionComparator, value: String): NodeInfoMatcher = { + comparator match { + case Equals => NodeInfoMatcher(s"Prop equals '${value}'", (node: NodeInfo) => node.hostname == value ) + case NotEquals => NodeInfoMatcher(s"Prop not equals '${value}'", (node: NodeInfo) => node.hostname != value ) + case Regex => NodeInfoMatcher(s"Prop matches regex '${value}'", (node: NodeInfo) => node.hostname.matches(value) ) + case NotRegex => NodeInfoMatcher(s"Prop matches not regex '${value}'", (node: NodeInfo) => !node.hostname.matches(value) ) + case Exists => NodeInfoMatcher(s"Prop exists", (node: NodeInfo) => node.hostname.nonEmpty ) + case NotExists => NodeInfoMatcher(s"Prop doesn't exists", (node: NodeInfo) => node.hostname.isEmpty ) + + } + } + +} /* * This comparator is used for "node properties"-like attribute, i.e: diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/DitQueryData.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/DitQueryData.scala index 880d0c4fc17..19170f047c5 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/DitQueryData.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/DitQueryData.scala @@ -192,7 +192,7 @@ class DitQueryData(dit: InventoryDit, nodeDit: NodeDit, rudderDit: RudderDit, ge ObjectCriterion(OC_NODE, Seq( Criterion("OS",OstypeComparator) , Criterion(A_NODE_UUID, StringComparator) - , Criterion(A_HOSTNAME, StringComparator) + , Criterion(A_HOSTNAME, NodeStringComparator) , Criterion(A_OS_NAME,OsNameComparator) , Criterion(A_OS_FULL_NAME, OrderedStringComparator) , Criterion(A_OS_VERSION, OrderedStringComparator) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala index a79588c7eeb..eb120e1f256 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala @@ -578,6 +578,10 @@ trait NodeInfoServiceCached extends NodeInfoService with NamedZioLogger with Cac ) } + def getAllNodesEntry() = { + nodeCache.map(x => x.nodeInfos.values.toSeq.map(_._1.nodeEntry)) + } + /** * Update cache, without doing anything with the data */ diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala index 21e35fbe6c1..2b33bb37c12 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala @@ -37,7 +37,6 @@ package com.normation.rudder.services.queries import java.util.regex.Pattern - import com.normation.inventory.domain._ import com.normation.inventory.ldap.core.LDAPConstants._ import com.normation.inventory.ldap.core._ @@ -47,8 +46,7 @@ import com.normation.rudder.domain._ import com.normation.rudder.domain.nodes.NodeInfo import com.normation.rudder.domain.queries._ import com.normation.rudder.repository.ldap.LDAPEntityMapper -import com.normation.rudder.services.nodes.LDAPNodeInfo -import com.normation.rudder.services.nodes.NodeInfoService +import com.normation.rudder.services.nodes.{LDAPNodeInfo, NodeInfoService, NodeInfoServiceCached} import com.normation.utils.Control.sequence import com.unboundid.ldap.sdk.DereferencePolicy.NEVER import com.unboundid.ldap.sdk.{LDAPConnection => _, SearchScope => _, _} @@ -111,6 +109,8 @@ final case class LDAPNodeQuery( //that map MUST not contains node related filters , objectTypesFilters: Map[DnType, Map[String, List[SubQuery]]] , nodeInfoFilters : Seq[NodeInfoMatcher] + , noFilterButTakeAllFromCache: Boolean // this is when we don't have ldapfilter, but only nodeinfo filter, so we + // need to take eveything from cache ) /* @@ -148,7 +148,7 @@ class AcceptedNodesLDAPQueryProcessor( nodeDit : NodeDit , inventoryDit : InventoryDit , processor : InternalLDAPQueryProcessor - , nodeInfoService: NodeInfoService + , nodeInfoService: NodeInfoServiceCached ) extends QueryProcessor with Loggable { private[this] case class QueryResult( @@ -174,7 +174,7 @@ class AcceptedNodesLDAPQueryProcessor( val timePreCompute = System.currentTimeMillis for { - res <- processor.internalQueryProcessor(query,select,limitToNodeIds,debugId).toBox + res <- processor.internalQueryProcessor(query,select,limitToNodeIds,debugId, nodeInfoService.getAllNodesEntry).toBox timeres = (System.currentTimeMillis - timePreCompute) _ = logger.debug(s"LDAP result: ${res.entries.size} entries obtained in ${timeres}ms for query ${query.toString}") ldapEntries <- nodeInfoService.getLDAPNodeInfo(res.entries.flatMap(x => x(A_NODE_UUID).map(NodeId(_))).toSet, res.nodeFilters, query.composition).toBox @@ -357,6 +357,8 @@ class InternalLDAPQueryProcessor( , select : Seq[String] = Seq() , limitToNodeIds: Option[Seq[NodeId]] = None , debugId : Long = 0L + , allNodesEntry : Option[Seq[LDAPEntry]] = None// this is hackish, to have the list of all node if + // only nodeinfofilters, so that we don't look for all ) : IOResult[LdapQueryProcessorResult] = { @@ -470,13 +472,29 @@ class InternalLDAPQueryProcessor( _ <- logPure.debug(s"[${debugId}] Start search for ${query.toString}") // Construct & normalize the data nq <- normalizedQuery - lots <- ldapObjectTypeSets(nq) - dmms <- dnMapMapSets(nq, lots) - optdms = dnMapSets(nq, dmms) + // special case: no query, but we create a dummy one, + // identified by noFilterButTakeAllFromCache = true + // in this case, we skip all this part + optdms <- { + if (nq.noFilterButTakeAllFromCache) { + None.succeed + } else { + + for { + lots <- ldapObjectTypeSets(nq) + dmms <- dnMapMapSets(nq, lots) + inneroptdms = dnMapSets(nq, dmms) + } yield { + inneroptdms + } + } + } // If dnMapSets returns a None, then it means that we are ANDing composition with an empty value // so we skip the last query results <- optdms match { - case None => + case None if nq.noFilterButTakeAllFromCache => + allNodesEntry.getOrElse(Seq()).succeed + case None => Seq[LDAPEntry]().succeed case Some(dms) => (for { @@ -973,17 +991,20 @@ class InternalLDAPQueryProcessor( subQueries = groupedSetFilter.view.mapValues(_.view.filterKeys { _ != "node" }.toMap).filterNot( _._2.isEmpty).toMap } yield { // at that point, it may happen that nodeFilters and otherFilters are empty - val mainFilters = if(groupedSetFilter.isEmpty) { + val (mainFilters, andAndEmpty) = if(groupedSetFilter.isEmpty) { query.composition match { // In that case, we add a "get all nodes" query and all filters will be done in node info. - case And => Some(Set[ExtendedFilter](LDAPFilter(BuildFilter.ALL))) + // we should have a specific case here, saying: it's empty, we are AND, so we AND with all the cache +// case And => (None, true) + + case And => (Some(Set[ExtendedFilter](LDAPFilter(BuildFilter.ALL))), true) // In that case, We should return no Nodes from this query and we will only query nodes with filters from node info. - case Or => None + case Or => (None, false) } - } else { nodeFilters } + } else { (nodeFilters, false) } val nodeInfos = nodeInfoFilters.map { case QueryFilter.NodeInfo(c, comp, value) => c.matches(comp, value)} - LDAPNodeQuery(mainFilters, query.composition, query.transform, subQueries, nodeInfos) + LDAPNodeQuery(mainFilters, query.composition, query.transform, subQueries, nodeInfos, andAndEmpty) } } } From 4533ceff86e5b5a546ce64119df56a23e018f910 Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Fri, 4 Feb 2022 21:38:10 +0100 Subject: [PATCH 02/14] fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed --- .../rudder/domain/queries/CmdbQuery.scala | 80 +++++++++++++++++-- .../rudder/domain/queries/DitQueryData.scala | 13 +-- 2 files changed, 80 insertions(+), 13 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala index 8bf7a5ed108..b0e0ffd512d 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala @@ -51,6 +51,7 @@ import com.normation.rudder.domain.nodes.NodeGroupId import com.normation.rudder.domain.nodes.NodeInfo import com.normation.rudder.domain.nodes.NodeState import com.normation.rudder.domain.properties.NodeProperty +import com.normation.rudder.domain.queries.OstypeComparator.osTypes import com.normation.rudder.services.queries._ import com.normation.zio._ import com.unboundid.ldap.sdk._ @@ -221,8 +222,72 @@ final case object NodeStateComparator extends NodeCriterionType { ) } +final case object NodeOstypeComparator extends NodeCriterionType { + override def comparators = Seq(Equals, NotEquals) + override protected def validateSubCase(v:String,comparator:CriterionComparator) = { + if(null == v || v.isEmpty) Left(Inconsistency("Empty string not allowed")) else Right(v) + } + + override def matches(comparator: CriterionComparator, value: String): NodeInfoMatcher = { + comparator match { + case Equals => NodeInfoMatcher(s"Prop equals '${value}'", (node: NodeInfo) => node.osDetails.os.kernelName == value ) + case _ => NodeInfoMatcher(s"Prop not equals '${value}'", (node: NodeInfo) => node.osDetails.os.kernelName != value ) + } + } + + override def toForm(value: String, func: String => Any, attrs: (String, String)*) : Elem = + SHtml.select( + (osTypes map (e => (e,e))).toSeq + , { if(osTypes.contains(value)) Full(value) else Empty} + , func + , attrs:_* + ) +} + +final case object NodeOsNameComparator extends NodeCriterionType { + + import net.liftweb.http.S + + val osNames = AixOS :: + BsdType.allKnownTypes.sortBy { _.name } ::: + LinuxType.allKnownTypes.sortBy { _.name } ::: + (SolarisOS :: Nil) ::: + WindowsType.allKnownTypes + + + override def comparators = Seq(Equals, NotEquals) + override protected def validateSubCase(v:String,comparator:CriterionComparator) = { + if(null == v || v.isEmpty) Left(Inconsistency("Empty string not allowed")) else Right(v) + } -final case object NodeStringComparator extends NodeCriterionType { + override def matches(comparator: CriterionComparator, value: String): NodeInfoMatcher = { + comparator match { + case Equals => NodeInfoMatcher(s"Prop equals '${value}'", (node: NodeInfo) => node.osDetails.os.name == value ) + case _ => NodeInfoMatcher(s"Prop not equals '${value}'", (node: NodeInfo) => node.osDetails.os.name != value ) + } + } + + private[this] def distribName(x: OsType): String = { + x match { + //add linux: for linux + case _: LinuxType => "Linux - " + S.?("os.name."+x.name) + case _: BsdType => "BSD - " + S.?("os.name."+x.name) + //nothing special for windows, Aix and Solaris + case _ => S.?("os.name."+x.name) + } + } + + override def toForm(value: String, func: String => Any, attrs: (String, String)*) : Elem = + SHtml.select( + osNames.map(e => (e.name,distribName(e))).toSeq, + {osNames.find(x => x.name == value).map( _.name)}, + func, + attrs:_* + ) +} + + +final case class NodeStringComparator(access: NodeInfo => String) extends NodeCriterionType { override val comparators = BaseComparators.comparators override protected def validateSubCase(v: String, comparator: CriterionComparator) = { @@ -235,13 +300,14 @@ final case object NodeStringComparator extends NodeCriterionType { } override def matches(comparator: CriterionComparator, value: String): NodeInfoMatcher = { + comparator match { - case Equals => NodeInfoMatcher(s"Prop equals '${value}'", (node: NodeInfo) => node.hostname == value ) - case NotEquals => NodeInfoMatcher(s"Prop not equals '${value}'", (node: NodeInfo) => node.hostname != value ) - case Regex => NodeInfoMatcher(s"Prop matches regex '${value}'", (node: NodeInfo) => node.hostname.matches(value) ) - case NotRegex => NodeInfoMatcher(s"Prop matches not regex '${value}'", (node: NodeInfo) => !node.hostname.matches(value) ) - case Exists => NodeInfoMatcher(s"Prop exists", (node: NodeInfo) => node.hostname.nonEmpty ) - case NotExists => NodeInfoMatcher(s"Prop doesn't exists", (node: NodeInfo) => node.hostname.isEmpty ) + case Equals => NodeInfoMatcher(s"Prop equals '${value}'", (node: NodeInfo) => access(node) == value ) + case NotEquals => NodeInfoMatcher(s"Prop not equals '${value}'", (node: NodeInfo) => access(node) != value ) + case Regex => NodeInfoMatcher(s"Prop matches regex '${value}'", (node: NodeInfo) => access(node).matches(value) ) + case NotRegex => NodeInfoMatcher(s"Prop matches not regex '${value}'", (node: NodeInfo) => !access(node).matches(value) ) + case Exists => NodeInfoMatcher(s"Prop exists", (node: NodeInfo) => access(node).nonEmpty ) + case NotExists => NodeInfoMatcher(s"Prop doesn't exists", (node: NodeInfo) => access(node).isEmpty ) } } diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/DitQueryData.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/DitQueryData.scala index 19170f047c5..300e947e853 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/DitQueryData.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/DitQueryData.scala @@ -48,6 +48,7 @@ import com.normation.rudder.domain.RudderLDAPConstants.A_NODE_GROUP_UUID import com.normation.rudder.domain.RudderLDAPConstants.A_NODE_PROPERTY import com.normation.rudder.domain.RudderLDAPConstants.A_STATE import com.normation.rudder.domain.RudderLDAPConstants.OC_RUDDER_NODE_GROUP +import com.normation.rudder.domain.nodes.NodeInfo import com.normation.rudder.services.queries.SpecialFilter import com.unboundid.ldap.sdk.DN import com.unboundid.ldap.sdk.Filter @@ -190,10 +191,10 @@ class DitQueryData(dit: InventoryDit, nodeDit: NodeDit, rudderDit: RudderDit, ge Criterion(A_MEMORY_CAPACITY, MemoryComparator) )), ObjectCriterion(OC_NODE, Seq( - Criterion("OS",OstypeComparator) - , Criterion(A_NODE_UUID, StringComparator) - , Criterion(A_HOSTNAME, NodeStringComparator) - , Criterion(A_OS_NAME,OsNameComparator) + Criterion("OS",NodeOstypeComparator) + , Criterion(A_NODE_UUID, NodeStringComparator(node => node.node.id.value)) + , Criterion(A_HOSTNAME, NodeStringComparator(node => node.hostname)) + , Criterion(A_OS_NAME,NodeOsNameComparator) , Criterion(A_OS_FULL_NAME, OrderedStringComparator) , Criterion(A_OS_VERSION, OrderedStringComparator) , Criterion(A_OS_SERVICE_PACK, OrderedStringComparator) @@ -205,9 +206,9 @@ class DitQueryData(dit: InventoryDit, nodeDit: NodeDit, rudderDit: RudderDit, ge , Criterion(A_AGENTS_NAME, AgentComparator) , Criterion(A_ACCOUNT, StringComparator) , Criterion(A_LIST_OF_IP, StringComparator) - , Criterion(A_ROOT_USER, StringComparator) + , Criterion(A_ROOT_USER, NodeStringComparator(node => node.localAdministratorAccountName)) , Criterion(A_INVENTORY_DATE, DateComparator) - , Criterion(A_POLICY_SERVER_UUID, StringComparator) + , Criterion(A_POLICY_SERVER_UUID, NodeStringComparator(node => node.policyServerId.value)) )), ObjectCriterion(OC_SOFTWARE, Seq( Criterion(A_NAME, StringComparator), From f2079bb1e7a1a290033e166b14befd153678429d Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Sun, 6 Feb 2022 21:15:19 +0100 Subject: [PATCH 03/14] fixup! fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed --- .../ldap/LDAPNodeGroupRepository.scala | 51 +++++++++++-------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPNodeGroupRepository.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPNodeGroupRepository.scala index d115a1bb913..f42db8dabc8 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPNodeGroupRepository.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPNodeGroupRepository.scala @@ -752,28 +752,37 @@ class WoLDAPNodeGroupRepository( con <- ldap existing <- getSGEntry(con, nodeGroup.id).notOptional("Error when trying to check for existence of group with id %s. Can not update".format(nodeGroup.id)) oldGroup <- mapper.entry2NodeGroup(existing).toIO.chainError("Error when trying to check for the group %s".format(nodeGroup.id.value)) - systemCheck <- if(onlyUpdateNodes) { - oldGroup.succeed - } else (oldGroup.isSystem, systemCall) match { - case (true, false) => "System group '%s' (%s) can not be modified".format(oldGroup.name, oldGroup.id.value).fail - case (false, true) => "You can not modify a non system group (%s) with that method".format(oldGroup.name).fail - case _ => oldGroup.succeed + // check if old group is the same as new group + result <- if (oldGroup.equals(nodeGroup)) { + None.succeed + } else { + for { + systemCheck <- if (onlyUpdateNodes) { + oldGroup.succeed + } else (oldGroup.isSystem, systemCall) match { + case (true, false) => "System group '%s' (%s) can not be modified".format(oldGroup.name, oldGroup.id.value).fail + case (false, true) => "You can not modify a non system group (%s) with that method".format(oldGroup.name).fail + case _ => oldGroup.succeed + } + name <- checkNameAlreadyInUse(con, nodeGroup.name, nodeGroup.id) + exists <- ZIO.when(name && !onlyUpdateNodes) { + s"Cannot change the group name to ${nodeGroup.name} : there is already a group with the same name".fail + } + onlyNodes <- if (!onlyUpdateNodes) { + UIO.unit + } else { //check that nothing but the node list changed + if (nodeGroup.copy(serverList = oldGroup.serverList) == oldGroup) { + UIO.unit + } else { + logPure.debug(s"Inconsistency when modifying node lists for nodeGroup ${nodeGroup.name}: previous content was ${oldGroup}, new is ${nodeGroup} - only the node list should change") *> + "The group configuration changed compared to the reference group you want to change the node list for. Aborting to preserve consistency".fail + } + } + change <- saveModifyNodeGroupDiff(existing, con, nodeGroup, modId, actor, reason) + } yield { + change + } } - name <- checkNameAlreadyInUse(con, nodeGroup.name, nodeGroup.id) - exists <- ZIO.when(name && !onlyUpdateNodes) { - s"Cannot change the group name to ${nodeGroup.name} : there is already a group with the same name".fail - } - onlyNodes <- if(!onlyUpdateNodes) { - UIO.unit - } else { //check that nothing but the node list changed - if(nodeGroup.copy(serverList = oldGroup.serverList) == oldGroup) { - UIO.unit - } else { - logPure.debug(s"Inconsistency when modifying node lists for nodeGroup ${nodeGroup.name}: previous content was ${oldGroup}, new is ${nodeGroup} - only the node list should change") *> - "The group configuration changed compared to the reference group you want to change the node list for. Aborting to preserve consistency".fail - } - } - result <- saveModifyNodeGroupDiff(existing, con, nodeGroup, modId, actor, reason) } yield { result } From 7ce250a2a4d8659f8561aabc1420982a04a38c44 Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Sun, 6 Feb 2022 22:02:41 +0100 Subject: [PATCH 04/14] fixup! fixup! fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed --- .../ldap/LDAPNodeGroupRepository.scala | 2 + .../services/nodes/NodeInfoService.scala | 6 +-- .../services/queries/LdapQueryProcessor.scala | 47 +++++-------------- .../jdbc/ReportingServiceTest.scala | 2 +- .../CachedFindRuleNodeStatusReportsTest.scala | 2 +- .../com/normation/rudder/MockServices.scala | 2 +- .../TestMigrateSystemTechnique7_0.scala | 2 +- 7 files changed, 22 insertions(+), 41 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPNodeGroupRepository.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPNodeGroupRepository.scala index f42db8dabc8..df79dfb93ce 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPNodeGroupRepository.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPNodeGroupRepository.scala @@ -754,8 +754,10 @@ class WoLDAPNodeGroupRepository( oldGroup <- mapper.entry2NodeGroup(existing).toIO.chainError("Error when trying to check for the group %s".format(nodeGroup.id.value)) // check if old group is the same as new group result <- if (oldGroup.equals(nodeGroup)) { + logger.info(s"Nothing to update for ${nodeGroup.id}") None.succeed } else { + logger.info(s"Check what changed for for ${nodeGroup.id}") for { systemCheck <- if (onlyUpdateNodes) { oldGroup.succeed diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala index eb120e1f256..f944e22cfc3 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala @@ -115,7 +115,7 @@ trait NodeInfoService { * Retrieve minimal information needed for the node info, used (only) by the * LDAP QueryProcessor. */ - def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition) : IOResult[Set[LDAPNodeInfo]] + def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition) : IOResult[Seq[NodeInfo]] /** * Return a NodeInfo from a NodeId. First check the ou=Node, then fetch the other data @@ -871,10 +871,10 @@ trait NodeInfoServiceCached extends NodeInfoService with NamedZioLogger with Cac cache.view.mapValues(_._2.node).toMap.succeed } - override def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition): IOResult[Set[LDAPNodeInfo]] = { + override def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition): IOResult[Seq[NodeInfo]] = { // if nodeIds is empty and composition is and, return an empty set; with or, we need to run it in all cases if (nodeIds.isEmpty && composition == And) { - Set[LDAPNodeInfo]().succeed + Seq[NodeInfo]().succeed } else { withUpToDateCache(s"${nodeIds.size} ldap node info") { cache => PostFilterNodeFromInfoService.getLDAPNodeInfo(nodeIds, predicates, composition, cache).succeed diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala index 2b33bb37c12..a94d0da9de4 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala @@ -168,29 +168,24 @@ class AcceptedNodesLDAPQueryProcessor( query:QueryTrait, select:Seq[String], limitToNodeIds:Option[Seq[NodeId]] - ) : Box[Seq[QueryResult]] = { + ) : Box[Seq[NodeInfo]] = { val debugId = if(logger.isDebugEnabled) Helpers.nextNum else 0L val timePreCompute = System.currentTimeMillis for { - res <- processor.internalQueryProcessor(query,select,limitToNodeIds,debugId, nodeInfoService.getAllNodesEntry).toBox + res <- processor.internalQueryProcessor(query,select,limitToNodeIds,debugId, nodeInfoService.getAllNodesEntry()).toBox timeres = (System.currentTimeMillis - timePreCompute) _ = logger.debug(s"LDAP result: ${res.entries.size} entries obtained in ${timeres}ms for query ${query.toString}") - ldapEntries <- nodeInfoService.getLDAPNodeInfo(res.entries.flatMap(x => x(A_NODE_UUID).map(NodeId(_))).toSet, res.nodeFilters, query.composition).toBox + nodesInfos <- nodeInfoService.getLDAPNodeInfo(res.entries.flatMap(x => x(A_NODE_UUID).map(NodeId(_))).toSet, res.nodeFilters, query.composition).toBox ldapEntryTime = (System.currentTimeMillis - timePreCompute - timeres) - _ = logger.debug(s"[post-filter:rudderNode] Found ${ldapEntries.size} nodes when filtering for info service existence and properties (${ldapEntryTime} ms)") + _ = logger.debug(s"[post-filter:rudderNode] Found ${nodesInfos.size} nodes when filtering for info service existence and properties (${ldapEntryTime} ms)") } yield { - - val inNodes = ldapEntries.map { case LDAPNodeInfo(nodeEntry, nodeInv, machineInv) => - QueryResult(nodeEntry, nodeInv, machineInv) - } - if(logger.isDebugEnabled) { - val filtered = res.entries.map( _(A_NODE_UUID).get ).toSet -- inNodes.flatMap { case QueryResult(e, _, _) => e(A_NODE_UUID) }.toSet + val filtered = res.entries.map( _(A_NODE_UUID).get ).toSet -- nodesInfos.map( x => x.node.id.value) if(filtered.nonEmpty) { - logger.debug(s"[${debugId}] [post-filter:rudderNode] ${inNodes.size} results (following nodes not in ou=Nodes,cn=rudder-configuration or not matching filters on NodeInfo: ${filtered.mkString(", ")}") + logger.debug(s"[${debugId}] [post-filter:rudderNode] ${nodesInfos.size} results (following nodes not in ou=Nodes,cn=rudder-configuration or not matching filters on NodeInfo: ${filtered.mkString(", ")}") } } @@ -200,15 +195,15 @@ class AcceptedNodesLDAPQueryProcessor( case NodeReturnType => // we have a special case for the root node that always never to that group, even if some weird // scenario lead to the removal (or non addition) of them - val withoutServerRole = inNodes.filterNot { case QueryResult(e, inv, _) => e(A_NODE_UUID) == Some("root") } + val withoutServerRole = nodesInfos.filterNot { case x: NodeInfo => x.node.id.value ==("root") } if(logger.isDebugEnabled) { - val filtered = (inNodes.flatMap { case QueryResult(e, _, _) => e(A_NODE_UUID) }).toSet -- withoutServerRole.flatMap { case QueryResult(e, _, _) => e(A_NODE_UUID) } + val filtered = nodesInfos.filter { case x: NodeInfo => x.node.id.value ==("root") } if(!filtered.isEmpty) { logger.debug("[%s] [post-filter:policyServer] %s results".format(debugId, withoutServerRole.size, filtered.mkString(", "))) } } withoutServerRole.toSeq - case NodeAndPolicyServerReturnType => inNodes.toSeq + case NodeAndPolicyServerReturnType => nodesInfos.toSeq } } } @@ -216,28 +211,12 @@ class AcceptedNodesLDAPQueryProcessor( override def process(query:QueryTrait) : Box[Seq[NodeInfo]] = { //only keep the one of the form Full(...) - queryAndChekNodeId(query, NodeInfoService.nodeInfoAttributes, None).map { seq => seq.flatMap { - case QueryResult(nodeEntry, inventoryEntry,machine) => - processor.ldapMapper.convertEntriesToNodeInfos(nodeEntry, inventoryEntry,machine).toBox match { - case Full(nodeInfo) => Seq(nodeInfo) - case e:EmptyBox => - logger.error((e ?~! "Ignoring entry in result set").messageChain) - Seq() - } - } } + queryAndChekNodeId(query, NodeInfoService.nodeInfoAttributes, None) } override def processOnlyId(query:QueryTrait) : Box[Seq[NodeId]] = { //only keep the one of the form Full(...) - queryAndChekNodeId(query, Seq(A_NODE_UUID), None).map { seq => seq.flatMap { - case QueryResult(nodeEntry, _ , _) => - nodeDit.NODES.NODE.idFromDn(nodeEntry.dn) match { - case Some(nodeId) => Some(nodeId) - case None => - logger.error(s"Error when processing query ${query.toJSONString}: fetched node entry ${nodeEntry.toString()} is not a correct nodeId") - None - } - } } + queryAndChekNodeId(query, Seq(A_NODE_UUID), None).map(seq => seq.map( y => y.node.id)) } } @@ -249,7 +228,7 @@ class AcceptedNodesLDAPQueryProcessor( */ object PostFilterNodeFromInfoService { val logger = LoggerFactory.getLogger("com.normation.rudder.services.queries") - def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, nodes: Map[NodeId, (LDAPNodeInfo, NodeInfo)]): Set[LDAPNodeInfo] = { + def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, nodes: Map[NodeId, (LDAPNodeInfo, NodeInfo)]): Seq[NodeInfo] = { def comp(a: Boolean, b: Boolean) = composition match { case And => a && b case Or => a || b @@ -283,7 +262,7 @@ object PostFilterNodeFromInfoService { } } - nodes.collect { case (_, (x, y)) if predicate(y, predicates) => x }.toSet + nodes.collect { case (_, (_, y)) if predicate(y, predicates) => y }.toSeq } } diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala index 1c6f72d0fba..0b760ad26b0 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala @@ -95,7 +95,7 @@ class ReportingServiceTest extends DBCommon with BoxSpecMatcher { } object nodeInfoService extends NodeInfoService { - def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition) : IOResult[Set[LDAPNodeInfo]] = ??? + def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition) : IOResult[Seq[NodeInfo]] = ??? def getNodeInfo(nodeId: NodeId) : IOResult[Option[NodeInfo]] = ??? def getNodeInfos(nodesId: Set[NodeId]) : IOResult[Set[NodeInfo]] = ??? def getNode(nodeId: NodeId): Box[Node] = ??? diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala index 700dc4c2584..35541079142 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala @@ -132,7 +132,7 @@ class CachedFindRuleNodeStatusReportsTest extends Specification { ) object testNodeInfoService extends NodeInfoService { - def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition) : IOResult[Set[LDAPNodeInfo]] = ??? + def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition) : IOResult[Seq[NodeInfo]] = ??? def getNodeInfo(nodeId: NodeId) : IOResult[Option[NodeInfo]] = ??? def getNodeInfos(nodesId: Set[NodeId]) : IOResult[Set[NodeInfo]] = ??? def getNode(nodeId: NodeId): Box[Node] = ??? diff --git a/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala b/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala index e62ad5cfbe5..85be43245d9 100644 --- a/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala +++ b/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala @@ -1513,7 +1513,7 @@ z5VEb9yx2KikbWyChM1Akp82AV5BzqE80QIBIw== override def getAllNodeInventories(inventoryStatus: InventoryStatus): IOResult[Map[NodeId, NodeInventory]] = getGenericAll(inventoryStatus, _fullInventory(_).map(_.node)) // not implemented yet - override def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition): IOResult[Set[LDAPNodeInfo]] = ??? + override def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition): IOResult[Seq[NodeInfo]] = ??? override def getNumberOfManagedNodes: Int = ??? override def save(serverAndMachine: FullInventory): IOResult[Seq[LDIFChangeRecord]] = ??? override def delete(id: NodeId, inventoryStatus: InventoryStatus): IOResult[Seq[LDIFChangeRecord]] = ??? diff --git a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala index c03bc00130f..cff5eca64aa 100644 --- a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala +++ b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala @@ -164,7 +164,7 @@ class TestMigrateSystemTechniques7_0 extends Specification { val nodeInfoService = new NodeInfoService { override def getAll(): IOResult[Map[NodeId, NodeInfo]] = List(root, relay1).map(x => (x.id, x)).toMap.succeed - override def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition): IOResult[Set[LDAPNodeInfo]] = ??? + override def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition): IOResult[Seq[NodeInfo]] = ??? override def getNodeInfo(nodeId: NodeId): IOResult[Option[NodeInfo]] = ??? override def getNodeInfos(nodeIds: Set[NodeId]): IOResult[Set[NodeInfo]] = ??? override def getNumberOfManagedNodes: Int = ??? From 60b11705a56ba4647912aa6c25799d1d716365f7 Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Mon, 7 Feb 2022 16:38:13 +0100 Subject: [PATCH 05/14] fixup! fixup! fixup! fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed --- .../rudder/domain/queries/DitQueryData.scala | 1 - .../services/nodes/NodeInfoService.scala | 27 +-- .../services/queries/LdapQueryProcessor.scala | 101 ++++++---- .../jdbc/ReportingServiceTest.scala | 3 +- .../services/queries/TestQueryProcessor.scala | 175 +++++++++++++++++- .../CachedFindRuleNodeStatusReportsTest.scala | 3 +- .../com/normation/rudder/MockServices.scala | 4 +- .../bootstrap/liftweb/RudderConfig.scala | 3 +- .../web/components/SearchNodeComponent.scala | 2 +- .../TestMigrateSystemTechnique7_0.scala | 3 +- 10 files changed, 255 insertions(+), 67 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/DitQueryData.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/DitQueryData.scala index 300e947e853..ec4e4eee293 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/DitQueryData.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/DitQueryData.scala @@ -48,7 +48,6 @@ import com.normation.rudder.domain.RudderLDAPConstants.A_NODE_GROUP_UUID import com.normation.rudder.domain.RudderLDAPConstants.A_NODE_PROPERTY import com.normation.rudder.domain.RudderLDAPConstants.A_STATE import com.normation.rudder.domain.RudderLDAPConstants.OC_RUDDER_NODE_GROUP -import com.normation.rudder.domain.nodes.NodeInfo import com.normation.rudder.services.queries.SpecialFilter import com.unboundid.ldap.sdk.DN import com.unboundid.ldap.sdk.Filter diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala index f944e22cfc3..89b2eff24bf 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala @@ -112,10 +112,12 @@ final case class LDAPNodeInfo( trait NodeInfoService { /** - * Retrieve minimal information needed for the node info, used (only) by the - * LDAP QueryProcessor. + * Filter the nodeinfos that match the predicate & composition - don't rely on the cache + * used only by the LDAP QueryProcessor. + * foundNodeInfos are the nodeinfo found by ldap query + * allNodeInfos are all the know nodeInfos to date */ - def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition) : IOResult[Seq[NodeInfo]] + def getLDAPNodeInfo(foundNodeInfos: Seq[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodeInfos: Seq[NodeInfo]): Seq[NodeInfo] /** * Return a NodeInfo from a NodeId. First check the ou=Node, then fetch the other data @@ -157,6 +159,7 @@ trait NodeInfoService { */ def getAllNodes() : IOResult[Map[NodeId, Node]] + def getAllNodeInfos():IOResult[Seq[NodeInfo]] /** * Get all systen node ids, for example * policy server node ids. @@ -578,10 +581,6 @@ trait NodeInfoServiceCached extends NodeInfoService with NamedZioLogger with Cac ) } - def getAllNodesEntry() = { - nodeCache.map(x => x.nodeInfos.values.toSeq.map(_._1.nodeEntry)) - } - /** * Update cache, without doing anything with the data */ @@ -871,14 +870,16 @@ trait NodeInfoServiceCached extends NodeInfoService with NamedZioLogger with Cac cache.view.mapValues(_._2.node).toMap.succeed } - override def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition): IOResult[Seq[NodeInfo]] = { + def getAllNodeInfos():IOResult[Seq[NodeInfo]] = withUpToDateCache("all nodeinfos") { cache => + cache.values.map(_._2).toSeq.succeed + } + + override def getLDAPNodeInfo(foundNodeInfos: Seq[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodeInfos: Seq[NodeInfo]): Seq[NodeInfo] = { // if nodeIds is empty and composition is and, return an empty set; with or, we need to run it in all cases - if (nodeIds.isEmpty && composition == And) { - Seq[NodeInfo]().succeed + if (foundNodeInfos.isEmpty && composition == And) { + Seq[NodeInfo]() } else { - withUpToDateCache(s"${nodeIds.size} ldap node info") { cache => - PostFilterNodeFromInfoService.getLDAPNodeInfo(nodeIds, predicates, composition, cache).succeed - } + PostFilterNodeFromInfoService.getLDAPNodeInfo(foundNodeInfos, predicates, composition, allNodeInfos) } } diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala index a94d0da9de4..35a18142036 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala @@ -46,7 +46,7 @@ import com.normation.rudder.domain._ import com.normation.rudder.domain.nodes.NodeInfo import com.normation.rudder.domain.queries._ import com.normation.rudder.repository.ldap.LDAPEntityMapper -import com.normation.rudder.services.nodes.{LDAPNodeInfo, NodeInfoService, NodeInfoServiceCached} +import com.normation.rudder.services.nodes.{NodeInfoService, NodeInfoServiceCached} import com.normation.utils.Control.sequence import com.unboundid.ldap.sdk.DereferencePolicy.NEVER import com.unboundid.ldap.sdk.{LDAPConnection => _, SearchScope => _, _} @@ -123,8 +123,8 @@ final case class SubQuery(subQueryId: String, dnType: DnType, objectTypeName: St final case class LdapQueryProcessorResult( - // list of entries from inventory matching the search - entries : Seq[LDAPEntry] + // list of NodeInfo matching the search + entries : Seq[NodeInfo] // a post filter to run on node info , nodeFilters: Seq[NodeInfoMatcher] ) @@ -174,16 +174,17 @@ class AcceptedNodesLDAPQueryProcessor( val timePreCompute = System.currentTimeMillis for { - res <- processor.internalQueryProcessor(query,select,limitToNodeIds,debugId, nodeInfoService.getAllNodesEntry()).toBox + allNodeInfos <- nodeInfoService.getAllNodeInfos().toBox + res <- processor.internalQueryProcessor(query,select,limitToNodeIds,debugId, allNodeInfos).toBox timeres = (System.currentTimeMillis - timePreCompute) _ = logger.debug(s"LDAP result: ${res.entries.size} entries obtained in ${timeres}ms for query ${query.toString}") - nodesInfos <- nodeInfoService.getLDAPNodeInfo(res.entries.flatMap(x => x(A_NODE_UUID).map(NodeId(_))).toSet, res.nodeFilters, query.composition).toBox - ldapEntryTime = (System.currentTimeMillis - timePreCompute - timeres) - _ = logger.debug(s"[post-filter:rudderNode] Found ${nodesInfos.size} nodes when filtering for info service existence and properties (${ldapEntryTime} ms)") + nodesInfos = nodeInfoService.getLDAPNodeInfo(res.entries, res.nodeFilters, query.composition, allNodeInfos) + filterTime = (System.currentTimeMillis - timePreCompute - timeres) + _ = logger.debug(s"[post-filter:rudderNode] Found ${nodesInfos.size} nodes when filtering for info service existence and properties (${filterTime} ms)") } yield { if(logger.isDebugEnabled) { - val filtered = res.entries.map( _(A_NODE_UUID).get ).toSet -- nodesInfos.map( x => x.node.id.value) + val filtered = res.entries.map( x => x.node.id.value).diff(nodesInfos.map( x => x.node.id.value)) if(filtered.nonEmpty) { logger.debug(s"[${debugId}] [post-filter:rudderNode] ${nodesInfos.size} results (following nodes not in ou=Nodes,cn=rudder-configuration or not matching filters on NodeInfo: ${filtered.mkString(", ")}") } @@ -228,7 +229,12 @@ class AcceptedNodesLDAPQueryProcessor( */ object PostFilterNodeFromInfoService { val logger = LoggerFactory.getLogger("com.normation.rudder.services.queries") - def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, nodes: Map[NodeId, (LDAPNodeInfo, NodeInfo)]): Seq[NodeInfo] = { + def getLDAPNodeInfo( + foundNodeInfos: Seq[NodeInfo] // the one from the search + , predicates : Seq[NodeInfoMatcher] + , composition : CriterionComposition + , allNodesInfos : Seq[NodeInfo] // all the nodeinfo there is + ): Seq[NodeInfo] = { def comp(a: Boolean, b: Boolean) = composition match { case And => a && b case Or => a || b @@ -245,9 +251,10 @@ object PostFilterNodeFromInfoService { override def matches(node: NodeInfo): Boolean = comp(a.matches(node), b.matches(node)) } + val foundNodeIds = foundNodeInfos.map(_.id) // if there is no predicates (ie no specific filter on NodeInfo), we should just keep nodes from our list def predicate(nodeInfo : NodeInfo, pred: Seq[NodeInfoMatcher]) = { - val contains = nodeIds.contains(nodeInfo.id) + val contains = foundNodeIds.contains(nodeInfo.id) if (pred.isEmpty) { // in that case, whatever the query composition, we can only return what was already found. contains @@ -255,6 +262,9 @@ object PostFilterNodeFromInfoService { val combined = pred.reduceLeft(combine) val validPredicates = combined.matches(nodeInfo) val res = comp(contains, validPredicates) + + println(s"${nodeInfo.id.value}: ${if(res) "OK" else "NOK"} for [${combined.debugString}]") + if(logger.isTraceEnabled()) { logger.trace(s"${nodeInfo.id.value}: ${if(res) "OK" else "NOK"} for [${combined.debugString}]") } @@ -262,7 +272,7 @@ object PostFilterNodeFromInfoService { } } - nodes.collect { case (_, (_, y)) if predicate(y, predicates) => y }.toSeq + allNodesInfos.collect { case nodeinfo if predicate(nodeinfo, predicates) => nodeinfo } } } @@ -273,8 +283,9 @@ object PostFilterNodeFromInfoService { * for pending nodes */ class PendingNodesLDAPQueryChecker( - val checker:InternalLDAPQueryProcessor -) extends QueryChecker { + val checker:InternalLDAPQueryProcessor + , nodeInfoService: NodeInfoService +) extends QueryChecker with Loggable { override def check(query:QueryTrait, limitToNodeIds:Option[Seq[NodeId]]) : Box[Seq[NodeId]] = { if(query.criteria.isEmpty) { @@ -283,13 +294,24 @@ class PendingNodesLDAPQueryChecker( ) Full(Seq.empty[NodeId]) } else { + val timePreCompute = System.currentTimeMillis for { - res <- checker.internalQueryProcessor(query, Seq("1.1"), limitToNodeIds).toBox - ids <- sequence(res.entries) { entry => - checker.ldapMapper.nodeDn2OptNodeId(entry.dn).toBox ?~! "Can not get node ID from dn %s".format(entry.dn) - } + // get the pending node onfos we are considering + allPendingNodeInfos <- nodeInfoService.getPendingNodeInfos().toBox + pendingNodeInfos = limitToNodeIds match { + case None => allPendingNodeInfos.values.toSeq + case Some(ids) => allPendingNodeInfos.collect { case (nodeId, nodeInfo) if ids.contains(nodeId) => nodeInfo}.toSeq + } + res <- checker.internalQueryProcessor(query, Seq("1.1"), limitToNodeIds, 0, pendingNodeInfos).toBox + timeres = (System.currentTimeMillis - timePreCompute) + _ = logger.debug(s"LDAP result: ${res.entries.size} entries in pending nodes obtained in ${timeres}ms for query ${query.toString}") + + nodesInfos = nodeInfoService.getLDAPNodeInfo(res.entries, res.nodeFilters, query.composition, pendingNodeInfos) + filterTime = (System.currentTimeMillis - timePreCompute - timeres) + _ = logger.debug(s"[post-filter:rudderNode] Found ${nodesInfos.size} nodes when filtering pending nodes for info service existence and properties (${filterTime} ms)") + } yield { - ids + nodesInfos.map(_.node.id) } } } @@ -336,8 +358,9 @@ class InternalLDAPQueryProcessor( , select : Seq[String] = Seq() , limitToNodeIds: Option[Seq[NodeId]] = None , debugId : Long = 0L - , allNodesEntry : Option[Seq[LDAPEntry]] = None// this is hackish, to have the list of all node if - // only nodeinfofilters, so that we don't look for all + , allNodeInfos : Seq[NodeInfo] // this is hackish, to have the list of all node if + // only nodeinfofilters, so that we don't look for all + // it is always called, no need to be a lambda ) : IOResult[LdapQueryProcessorResult] = { @@ -472,12 +495,13 @@ class InternalLDAPQueryProcessor( // so we skip the last query results <- optdms match { case None if nq.noFilterButTakeAllFromCache => - allNodesEntry.getOrElse(Seq()).succeed + allNodeInfos.succeed case None => - Seq[LDAPEntry]().succeed + Seq[NodeInfo]().succeed case Some(dms) => (for { // Ok, do the computation here + // still rely on LDAP here _ <- logPure.ifTraceEnabled { ZIO.foreach_(dms) { case (dnType, dns) => logPure.trace(s"/// ${dnType} ==> ${dns.map(_.getRDN).mkString(", ")}") @@ -493,41 +517,40 @@ class InternalLDAPQueryProcessor( rt = nodeObjectTypes.copy(filter = finalLdapFilter) _ <- logPure.debug(s"[${debugId}] |- (final query) ${rt}") entries <- executeQuery(rt.baseDn, rt.scope, nodeObjectTypes.objectFilter, rt.filter, finalSpecialFilters, select.toSet, nq.composition, debugId) - } yield entries). + // convert these entries into nodeInfo + nodesId = entries.flatMap(x => x(A_NODE_UUID)) + nodeInfos = allNodeInfos.filter(nodeInfo => nodesId.contains(nodeInfo.node.id.value)) + } yield nodeInfos). tapError(err => logPure.debug(s"[${debugId}] `-> error: ${err.fullMsg}")). tap(seq => logPure.debug(s"[${debugId}] `-> ${seq.size} results")) } - inverted <- query.transform match { - case ResultTransformation.Identity => results.succeed + // No more LDAP query is required here + inverted = query.transform match { + case ResultTransformation.Identity => results case ResultTransformation.Invert => - for { - _ <- logPure.debug(s"[${debugId}] |- (need to get all nodeIds for inversion) ") - allIds <- executeQuery(nodeObjectTypes.baseDn, nodeObjectTypes.scope, nodeObjectTypes.objectFilter, Some(ALL), Set(), Set(), nq.composition, debugId) - ids = results.flatMap(x => x(A_NODE_UUID)).toSet - res = allIds.filterNot( e => ids.contains(e.value_!(A_NODE_UUID)) ) - _ <- logPure.debug(s"[${debugId}] |- (invert) entries after inversion: ${res.size}") - } yield { + logEffect.debug(s"[${debugId}] |- (need to get all nodeIds for inversion) ") + val res = allNodeInfos.diff(results) + logEffect.debug(s"[${debugId}] |- (invert) entries after inversion: ${res.size}") res } - } // distinctBy computes the hashcode of the object // It is really expensive on LDAP entries. // The dn string is already computed, so the toString is a good alternative (and as also unicity) - postFiltered = postFilterNode(inverted.distinctBy(_.dn.toString), query.returnType, limitToNodeIds) + postFiltered = postFilterNode(inverted.distinctBy(_.node.id.value), query.returnType, limitToNodeIds) } yield { LdapQueryProcessorResult(postFiltered, nq.nodeInfoFilters) } } /** - * That method allows to post-process a list of nodes based on + * That method allows to post-process a list of NodeInfo based on * the resultType. * - step1: ~filter out policy server if we only want "simple" nodes~ => no, we need to do * that in `queryAndChekNodeId` where we know about server roles * - step2: filter out nodes based on a given list of acceptable entries */ - private[this] def postFilterNode(entries: Seq[LDAPEntry], returnType: QueryReturnType, limitToNodeIds:Option[Seq[NodeId]]) : Seq[LDAPEntry] = { - + private[this] def postFilterNode(entries: Seq[NodeInfo], returnType: QueryReturnType, limitToNodeIds:Option[Seq[NodeId]]) : Seq[NodeInfo] = { +// keeping this for backport option to 6.2 val step1 = returnType match { //actually, we are able at that point to know if we have a policy server, //so we don't post-process anything. @@ -536,8 +559,8 @@ class InternalLDAPQueryProcessor( } val step2 = limitToNodeIds match { case None => step1 - case Some(seq) => step1.filter(e => - seq.exists(nodeId => nodeId.value == e(A_NODE_UUID).getOrElse("Missing attribute %s in node entry, that case is not supported.").format(A_NODE_UUID)) + case Some(seq) => step1.filter(nodeInfo => + seq.exists(nodeId => nodeId.value == nodeInfo.node.id.value) ) } diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala index 0b760ad26b0..7d9a060e9e3 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala @@ -95,11 +95,12 @@ class ReportingServiceTest extends DBCommon with BoxSpecMatcher { } object nodeInfoService extends NodeInfoService { - def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition) : IOResult[Seq[NodeInfo]] = ??? + def getLDAPNodeInfo(foundNodeInfos: Seq[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodeInfos: Seq[NodeInfo]): Seq[NodeInfo]= ??? def getNodeInfo(nodeId: NodeId) : IOResult[Option[NodeInfo]] = ??? def getNodeInfos(nodesId: Set[NodeId]) : IOResult[Set[NodeInfo]] = ??? def getNode(nodeId: NodeId): Box[Node] = ??? def getAllNodes() : IOResult[Map[NodeId, Node]] = ??? + def getAllNodeInfos():IOResult[Seq[NodeInfo]] = ??? def getAllNodesIds(): IOResult[Set[NodeId]] = ??? def getAllSystemNodeIds() : IOResult[Seq[NodeId]] = ??? def getPendingNodeInfos(): IOResult[Map[NodeId, NodeInfo]] = ??? diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala index acce4f65d59..2d329e8f69c 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala @@ -188,7 +188,7 @@ class TestQueryProcessor extends Loggable { """).openOrThrowException("For tests"), Nil) - testQueries( q0 :: q1 :: q2 :: Nil, true) + testQueries( q0 :: q1 :: q2 :: Nil, false) } @Test def basicQueriesOnOneNodeParameter(): Unit = { @@ -311,7 +311,61 @@ class TestQueryProcessor extends Loggable { """).openOrThrowException("For tests"), s(0) :: s(1) :: s(2) :: s(3) :: s(4) :: s(5) :: s(6) :: s(7) :: Nil) - testQueries(q1 :: q2 :: q3 :: q4 :: q5 :: q6 :: q7 :: Nil, true) + testQueries(q1 :: q2 :: q3 :: q4 :: q5 :: q6 :: q7 :: Nil, false) + } + + // group of group, with or/and composition + @Test def groupOfgroupsDoIntenalQueryTest(): Unit = { + val q1 = TestQuery( + "q1", + parser(""" + { "select":"node", "where":[ + { "objectType":"group", "attribute":"nodeGroupId", "comparator":"eq", "value":"test-group-node1" } + ] } + """).openOrThrowException("For tests"), + s(1) :: Nil) + + val q2 = TestQuery( + "q2", + parser(""" + { "select":"node", "composition":"or", "where":[ + { "objectType":"group", "attribute":"nodeGroupId", "comparator":"eq", "value":"test-group-node1" } + , { "objectType":"group", "attribute":"nodeGroupId", "comparator":"eq", "value":"test-group-node2" } + ] } + """).openOrThrowException("For tests"), + s(1) :: s(2) :: Nil) + + val q3 = TestQuery( + "q3", + parser(""" + { "select":"node", "where":[ + { "objectType":"group", "attribute":"nodeGroupId", "comparator":"eq", "value":"test-group-node1" } + , { "objectType":"node", "attribute":"ram", "comparator":"gt", "value":"1" } + ] } + """).openOrThrowException("For tests"), + s(1) :: Nil) + + val q4 = TestQuery( + "q4", + parser(""" + { "select":"node", "where":[ + { "objectType":"group", "attribute":"nodeGroupId", "comparator":"eq", "value":"test-group-node1" } + , { "objectType":"group", "attribute":"nodeGroupId", "comparator":"eq", "value":"test-group-node12" } + ] } + """).openOrThrowException("For tests"), + s(1) :: Nil) + + val q5 = TestQuery( + "q5", + parser(""" + { "select":"node", "where":[ + { "objectType":"group", "attribute":"nodeGroupId", "comparator":"eq", "value":"test-group-node12" } + , { "objectType":"group", "attribute":"nodeGroupId", "comparator":"eq", "value":"test-group-node23" } + ] } + """).openOrThrowException("For tests"), + s(2) :: Nil) + + testQueries(q1 :: q2 :: q3 :: q4 :: q5 :: Nil, true) } @Test def machineComponentQueries(): Unit = { @@ -588,7 +642,114 @@ class TestQueryProcessor extends Loggable { // """).openOrThrowException("For tests"), // s.filterNot(n => n == s(2)) ) - testQueries(q0 :: q1 :: q1_ :: q2 :: q2_ :: q3 :: q3_2 :: q4 :: q5 :: q6 :: q7 :: q8 :: q9 :: q10 :: q11 :: Nil, true) + testQueries(q0 :: q1 :: q1_ :: q2 :: q2_ :: q3 :: q3_2 :: q4 :: q5 :: q6 :: q7 :: q8 :: q9 :: q10 :: q11 :: Nil, false) + } + + @Test def regexQueriesInventories(): Unit = { + // this test if for the queries that can be performed using only LDAP + //regex and "subqueries" for logical elements should not be contradictory + //here, we have to *only* search for logical elements with the regex + //and cn is both on node and logical elements + val q0 = TestQuery( + "q0", + parser(""" + { "select":"node", "composition":"or" , "where":[ + , { "objectType":"fileSystemLogicalElement", "attribute":"description", "comparator":"regex", "value":"matchOnM[e]" } + ] } + """).openOrThrowException("For tests"), + s(3) :: Nil) + + + //on software, machine, machine element, node element + val q2 = TestQuery( + "q2", + parser(""" + { "select":"nodeAndPolicyServer", "where":[ + { "objectType":"software", "attribute":"cn", "comparator":"regex" , "value":"Software [0-9]" } + , { "objectType":"machine", "attribute":"machineId", "comparator":"regex" , "value":"machine[0-2]" } + , { "objectType":"fileSystemLogicalElement", "attribute":"fileSystemFreeSpace", "comparator":"regex", "value":"[01]{2}" } + , { "objectType":"biosPhysicalElement", "attribute":"softwareVersion", "comparator":"regex", "value":"[6.0]+" } + ] } + """).openOrThrowException("For tests"), + s(7) :: Nil) + + val q2_ = TestQuery("q2_", query = q2.query match { case q : Query => q.copy(composition = Or); case q : NewQuery => q.copy(composition = Or)}, + ( + s(2) :: s(7) :: //software + s(4) :: s(5) :: s(6) :: s(7) :: //machine + s(2) :: root :: // free space + s(2) :: //bios + Nil).distinct) + + val q5 = TestQuery( + "q5", + parser(""" + { "select":"nodeAndPolicyServer","composition":"or", "where":[ + , { "objectType":"fileSystemLogicalElement" , "attribute":"mountPoint" , "comparator":"regex", "value":"[/]" } + ] } + """).openOrThrowException("For tests"), + s(3) :: s(7) :: root :: Nil) + + //same as q5 on IP, to test with escaping + //192.168.56.101 is for node3 + val q8 = TestQuery( + "q8", + parser(""" + { "select":"node", "where":[ + { "objectType":"node" , "attribute":"ipHostNumber" , "comparator":"notRegex", "value":"192.168.56.101" } + ] } + """).openOrThrowException("For tests"), + s.filterNot( _ == s(1)) ) + + //typical use case for server on internal/dmz/both: want intenal (but not both) + //that test a match regex and not regex + val q9 = TestQuery( + "q9", + parser(""" + { "select":"node", "where":[ + { "objectType":"node" , "attribute":"ipHostNumber" , "comparator":"regex", "value":"127.0.0.*" } + , { "objectType":"node" , "attribute":"ipHostNumber" , "comparator":"notRegex", "value":"192.168.56.10[23]" } + ] } + """).openOrThrowException("For tests"), + Seq(s(1), s(4)) ) + //s0,5,6,7,8 not ok because no 127.0.0.1 + //s1 ok because not in "not regex" pattern + //s2,s3 not ok because in the "not regex" pattern + //s4 ok because only 127.0.0.1 + + // test query that matches a software version + val q10 = TestQuery( + "q10", + parser(""" + { "select":"node", "where":[ + { "objectType":"software", "attribute":"softwareVersion", "comparator":"regex", "value":"1\\.0.*" } + ] } + """).openOrThrowException("For tests"), + Seq(s(2), s(7)) ) + + // test "notRegex" query: "I want node for which ram is not "100000000" (ie not node1) + val q11 = TestQuery( + "q11", + parser(""" + { "select":"node", "where":[ + { "objectType":"node", "attribute":"ram", "comparator":"notRegex", "value":"100000000" } + ] } + """).openOrThrowException("For tests"), + s.filterNot(n => n == s(1)) ) + + // test query that doesn't match a software name, ie we want all nodes on which "software 1" is not + // installed (we don't care if there is 0 or 1000 other software) + // THIS DOES NOT WORK DUE TO: https://issues.rudder.io/issues/19137 + // val q12 = TestQuery( + // "q12", + // parser(""" + // { "select":"node", "composition":"or", "where":[ + // { "objectType":"software", "attribute":"cn", "comparator":"notRegex", "value":"Software 1" } + // ] } + // """).openOrThrowException("For tests"), + // s.filterNot(n => n == s(2)) ) + + testQueries(q0 :: q2 :: q2_ :: q5 :: q8 :: q9 :: q10 :: q11 :: Nil, true) } @Test def invertQueries(): Unit = { @@ -704,7 +865,7 @@ class TestQueryProcessor extends Loggable { """).openOrThrowException("For tests"), sr) - testQueries( q0 :: q1 :: Nil, true) + testQueries( q0 :: q1 :: Nil, false) } @Test def agentTypeQueries: Unit = { @@ -1077,10 +1238,10 @@ class TestQueryProcessor extends Loggable { if (doInternalQueryTest) { logger.debug("Testing with expected entries, This test should be ignored when we are looking for Nodes with NodeInfo and inventory (ie when we are looking for property and environement variable") + val allNodesInfos = nodeInfoService.getAllNodeInfos().runNow val foundWithLimit = - (internalLDAPQueryProcessor.internalQueryProcessor(query, limitToNodeIds = Some(ids)).runNow.entries.map { - entry => - NodeId(entry("nodeId").get) + (internalLDAPQueryProcessor.internalQueryProcessor(query, limitToNodeIds = Some(ids), allNodeInfos= allNodesInfos).runNow.entries.map { + _.node.id }).distinct.sortBy( _.value ) assertEquals( diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala index 35541079142..1e3d377ebc1 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala @@ -132,12 +132,13 @@ class CachedFindRuleNodeStatusReportsTest extends Specification { ) object testNodeInfoService extends NodeInfoService { - def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition) : IOResult[Seq[NodeInfo]] = ??? + def getLDAPNodeInfo(nodeInfos: Seq[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Seq[NodeInfo]) : Seq[NodeInfo] = ??? def getNodeInfo(nodeId: NodeId) : IOResult[Option[NodeInfo]] = ??? def getNodeInfos(nodesId: Set[NodeId]) : IOResult[Set[NodeInfo]] = ??? def getNode(nodeId: NodeId): Box[Node] = ??? def getAllNodes() : IOResult[Map[NodeId, Node]] = ??? def getAllNodesIds(): IOResult[Set[NodeId]] = ??? + def getAllNodeInfos():IOResult[Seq[NodeInfo]] = ??? def getAllSystemNodeIds() : IOResult[Seq[NodeId]] = ??? def getPendingNodeInfos(): IOResult[Map[NodeId, NodeInfo]] = ??? def getPendingNodeInfo(nodeId: NodeId): IOResult[Option[NodeInfo]] = ??? diff --git a/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala b/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala index 85be43245d9..2ac07109a8a 100644 --- a/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala +++ b/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala @@ -79,7 +79,6 @@ import com.normation.rudder.domain.queries.CriterionComposition import com.normation.rudder.domain.queries.NodeInfoMatcher import com.normation.rudder.repository.RoRuleRepository import com.normation.rudder.repository.WoRuleRepository -import com.normation.rudder.services.nodes.LDAPNodeInfo import com.normation.rudder.services.nodes.NodeInfoService import com.normation.rudder.services.policies.NodeConfiguration import com.normation.rudder.services.policies.ParameterForConfiguration @@ -1489,6 +1488,7 @@ z5VEb9yx2KikbWyChM1Akp82AV5BzqE80QIBIw== } override def getAllNodes(): IOResult[Map[NodeId, Node]] = getAll().map(_.map(kv => (kv._1, kv._2.node))) + override def getAllNodeInfos():IOResult[Seq[NodeInfo]] = getAll().map(_.values.toSeq) override def getAllNodesIds(): IOResult[Set[NodeId]] = getAllNodes().map(_.keySet) override def getAllSystemNodeIds(): IOResult[Seq[NodeId]] = { nodeBase.get.map(_.collect { case (id, n) if(n.info.isSystem) => id }.toSeq ) @@ -1513,7 +1513,7 @@ z5VEb9yx2KikbWyChM1Akp82AV5BzqE80QIBIw== override def getAllNodeInventories(inventoryStatus: InventoryStatus): IOResult[Map[NodeId, NodeInventory]] = getGenericAll(inventoryStatus, _fullInventory(_).map(_.node)) // not implemented yet - override def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition): IOResult[Seq[NodeInfo]] = ??? + override def getLDAPNodeInfo(nodeInfos: Seq[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Seq[NodeInfo]): Seq[NodeInfo] = ??? override def getNumberOfManagedNodes: Int = ??? override def save(serverAndMachine: FullInventory): IOResult[Seq[LDIFChangeRecord]] = ??? override def delete(id: NodeId, inventoryStatus: InventoryStatus): IOResult[Seq[LDIFChangeRecord]] = ??? diff --git a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala index f85bd1154b9..2af28d33257 100644 --- a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala +++ b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala @@ -1666,7 +1666,8 @@ object RudderConfig extends Loggable { // here, we don't want to look for subgroups to show them in the form => always return an empty list , new DitQueryData(pendingNodesDitImpl, nodeDit, rudderDit, () => Nil.succeed) , ldapEntityMapper - ) + ), + nodeInfoServiceImpl ) private[this] lazy val dynGroupServiceImpl = new DynGroupServiceImpl(rudderDitImpl, roLdap, ldapEntityMapper) diff --git a/webapp/sources/rudder/rudder-web/src/main/scala/com/normation/rudder/web/components/SearchNodeComponent.scala b/webapp/sources/rudder/rudder-web/src/main/scala/com/normation/rudder/web/components/SearchNodeComponent.scala index d17b98f23ea..5e0355423ac 100644 --- a/webapp/sources/rudder/rudder-web/src/main/scala/com/normation/rudder/web/components/SearchNodeComponent.scala +++ b/webapp/sources/rudder/rudder-web/src/main/scala/com/normation/rudder/web/components/SearchNodeComponent.scala @@ -610,7 +610,7 @@ object SearchNodeComponent { val defaultLine : CriterionLine = { //in case of further modification in ditQueryData require(ditQueryData.criteriaMap(OC_NODE).criteria(0).name == "OS", "Error in search node criterion default line, did you change DitQueryData ?") - require(ditQueryData.criteriaMap(OC_NODE).criteria(0).cType.isInstanceOf[OstypeComparator.type], "Error in search node criterion default line, did you change DitQueryData ?") + require(ditQueryData.criteriaMap(OC_NODE).criteria(0).cType.isInstanceOf[NodeOstypeComparator.type], "Error in search node criterion default line, did you change DitQueryData ?") CriterionLine( objectType = ditQueryData.criteriaMap(OC_NODE) , attribute = ditQueryData.criteriaMap(OC_NODE).criteria(0) diff --git a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala index cff5eca64aa..eccb32a371e 100644 --- a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala +++ b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala @@ -164,12 +164,13 @@ class TestMigrateSystemTechniques7_0 extends Specification { val nodeInfoService = new NodeInfoService { override def getAll(): IOResult[Map[NodeId, NodeInfo]] = List(root, relay1).map(x => (x.id, x)).toMap.succeed - override def getLDAPNodeInfo(nodeIds: Set[NodeId], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition): IOResult[Seq[NodeInfo]] = ??? + override def getLDAPNodeInfo(foundNodeInfos: Seq[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Seq[NodeInfo]): Seq[NodeInfo] = ??? override def getNodeInfo(nodeId: NodeId): IOResult[Option[NodeInfo]] = ??? override def getNodeInfos(nodeIds: Set[NodeId]): IOResult[Set[NodeInfo]] = ??? override def getNumberOfManagedNodes: Int = ??? override def getAllNodesIds(): IOResult[Set[NodeId]] = ??? override def getAllNodes(): IOResult[Map[NodeId, Node]] = ??? + override def getAllNodeInfos(): IOResult[Seq[NodeInfo]] = ??? override def getAllSystemNodeIds(): IOResult[Seq[NodeId]] = ??? override def getPendingNodeInfos(): IOResult[Map[NodeId, NodeInfo]] = ??? override def getPendingNodeInfo(nodeId: NodeId): IOResult[Option[NodeInfo]] = ??? From 63f288d2a96046d8404b1729106bd5eccda51477 Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Mon, 7 Feb 2022 16:44:35 +0100 Subject: [PATCH 06/14] fixup! fixup! fixup! fixup! fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed --- .../normation/rudder/services/queries/LdapQueryProcessor.scala | 2 -- 1 file changed, 2 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala index 35a18142036..960deb98033 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala @@ -263,8 +263,6 @@ object PostFilterNodeFromInfoService { val validPredicates = combined.matches(nodeInfo) val res = comp(contains, validPredicates) - println(s"${nodeInfo.id.value}: ${if(res) "OK" else "NOK"} for [${combined.debugString}]") - if(logger.isTraceEnabled()) { logger.trace(s"${nodeInfo.id.value}: ${if(res) "OK" else "NOK"} for [${combined.debugString}]") } From 1246f0a8bcbb3118e1cc7332016f9d9d321b6f19 Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Mon, 7 Feb 2022 17:22:56 +0100 Subject: [PATCH 07/14] fixup! fixup! fixup! fixup! fixup! fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed --- .../services/nodes/NodeInfoService.scala | 12 ++-- .../services/queries/LdapQueryProcessor.scala | 56 +++++++++++-------- .../services/queries/QueryProcessor.scala | 6 +- .../jdbc/ReportingServiceTest.scala | 2 +- .../services/queries/TestQueryProcessor.scala | 4 +- .../CachedFindRuleNodeStatusReportsTest.scala | 4 +- .../normation/rudder/rest/lift/NodeApi.scala | 2 +- .../com/normation/rudder/MockServices.scala | 2 +- .../TestMigrateSystemTechnique7_0.scala | 4 +- 9 files changed, 51 insertions(+), 41 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala index 89b2eff24bf..09dea405730 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala @@ -117,7 +117,7 @@ trait NodeInfoService { * foundNodeInfos are the nodeinfo found by ldap query * allNodeInfos are all the know nodeInfos to date */ - def getLDAPNodeInfo(foundNodeInfos: Seq[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodeInfos: Seq[NodeInfo]): Seq[NodeInfo] + def getLDAPNodeInfo(foundNodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodeInfos: Set[NodeInfo]): Set[NodeInfo] /** * Return a NodeInfo from a NodeId. First check the ou=Node, then fetch the other data @@ -159,7 +159,7 @@ trait NodeInfoService { */ def getAllNodes() : IOResult[Map[NodeId, Node]] - def getAllNodeInfos():IOResult[Seq[NodeInfo]] + def getAllNodeInfos():IOResult[Set[NodeInfo]] /** * Get all systen node ids, for example * policy server node ids. @@ -870,14 +870,14 @@ trait NodeInfoServiceCached extends NodeInfoService with NamedZioLogger with Cac cache.view.mapValues(_._2.node).toMap.succeed } - def getAllNodeInfos():IOResult[Seq[NodeInfo]] = withUpToDateCache("all nodeinfos") { cache => - cache.values.map(_._2).toSeq.succeed + def getAllNodeInfos():IOResult[Set[NodeInfo]] = withUpToDateCache("all nodeinfos") { cache => + cache.values.map(_._2).toSet.succeed } - override def getLDAPNodeInfo(foundNodeInfos: Seq[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodeInfos: Seq[NodeInfo]): Seq[NodeInfo] = { + override def getLDAPNodeInfo(foundNodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodeInfos: Set[NodeInfo]): Set[NodeInfo] = { // if nodeIds is empty and composition is and, return an empty set; with or, we need to run it in all cases if (foundNodeInfos.isEmpty && composition == And) { - Seq[NodeInfo]() + Set[NodeInfo]() } else { PostFilterNodeFromInfoService.getLDAPNodeInfo(foundNodeInfos, predicates, composition, allNodeInfos) } diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala index 960deb98033..c87be8f2a3d 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala @@ -124,7 +124,7 @@ final case class SubQuery(subQueryId: String, dnType: DnType, objectTypeName: St final case class LdapQueryProcessorResult( // list of NodeInfo matching the search - entries : Seq[NodeInfo] + entries : Set[NodeInfo] // a post filter to run on node info , nodeFilters: Seq[NodeInfoMatcher] ) @@ -168,7 +168,7 @@ class AcceptedNodesLDAPQueryProcessor( query:QueryTrait, select:Seq[String], limitToNodeIds:Option[Seq[NodeId]] - ) : Box[Seq[NodeInfo]] = { + ) : Box[Set[NodeInfo]] = { val debugId = if(logger.isDebugEnabled) Helpers.nextNum else 0L val timePreCompute = System.currentTimeMillis @@ -203,19 +203,19 @@ class AcceptedNodesLDAPQueryProcessor( logger.debug("[%s] [post-filter:policyServer] %s results".format(debugId, withoutServerRole.size, filtered.mkString(", "))) } } - withoutServerRole.toSeq - case NodeAndPolicyServerReturnType => nodesInfos.toSeq + withoutServerRole + case NodeAndPolicyServerReturnType => nodesInfos } } } - override def process(query:QueryTrait) : Box[Seq[NodeInfo]] = { + override def process(query:QueryTrait) : Box[Set[NodeInfo]] = { //only keep the one of the form Full(...) queryAndChekNodeId(query, NodeInfoService.nodeInfoAttributes, None) } - override def processOnlyId(query:QueryTrait) : Box[Seq[NodeId]] = { + override def processOnlyId(query:QueryTrait) : Box[Set[NodeId]] = { //only keep the one of the form Full(...) queryAndChekNodeId(query, Seq(A_NODE_UUID), None).map(seq => seq.map( y => y.node.id)) } @@ -230,16 +230,16 @@ class AcceptedNodesLDAPQueryProcessor( object PostFilterNodeFromInfoService { val logger = LoggerFactory.getLogger("com.normation.rudder.services.queries") def getLDAPNodeInfo( - foundNodeInfos: Seq[NodeInfo] // the one from the search + foundNodeInfos: Set[NodeInfo] // the one from the search, need to be a Set for fast "contain" , predicates : Seq[NodeInfoMatcher] , composition : CriterionComposition - , allNodesInfos : Seq[NodeInfo] // all the nodeinfo there is - ): Seq[NodeInfo] = { + , allNodesInfos : Set[NodeInfo] // all the nodeinfo there is + ): Set[NodeInfo] = { def comp(a: Boolean, b: Boolean) = composition match { case And => a && b case Or => a || b } - // utliity to combine predicates according to comp + // utility to combine predicates according to comp def combine(a: NodeInfoMatcher, b: NodeInfoMatcher) = new NodeInfoMatcher { override val debugString = { val c = composition match { @@ -252,14 +252,20 @@ object PostFilterNodeFromInfoService { } val foundNodeIds = foundNodeInfos.map(_.id) + val combined = predicates.reduceLeft(combine) + // if there is no predicates (ie no specific filter on NodeInfo), we should just keep nodes from our list - def predicate(nodeInfo : NodeInfo, pred: Seq[NodeInfoMatcher]) = { - val contains = foundNodeIds.contains(nodeInfo.id) + def predicate(nodeInfo : NodeInfo, pred: Seq[NodeInfoMatcher], composition: CriterionComposition) = { + val contains = composition match { + case Or => foundNodeIds.contains(nodeInfo.id) // we combine with all + case And => true // we combine with all + + } + if (pred.isEmpty) { // in that case, whatever the query composition, we can only return what was already found. contains } else { - val combined = pred.reduceLeft(combine) val validPredicates = combined.matches(nodeInfo) val res = comp(contains, validPredicates) @@ -270,7 +276,11 @@ object PostFilterNodeFromInfoService { } } - allNodesInfos.collect { case nodeinfo if predicate(nodeinfo, predicates) => nodeinfo } + composition match { + case Or => allNodesInfos.collect { case nodeinfo if predicate(nodeinfo, predicates, composition) => nodeinfo } + case And => foundNodeInfos.collect { case nodeinfo if predicate(nodeinfo, predicates, composition) => nodeinfo } + } + } } @@ -285,26 +295,26 @@ class PendingNodesLDAPQueryChecker( , nodeInfoService: NodeInfoService ) extends QueryChecker with Loggable { - override def check(query:QueryTrait, limitToNodeIds:Option[Seq[NodeId]]) : Box[Seq[NodeId]] = { + override def check(query:QueryTrait, limitToNodeIds:Option[Seq[NodeId]]) : Box[Set[NodeId]] = { if(query.criteria.isEmpty) { LoggerFactory.getILoggerFactory.getLogger(Logger.loggerNameFor(classOf[InternalLDAPQueryProcessor])).debug( s"Checking a query with 0 criterium will always lead to 0 nodes: ${query}" ) - Full(Seq.empty[NodeId]) + Full(Set.empty[NodeId]) } else { val timePreCompute = System.currentTimeMillis for { // get the pending node onfos we are considering allPendingNodeInfos <- nodeInfoService.getPendingNodeInfos().toBox pendingNodeInfos = limitToNodeIds match { - case None => allPendingNodeInfos.values.toSeq - case Some(ids) => allPendingNodeInfos.collect { case (nodeId, nodeInfo) if ids.contains(nodeId) => nodeInfo}.toSeq + case None => allPendingNodeInfos.values.toSet + case Some(ids) => allPendingNodeInfos.collect { case (nodeId, nodeInfo) if ids.contains(nodeId) => nodeInfo}.toSet } res <- checker.internalQueryProcessor(query, Seq("1.1"), limitToNodeIds, 0, pendingNodeInfos).toBox timeres = (System.currentTimeMillis - timePreCompute) _ = logger.debug(s"LDAP result: ${res.entries.size} entries in pending nodes obtained in ${timeres}ms for query ${query.toString}") - nodesInfos = nodeInfoService.getLDAPNodeInfo(res.entries, res.nodeFilters, query.composition, pendingNodeInfos) + nodesInfos = nodeInfoService.getLDAPNodeInfo(res.entries.toSet, res.nodeFilters, query.composition, pendingNodeInfos) filterTime = (System.currentTimeMillis - timePreCompute - timeres) _ = logger.debug(s"[post-filter:rudderNode] Found ${nodesInfos.size} nodes when filtering pending nodes for info service existence and properties (${filterTime} ms)") @@ -356,7 +366,7 @@ class InternalLDAPQueryProcessor( , select : Seq[String] = Seq() , limitToNodeIds: Option[Seq[NodeId]] = None , debugId : Long = 0L - , allNodeInfos : Seq[NodeInfo] // this is hackish, to have the list of all node if + , allNodeInfos : Set[NodeInfo] // this is hackish, to have the list of all node if // only nodeinfofilters, so that we don't look for all // it is always called, no need to be a lambda ) : IOResult[LdapQueryProcessorResult] = { @@ -495,7 +505,7 @@ class InternalLDAPQueryProcessor( case None if nq.noFilterButTakeAllFromCache => allNodeInfos.succeed case None => - Seq[NodeInfo]().succeed + Set[NodeInfo]().succeed case Some(dms) => (for { // Ok, do the computation here @@ -534,7 +544,7 @@ class InternalLDAPQueryProcessor( // distinctBy computes the hashcode of the object // It is really expensive on LDAP entries. // The dn string is already computed, so the toString is a good alternative (and as also unicity) - postFiltered = postFilterNode(inverted.distinctBy(_.node.id.value), query.returnType, limitToNodeIds) + postFiltered = postFilterNode(inverted, query.returnType, limitToNodeIds) } yield { LdapQueryProcessorResult(postFiltered, nq.nodeInfoFilters) } @@ -547,7 +557,7 @@ class InternalLDAPQueryProcessor( * that in `queryAndChekNodeId` where we know about server roles * - step2: filter out nodes based on a given list of acceptable entries */ - private[this] def postFilterNode(entries: Seq[NodeInfo], returnType: QueryReturnType, limitToNodeIds:Option[Seq[NodeId]]) : Seq[NodeInfo] = { + private[this] def postFilterNode(entries: Set[NodeInfo], returnType: QueryReturnType, limitToNodeIds:Option[Seq[NodeId]]) : Set[NodeInfo] = { // keeping this for backport option to 6.2 val step1 = returnType match { //actually, we are able at that point to know if we have a policy server, diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/QueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/QueryProcessor.scala index 72c45b864c0..fb14f2371e6 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/QueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/QueryProcessor.scala @@ -50,13 +50,13 @@ trait QueryProcessor { * @param select - attributes to fetch in the ldap entry. If empty, all attributes are fetched * @return */ - def process(query:QueryTrait) : Box[Seq[NodeInfo]] + def process(query:QueryTrait) : Box[Set[NodeInfo]] /** * Only get node ids corresponding to that request, with minimal consistency check. * This method is useful to maximize performance (low memory, high throughout) for ex for dynamic groups. */ - def processOnlyId(query:QueryTrait) : Box[Seq[NodeId]] + def processOnlyId(query:QueryTrait) : Box[Set[NodeId]] } @@ -74,6 +74,6 @@ trait QueryChecker { * Full(seq) with seq being the list of nodeId which verify * query. */ - def check(query:QueryTrait, nodeIds:Option[Seq[NodeId]]) : Box[Seq[NodeId]] + def check(query:QueryTrait, nodeIds:Option[Seq[NodeId]]) : Box[Set[NodeId]] } diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala index 7d9a060e9e3..a7a9b61a1e0 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala @@ -95,7 +95,7 @@ class ReportingServiceTest extends DBCommon with BoxSpecMatcher { } object nodeInfoService extends NodeInfoService { - def getLDAPNodeInfo(foundNodeInfos: Seq[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodeInfos: Seq[NodeInfo]): Seq[NodeInfo]= ??? + def getLDAPNodeInfo(foundNodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodeInfos: Set[NodeInfo]): Set[NodeInfo]= ??? def getNodeInfo(nodeId: NodeId) : IOResult[Option[NodeInfo]] = ??? def getNodeInfos(nodesId: Set[NodeId]) : IOResult[Set[NodeInfo]] = ??? def getNode(nodeId: NodeId): Box[Node] = ??? diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala index 2d329e8f69c..6e716868ee8 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala @@ -1217,7 +1217,7 @@ class TestQueryProcessor extends Loggable { private def testQueryResultProcessor(name:String,query:QueryTrait, nodes:Seq[NodeId], doInternalQueryTest : Boolean) = { val ids = nodes.sortBy( _.value ) - val found = queryProcessor.process(query).openOrThrowException("For tests").map { _.id }.sortBy( _.value ) + val found = queryProcessor.process(query).openOrThrowException("For tests").map { _.id }.toSeq.sortBy( _.value ) //also test with requiring only the expected node to check consistancy //(that should not change anything) @@ -1242,7 +1242,7 @@ class TestQueryProcessor extends Loggable { val foundWithLimit = (internalLDAPQueryProcessor.internalQueryProcessor(query, limitToNodeIds = Some(ids), allNodeInfos= allNodesInfos).runNow.entries.map { _.node.id - }).distinct.sortBy( _.value ) + }).toSeq.distinct.sortBy( _.value ) assertEquals( s"[${name}] Size differs between expected and found entries (InternalQueryProcessor, only inventory fields)\n Found: ${foundWithLimit}\n Expected: ${ids}" diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala index 1e3d377ebc1..781a4901837 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala @@ -132,13 +132,13 @@ class CachedFindRuleNodeStatusReportsTest extends Specification { ) object testNodeInfoService extends NodeInfoService { - def getLDAPNodeInfo(nodeInfos: Seq[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Seq[NodeInfo]) : Seq[NodeInfo] = ??? + def getLDAPNodeInfo(nodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Set[NodeInfo]) : Seq[NodeInfo] = ??? def getNodeInfo(nodeId: NodeId) : IOResult[Option[NodeInfo]] = ??? def getNodeInfos(nodesId: Set[NodeId]) : IOResult[Set[NodeInfo]] = ??? def getNode(nodeId: NodeId): Box[Node] = ??? def getAllNodes() : IOResult[Map[NodeId, Node]] = ??? def getAllNodesIds(): IOResult[Set[NodeId]] = ??? - def getAllNodeInfos():IOResult[Seq[NodeInfo]] = ??? + def getAllNodeInfos():IOResult[Set[NodeInfo]] = ??? def getAllSystemNodeIds() : IOResult[Seq[NodeId]] = ??? def getPendingNodeInfos(): IOResult[Map[NodeId, NodeInfo]] = ??? def getPendingNodeInfo(nodeId: NodeId): IOResult[Option[NodeInfo]] = ??? diff --git a/webapp/sources/rudder/rudder-rest/src/main/scala/com/normation/rudder/rest/lift/NodeApi.scala b/webapp/sources/rudder/rudder-rest/src/main/scala/com/normation/rudder/rest/lift/NodeApi.scala index 5b031c30b22..dc70e142d21 100644 --- a/webapp/sources/rudder/rudder-rest/src/main/scala/com/normation/rudder/rest/lift/NodeApi.scala +++ b/webapp/sources/rudder/rudder-rest/src/main/scala/com/normation/rudder/rest/lift/NodeApi.scala @@ -1054,7 +1054,7 @@ class NodeApiService6 ( case _ => Failure(s"Invalid branch used for nodes query, expected either AcceptedInventory or PendingInventory, got ${state}") } } yield { - listNodes(state,detailLevel,Some(nodeIds),version) + listNodes(state,detailLevel,Some(nodeIds.toSeq),version) } ) match { case Full(resp) => { diff --git a/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala b/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala index 2ac07109a8a..18b7ca249c0 100644 --- a/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala +++ b/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala @@ -1513,7 +1513,7 @@ z5VEb9yx2KikbWyChM1Akp82AV5BzqE80QIBIw== override def getAllNodeInventories(inventoryStatus: InventoryStatus): IOResult[Map[NodeId, NodeInventory]] = getGenericAll(inventoryStatus, _fullInventory(_).map(_.node)) // not implemented yet - override def getLDAPNodeInfo(nodeInfos: Seq[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Seq[NodeInfo]): Seq[NodeInfo] = ??? + override def getLDAPNodeInfo(nodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Set[NodeInfo]): Set[NodeInfo] = ??? override def getNumberOfManagedNodes: Int = ??? override def save(serverAndMachine: FullInventory): IOResult[Seq[LDIFChangeRecord]] = ??? override def delete(id: NodeId, inventoryStatus: InventoryStatus): IOResult[Seq[LDIFChangeRecord]] = ??? diff --git a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala index eccb32a371e..04d2cfbccb2 100644 --- a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala +++ b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala @@ -164,13 +164,13 @@ class TestMigrateSystemTechniques7_0 extends Specification { val nodeInfoService = new NodeInfoService { override def getAll(): IOResult[Map[NodeId, NodeInfo]] = List(root, relay1).map(x => (x.id, x)).toMap.succeed - override def getLDAPNodeInfo(foundNodeInfos: Seq[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Seq[NodeInfo]): Seq[NodeInfo] = ??? + override def getLDAPNodeInfo(foundNodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Set[NodeInfo]): Set[NodeInfo] = ??? override def getNodeInfo(nodeId: NodeId): IOResult[Option[NodeInfo]] = ??? override def getNodeInfos(nodeIds: Set[NodeId]): IOResult[Set[NodeInfo]] = ??? override def getNumberOfManagedNodes: Int = ??? override def getAllNodesIds(): IOResult[Set[NodeId]] = ??? override def getAllNodes(): IOResult[Map[NodeId, Node]] = ??? - override def getAllNodeInfos(): IOResult[Seq[NodeInfo]] = ??? + override def getAllNodeInfos(): IOResult[Set[NodeInfo]] = ??? override def getAllSystemNodeIds(): IOResult[Seq[NodeId]] = ??? override def getPendingNodeInfos(): IOResult[Map[NodeId, NodeInfo]] = ??? override def getPendingNodeInfo(nodeId: NodeId): IOResult[Option[NodeInfo]] = ??? From de991f04427cd89f9d6a5543ac7f3af912fa5d4b Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Mon, 7 Feb 2022 22:11:38 +0100 Subject: [PATCH 08/14] fixup! fixup! fixup! fixup! fixup! fixup! fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed --- .../queries/DynGroupUpdaterService.scala | 4 +--- .../services/queries/LdapQueryProcessor.scala | 24 +++++++++++++++---- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/DynGroupUpdaterService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/DynGroupUpdaterService.scala index 1ee7b05f2b9..5a7cb7bcec6 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/DynGroupUpdaterService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/DynGroupUpdaterService.scala @@ -98,12 +98,10 @@ class DynGroupUpdaterServiceImpl( timePreCompute = System.currentTimeMillis query <- Box(group.query) ?~! s"No query defined for group '${group.name}' (${group.id.value})" newMembers <- queryProcessor.processOnlyId(query) ?~! s"Error when processing request for updating dynamic group '${group.name}' (${group.id.value})" - //save - newMemberIdsSet = newMembers.toSet timeGroupCompute = (System.currentTimeMillis - timePreCompute) _ = logger.debug(s"Dynamic group ${group.id.value} with name ${group.name} computed in ${timeGroupCompute} ms") } yield { - group.copy(serverList = newMemberIdsSet) + group.copy(serverList = newMembers) } } diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala index c87be8f2a3d..4fabc930f70 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala @@ -173,8 +173,17 @@ class AcceptedNodesLDAPQueryProcessor( val debugId = if(logger.isDebugEnabled) Helpers.nextNum else 0L val timePreCompute = System.currentTimeMillis + var initedCache : Option[Box[Set[NodeInfo]]] = None + def lambdaTest() = { + initedCache match { + case Some(value) => value + case None => initedCache = Some((nodeInfoService.getAllNodeInfos().toBox)) + initedCache.get + } + } + for { - allNodeInfos <- nodeInfoService.getAllNodeInfos().toBox + allNodeInfos <- nodeInfoService.getAllNodeInfos().toBox // TODO: make it a lambda again, but a clever one (with a cache) res <- processor.internalQueryProcessor(query,select,limitToNodeIds,debugId, allNodeInfos).toBox timeres = (System.currentTimeMillis - timePreCompute) _ = logger.debug(s"LDAP result: ${res.entries.size} entries obtained in ${timeres}ms for query ${query.toString}") @@ -259,7 +268,6 @@ object PostFilterNodeFromInfoService { val contains = composition match { case Or => foundNodeIds.contains(nodeInfo.id) // we combine with all case And => true // we combine with all - } if (pred.isEmpty) { @@ -277,8 +285,15 @@ object PostFilterNodeFromInfoService { } composition match { + // TODO: there is surely something we can do here case Or => allNodesInfos.collect { case nodeinfo if predicate(nodeinfo, predicates, composition) => nodeinfo } - case And => foundNodeInfos.collect { case nodeinfo if predicate(nodeinfo, predicates, composition) => nodeinfo } + case And => + if (predicates.isEmpty) { + // paththru + foundNodeInfos + } else { + foundNodeInfos.collect { case nodeinfo if predicate(nodeinfo, predicates, composition) => nodeinfo } + } } } @@ -314,7 +329,7 @@ class PendingNodesLDAPQueryChecker( timeres = (System.currentTimeMillis - timePreCompute) _ = logger.debug(s"LDAP result: ${res.entries.size} entries in pending nodes obtained in ${timeres}ms for query ${query.toString}") - nodesInfos = nodeInfoService.getLDAPNodeInfo(res.entries.toSet, res.nodeFilters, query.composition, pendingNodeInfos) + nodesInfos = nodeInfoService.getLDAPNodeInfo(res.entries, res.nodeFilters, query.composition, pendingNodeInfos) filterTime = (System.currentTimeMillis - timePreCompute - timeres) _ = logger.debug(s"[post-filter:rudderNode] Found ${nodesInfos.size} nodes when filtering pending nodes for info service existence and properties (${filterTime} ms)") @@ -533,6 +548,7 @@ class InternalLDAPQueryProcessor( tap(seq => logPure.debug(s"[${debugId}] `-> ${seq.size} results")) } // No more LDAP query is required here + // These transformation should be after nodeInfoService.getLDAPNodeInfo inverted = query.transform match { case ResultTransformation.Identity => results case ResultTransformation.Invert => From b4efc0b860ff3af39e6c2932a92a2cc4b4e8381d Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Tue, 8 Feb 2022 09:17:38 +0100 Subject: [PATCH 09/14] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed --- .../services/queries/LdapQueryProcessor.scala | 114 ++++++++++++------ .../jdbc/ReportingServiceTest.scala | 2 +- .../queries/TestPendingNodePolicies.scala | 4 +- .../services/queries/TestQueryProcessor.scala | 3 +- .../CachedFindRuleNodeStatusReportsTest.scala | 2 +- .../web/components/SearchNodeComponent.scala | 2 +- 6 files changed, 83 insertions(+), 44 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala index 4fabc930f70..53aa053bea4 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala @@ -62,6 +62,7 @@ import com.normation.errors._ import zio._ import zio.syntax._ import com.normation.ldap.sdk.syntax._ +import com.normation.zio.currentTimeMillis /* * We have two type of filters: @@ -173,47 +174,40 @@ class AcceptedNodesLDAPQueryProcessor( val debugId = if(logger.isDebugEnabled) Helpers.nextNum else 0L val timePreCompute = System.currentTimeMillis - var initedCache : Option[Box[Set[NodeInfo]]] = None - def lambdaTest() = { - initedCache match { - case Some(value) => value - case None => initedCache = Some((nodeInfoService.getAllNodeInfos().toBox)) - initedCache.get - } - } +// var initedCache : Option[IOResult[Set[NodeInfo]]] = None +// def lambdaTest() = { +// initedCache match { +// case Some(value) => value +// case None => initedCache = Some((nodeInfoService.getAllNodeInfos().toBox)) +// initedCache.get +// } +// } for { - allNodeInfos <- nodeInfoService.getAllNodeInfos().toBox // TODO: make it a lambda again, but a clever one (with a cache) - res <- processor.internalQueryProcessor(query,select,limitToNodeIds,debugId, allNodeInfos).toBox + // allNodeInfos <- nodeInfoService.getAllNodeInfos().toBox // TODO: make it a lambda again, but a clever one (with a cache) + res <- processor.internalQueryProcessor(query,select,limitToNodeIds,debugId, () => nodeInfoService.getAllNodeInfos).toBox timeres = (System.currentTimeMillis - timePreCompute) _ = logger.debug(s"LDAP result: ${res.entries.size} entries obtained in ${timeres}ms for query ${query.toString}") - nodesInfos = nodeInfoService.getLDAPNodeInfo(res.entries, res.nodeFilters, query.composition, allNodeInfos) - filterTime = (System.currentTimeMillis - timePreCompute - timeres) - _ = logger.debug(s"[post-filter:rudderNode] Found ${nodesInfos.size} nodes when filtering for info service existence and properties (${filterTime} ms)") + // nodesInfos = nodeInfoService.getLDAPNodeInfo(res.entries, res.nodeFilters, query.composition, allNodeInfos) + // filterTime = (System.currentTimeMillis - timePreCompute - timeres) + // _ = logger.debug(s"[post-filter:rudderNode] Found ${nodesInfos.size} nodes when filtering for info service existence and properties (${filterTime} ms)") } yield { - if(logger.isDebugEnabled) { - val filtered = res.entries.map( x => x.node.id.value).diff(nodesInfos.map( x => x.node.id.value)) - if(filtered.nonEmpty) { - logger.debug(s"[${debugId}] [post-filter:rudderNode] ${nodesInfos.size} results (following nodes not in ou=Nodes,cn=rudder-configuration or not matching filters on NodeInfo: ${filtered.mkString(", ")}") - } - } - - //filter out Rudder server component if necessary + //filter out Rudder server component if necessary query.returnType match { case NodeReturnType => // we have a special case for the root node that always never to that group, even if some weird // scenario lead to the removal (or non addition) of them - val withoutServerRole = nodesInfos.filterNot { case x: NodeInfo => x.node.id.value ==("root") } + val withoutServerRole = res.entries.filterNot { case x: NodeInfo => x.node.id.value ==("root") } if(logger.isDebugEnabled) { - val filtered = nodesInfos.filter { case x: NodeInfo => x.node.id.value ==("root") } + val filtered = res.entries.filter { case x: NodeInfo => x.node.id.value ==("root") } if(!filtered.isEmpty) { logger.debug("[%s] [post-filter:policyServer] %s results".format(debugId, withoutServerRole.size, filtered.mkString(", "))) } } withoutServerRole - case NodeAndPolicyServerReturnType => nodesInfos + case NodeAndPolicyServerReturnType => res.entries } } } @@ -261,7 +255,7 @@ object PostFilterNodeFromInfoService { } val foundNodeIds = foundNodeInfos.map(_.id) - val combined = predicates.reduceLeft(combine) + // if there is no predicates (ie no specific filter on NodeInfo), we should just keep nodes from our list def predicate(nodeInfo : NodeInfo, pred: Seq[NodeInfoMatcher], composition: CriterionComposition) = { @@ -274,6 +268,7 @@ object PostFilterNodeFromInfoService { // in that case, whatever the query composition, we can only return what was already found. contains } else { + val combined = predicates.reduceLeft(combine) val validPredicates = combined.matches(nodeInfo) val res = comp(contains, validPredicates) @@ -325,16 +320,16 @@ class PendingNodesLDAPQueryChecker( case None => allPendingNodeInfos.values.toSet case Some(ids) => allPendingNodeInfos.collect { case (nodeId, nodeInfo) if ids.contains(nodeId) => nodeInfo}.toSet } - res <- checker.internalQueryProcessor(query, Seq("1.1"), limitToNodeIds, 0, pendingNodeInfos).toBox + res <- checker.internalQueryProcessor(query, Seq("1.1"), limitToNodeIds, 0, () => pendingNodeInfos.succeed).toBox timeres = (System.currentTimeMillis - timePreCompute) _ = logger.debug(s"LDAP result: ${res.entries.size} entries in pending nodes obtained in ${timeres}ms for query ${query.toString}") - nodesInfos = nodeInfoService.getLDAPNodeInfo(res.entries, res.nodeFilters, query.composition, pendingNodeInfos) - filterTime = (System.currentTimeMillis - timePreCompute - timeres) - _ = logger.debug(s"[post-filter:rudderNode] Found ${nodesInfos.size} nodes when filtering pending nodes for info service existence and properties (${filterTime} ms)") + // nodesInfos = nodeInfoService.getLDAPNodeInfo(res.entries, res.nodeFilters, query.composition, pendingNodeInfos) + //filterTime = (System.currentTimeMillis - timePreCompute - timeres) + //_ = logger.debug(s"[post-filter:rudderNode] Found ${nodesInfos.size} nodes when filtering pending nodes for info service existence and properties (${filterTime} ms)") } yield { - nodesInfos.map(_.node.id) + res.entries.map(_.node.id) } } } @@ -381,9 +376,8 @@ class InternalLDAPQueryProcessor( , select : Seq[String] = Seq() , limitToNodeIds: Option[Seq[NodeId]] = None , debugId : Long = 0L - , allNodeInfos : Set[NodeInfo] // this is hackish, to have the list of all node if + , lambdaAllNodeInfos : () => IOResult[Set[NodeInfo]] // this is hackish, to have the list of all node if // only nodeinfofilters, so that we don't look for all - // it is always called, no need to be a lambda ) : IOResult[LdapQueryProcessorResult] = { @@ -494,7 +488,8 @@ class InternalLDAPQueryProcessor( for { //log start query - _ <- logPure.debug(s"[${debugId}] Start search for ${query.toString}") + _ <- logPure.debug(s"[${debugId}] Start search for ${query.toString}") + timeStart <- currentTimeMillis // Construct & normalize the data nq <- normalizedQuery // special case: no query, but we create a dummy one, @@ -514,11 +509,27 @@ class InternalLDAPQueryProcessor( } } } + + // Fetching all node infos if necessary + // This is an optimisation procedue, as it is a bit costly to fetch it, so we want to + // have it only if the query is an OR, and Invertion, or and AND and there ae + // no LDAP criteria + allNodesInfos <- query.composition match { + case Or => lambdaAllNodeInfos() + case And if nq.noFilterButTakeAllFromCache => lambdaAllNodeInfos() + case And if query.transform == ResultTransformation.Invert => lambdaAllNodeInfos() + case And if optdms.isDefined => lambdaAllNodeInfos() + + case _ => Set[NodeInfo]().succeed + } + timefetch <- currentTimeMillis + _ <- logPure.debug(s"LDAP result: fetching if necessary all nodesInfos in in nodes obtained in ${timefetch-timeStart} ms for query ${query.toString}") + // If dnMapSets returns a None, then it means that we are ANDing composition with an empty value // so we skip the last query results <- optdms match { case None if nq.noFilterButTakeAllFromCache => - allNodeInfos.succeed + allNodesInfos.succeed case None => Set[NodeInfo]().succeed case Some(dms) => @@ -542,18 +553,47 @@ class InternalLDAPQueryProcessor( entries <- executeQuery(rt.baseDn, rt.scope, nodeObjectTypes.objectFilter, rt.filter, finalSpecialFilters, select.toSet, nq.composition, debugId) // convert these entries into nodeInfo nodesId = entries.flatMap(x => x(A_NODE_UUID)) - nodeInfos = allNodeInfos.filter(nodeInfo => nodesId.contains(nodeInfo.node.id.value)) + nodeInfos = allNodesInfos.filter(nodeInfo => nodesId.contains(nodeInfo.node.id.value)) } yield nodeInfos). tapError(err => logPure.debug(s"[${debugId}] `-> error: ${err.fullMsg}")). tap(seq => logPure.debug(s"[${debugId}] `-> ${seq.size} results")) } // No more LDAP query is required here + // Do the filtering about non LDAP data here + timeldap <- currentTimeMillis + _ <- logPure.debug(s"LDAP result: ${results.size} entries in nodes obtained in ${timeldap-timeStart} ms for query ${query.toString}") + + + nodeInfoFiltered = query.composition match { + case And if results.isEmpty => + // And and nothing returns nothing + Set[NodeInfo]() + case And => + // If i'm doing and AND, there is no need for the allNodes here + PostFilterNodeFromInfoService.getLDAPNodeInfo(results, nq.nodeInfoFilters, query.composition, Set()) + case Or => + // Here we need the list of all nodes + PostFilterNodeFromInfoService.getLDAPNodeInfo(results, nq.nodeInfoFilters, query.composition, allNodesInfos) + } + timefilter <- currentTimeMillis + _ <- logPure.debug(s"[post-filter:rudderNode] Found ${nodeInfoFiltered.size} nodes when filtering for info service existence and properties (${timefilter-timeldap} ms)") + _ <- logPure.ifDebugEnabled{ + val filtered = results.map( x => x.node.id.value).diff(nodeInfoFiltered.map( x => x.node.id.value)) + if(filtered.nonEmpty) { + logPure.debug(s"[${debugId}] [post-filter:rudderNode] ${nodeInfoFiltered.size} results (following nodes not in ou=Nodes,cn=rudder-configuration or not matching filters on NodeInfo: ${filtered.mkString(", ")}") + } else { + logPure.debug(s"[${debugId}] [post-filter:rudderNode] ${nodeInfoFiltered.size} results (following nodes not in ou=Nodes,cn=rudder-configuration or not matching filters on NodeInfo: ${filtered.mkString(", ")}") + + } + } + + // These transformation should be after nodeInfoService.getLDAPNodeInfo inverted = query.transform match { - case ResultTransformation.Identity => results + case ResultTransformation.Identity => nodeInfoFiltered case ResultTransformation.Invert => logEffect.debug(s"[${debugId}] |- (need to get all nodeIds for inversion) ") - val res = allNodeInfos.diff(results) + val res = allNodesInfos.diff(nodeInfoFiltered) logEffect.debug(s"[${debugId}] |- (invert) entries after inversion: ${res.size}") res } diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala index a7a9b61a1e0..25b0e336e6d 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala @@ -100,7 +100,7 @@ class ReportingServiceTest extends DBCommon with BoxSpecMatcher { def getNodeInfos(nodesId: Set[NodeId]) : IOResult[Set[NodeInfo]] = ??? def getNode(nodeId: NodeId): Box[Node] = ??? def getAllNodes() : IOResult[Map[NodeId, Node]] = ??? - def getAllNodeInfos():IOResult[Seq[NodeInfo]] = ??? + def getAllNodeInfos():IOResult[Set[NodeInfo]] = ??? def getAllNodesIds(): IOResult[Set[NodeId]] = ??? def getAllSystemNodeIds() : IOResult[Seq[NodeId]] = ??? def getPendingNodeInfos(): IOResult[Map[NodeId, NodeInfo]] = ??? diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestPendingNodePolicies.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestPendingNodePolicies.scala index bdd58e19c12..cdcf4260d4c 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestPendingNodePolicies.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestPendingNodePolicies.scala @@ -158,7 +158,7 @@ class TestPendingNodePolicies extends Specification { // a fake query checker val queryChecker = new QueryChecker { - override def check(query: QueryTrait, nodeIds: Option[Seq[NodeId]]): Box[Seq[NodeId]] = { + override def check(query: QueryTrait, nodeIds: Option[Seq[NodeId]]): Box[Set[NodeId]] = { // make a 0 criteria request raise an error like LDAP would do, // see: https://www.rudder-project.org/redmine/issues/12338 if(query.criteria.isEmpty) { @@ -168,7 +168,7 @@ class TestPendingNodePolicies extends Specification { case x if(x == dummyQuery0) => Set.empty[NodeId] case x if(x == dummyQuery1) => Set(node) case x => Set(node) - }).intersect(nodeIds.getOrElse(Seq(node)).toSet).toSeq) + }).intersect(nodeIds.getOrElse(Seq(node)).toSet)) } } } diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala index 6e716868ee8..59e1be5aaa3 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala @@ -1238,9 +1238,8 @@ class TestQueryProcessor extends Loggable { if (doInternalQueryTest) { logger.debug("Testing with expected entries, This test should be ignored when we are looking for Nodes with NodeInfo and inventory (ie when we are looking for property and environement variable") - val allNodesInfos = nodeInfoService.getAllNodeInfos().runNow val foundWithLimit = - (internalLDAPQueryProcessor.internalQueryProcessor(query, limitToNodeIds = Some(ids), allNodeInfos= allNodesInfos).runNow.entries.map { + (internalLDAPQueryProcessor.internalQueryProcessor(query, limitToNodeIds = Some(ids), lambdaAllNodeInfos = (() => nodeInfoService.getAllNodeInfos())).runNow.entries.map { _.node.id }).toSeq.distinct.sortBy( _.value ) diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala index 781a4901837..5c00f42fd68 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala @@ -132,7 +132,7 @@ class CachedFindRuleNodeStatusReportsTest extends Specification { ) object testNodeInfoService extends NodeInfoService { - def getLDAPNodeInfo(nodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Set[NodeInfo]) : Seq[NodeInfo] = ??? + def getLDAPNodeInfo(foundNodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Set[NodeInfo]) : Set[NodeInfo] = ??? def getNodeInfo(nodeId: NodeId) : IOResult[Option[NodeInfo]] = ??? def getNodeInfos(nodesId: Set[NodeId]) : IOResult[Set[NodeInfo]] = ??? def getNode(nodeId: NodeId): Box[Node] = ??? diff --git a/webapp/sources/rudder/rudder-web/src/main/scala/com/normation/rudder/web/components/SearchNodeComponent.scala b/webapp/sources/rudder/rudder-web/src/main/scala/com/normation/rudder/web/components/SearchNodeComponent.scala index 5e0355423ac..6130c3194b3 100644 --- a/webapp/sources/rudder/rudder-web/src/main/scala/com/normation/rudder/web/components/SearchNodeComponent.scala +++ b/webapp/sources/rudder/rudder-web/src/main/scala/com/normation/rudder/web/components/SearchNodeComponent.scala @@ -192,7 +192,7 @@ class SearchNodeComponent( query = Some(newQuery) if(errors.isEmpty) { // ********* EXECUTE QUERY *********** - srvList = queryProcessor.process(newQuery) + srvList = queryProcessor.process(newQuery).map(_.toSeq) initUpdate = true searchFormHasError = false } else { From f3e2a00ac4fd563b6f7c362876c97f7971e683b5 Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Tue, 8 Feb 2022 21:00:56 +0100 Subject: [PATCH 10/14] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed --- .../rudder/domain/queries/CmdbQuery.scala | 88 ++----------------- .../ldap/LDAPNodeGroupRepository.scala | 8 +- .../services/nodes/NodeInfoService.scala | 23 +---- .../services/queries/LdapQueryProcessor.scala | 72 ++++----------- .../jdbc/ReportingServiceTest.scala | 2 - .../services/queries/TestQueryProcessor.scala | 4 +- .../CachedFindRuleNodeStatusReportsTest.scala | 4 - .../com/normation/rudder/MockServices.scala | 10 +-- .../TestMigrateSystemTechnique7_0.scala | 4 - 9 files changed, 34 insertions(+), 181 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala index b0e0ffd512d..a0230b4d08b 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala @@ -51,7 +51,6 @@ import com.normation.rudder.domain.nodes.NodeGroupId import com.normation.rudder.domain.nodes.NodeInfo import com.normation.rudder.domain.nodes.NodeState import com.normation.rudder.domain.properties.NodeProperty -import com.normation.rudder.domain.queries.OstypeComparator.osTypes import com.normation.rudder.services.queries._ import com.normation.zio._ import com.unboundid.ldap.sdk._ @@ -223,6 +222,7 @@ final case object NodeStateComparator extends NodeCriterionType { } final case object NodeOstypeComparator extends NodeCriterionType { + val osTypes = List("AIX", "BSD", "Linux", "Solaris", "Windows") override def comparators = Seq(Equals, NotEquals) override protected def validateSubCase(v:String,comparator:CriterionComparator) = { if(null == v || v.isEmpty) Left(Inconsistency("Empty string not allowed")) else Right(v) @@ -237,7 +237,7 @@ final case object NodeOstypeComparator extends NodeCriterionType { override def toForm(value: String, func: String => Any, attrs: (String, String)*) : Elem = SHtml.select( - (osTypes map (e => (e,e))).toSeq + (osTypes map (e => (e,e))).toSeq , { if(osTypes.contains(value)) Full(value) else Empty} , func , attrs:_* @@ -249,10 +249,10 @@ final case object NodeOsNameComparator extends NodeCriterionType { import net.liftweb.http.S val osNames = AixOS :: - BsdType.allKnownTypes.sortBy { _.name } ::: - LinuxType.allKnownTypes.sortBy { _.name } ::: - (SolarisOS :: Nil) ::: - WindowsType.allKnownTypes + BsdType.allKnownTypes.sortBy { _.name } ::: + LinuxType.allKnownTypes.sortBy { _.name } ::: + (SolarisOS :: Nil) ::: + WindowsType.allKnownTypes override def comparators = Seq(Equals, NotEquals) @@ -643,82 +643,6 @@ final case object MachineComparator extends LDAPCriterionType { ) } -final case object OstypeComparator extends LDAPCriterionType { - val osTypes = List("AIX", "BSD", "Linux", "Solaris", "Windows") - override def comparators = Seq(Equals, NotEquals) - override protected def validateSubCase(v:String,comparator:CriterionComparator) = { - if(null == v || v.isEmpty) Left(Inconsistency("Empty string not allowed")) else Right(v) - } - override def toLDAP(value:String) = Right(value) - - override def buildFilter(attributeName:String,comparator:CriterionComparator,value:String) : Filter = { - val v = value match { - case "Windows" => OC_WINDOWS_NODE - case "Linux" => OC_LINUX_NODE - case "Solaris" => OC_SOLARIS_NODE - case "AIX" => OC_AIX_NODE - case "BSD" => OC_BSD_NODE - case _ => OC_UNIX_NODE - } - comparator match { - //for equals and not equals, check value for jocker - case Equals => IS(v) - case _ => NOT(IS(v)) - } - } - - override def toForm(value: String, func: String => Any, attrs: (String, String)*) : Elem = - SHtml.select( - (osTypes map (e => (e,e))).toSeq - , { if(osTypes.contains(value)) Full(value) else Empty} - , func - , attrs:_* - ) -} - -final case object OsNameComparator extends LDAPCriterionType { - import net.liftweb.http.S - - val osNames = AixOS :: - BsdType.allKnownTypes.sortBy { _.name } ::: - LinuxType.allKnownTypes.sortBy { _.name } ::: - (SolarisOS :: Nil) ::: - WindowsType.allKnownTypes - - override def comparators = Seq(Equals, NotEquals) - override protected def validateSubCase(v:String,comparator:CriterionComparator) = { - if(null == v || v.isEmpty) Left(Inconsistency("Empty string not allowed")) else Right(v) - } - override def toLDAP(value:String) = Right(value) - - override def buildFilter(attributeName:String,comparator:CriterionComparator,value:String) : Filter = { - val osName = comparator match { - //for equals and not equals, check value for jocker - case Equals => EQ(A_OS_NAME, value) - case _ => NOT(EQ(A_OS_NAME, value)) - } - AND(EQ(A_OC,OC_NODE),osName) - } - - private[this] def distribName(x: OsType): String = { - x match { - //add linux: for linux - case _: LinuxType => "Linux - " + S.?("os.name."+x.name) - case _: BsdType => "BSD - " + S.?("os.name."+x.name) - //nothing special for windows, Aix and Solaris - case _ => S.?("os.name."+x.name) - } - } - - override def toForm(value: String, func: String => Any, attrs: (String, String)*) : Elem = - SHtml.select( - osNames.map(e => (e.name,distribName(e))).toSeq, - {osNames.find(x => x.name == value).map( _.name)}, - func, - attrs:_* - ) -} - /* * Agent comparator is kind of scpecial, because it needs to accomodate to the following cases: * - historically, agent names were only "Nova" and "Community" (understood "cfengine", of course) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPNodeGroupRepository.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPNodeGroupRepository.scala index df79dfb93ce..4d5ee26df06 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPNodeGroupRepository.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPNodeGroupRepository.scala @@ -754,10 +754,8 @@ class WoLDAPNodeGroupRepository( oldGroup <- mapper.entry2NodeGroup(existing).toIO.chainError("Error when trying to check for the group %s".format(nodeGroup.id.value)) // check if old group is the same as new group result <- if (oldGroup.equals(nodeGroup)) { - logger.info(s"Nothing to update for ${nodeGroup.id}") None.succeed } else { - logger.info(s"Check what changed for for ${nodeGroup.id}") for { systemCheck <- if (onlyUpdateNodes) { oldGroup.succeed @@ -766,8 +764,8 @@ class WoLDAPNodeGroupRepository( case (false, true) => "You can not modify a non system group (%s) with that method".format(oldGroup.name).fail case _ => oldGroup.succeed } - name <- checkNameAlreadyInUse(con, nodeGroup.name, nodeGroup.id) - exists <- ZIO.when(name && !onlyUpdateNodes) { + name <- checkNameAlreadyInUse(con, nodeGroup.name, nodeGroup.id) + exists <- ZIO.when(name && !onlyUpdateNodes) { s"Cannot change the group name to ${nodeGroup.name} : there is already a group with the same name".fail } onlyNodes <- if (!onlyUpdateNodes) { @@ -780,7 +778,7 @@ class WoLDAPNodeGroupRepository( "The group configuration changed compared to the reference group you want to change the node list for. Aborting to preserve consistency".fail } } - change <- saveModifyNodeGroupDiff(existing, con, nodeGroup, modId, actor, reason) + change <- saveModifyNodeGroupDiff(existing, con, nodeGroup, modId, actor, reason) } yield { change } diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala index 09dea405730..e2d352116f2 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala @@ -55,8 +55,6 @@ import com.normation.rudder.repository.CachedRepository import com.normation.inventory.ldap.core.InventoryDit import com.normation.inventory.ldap.core.InventoryMapper import com.normation.rudder.domain.Constants -import com.normation.rudder.domain.queries.CriterionComposition -import com.normation.rudder.domain.queries.NodeInfoMatcher import com.normation.ldap.sdk.LDAPIOResult._ import zio._ import zio.syntax._ @@ -65,11 +63,9 @@ import com.normation.zio._ import com.normation.ldap.sdk.syntax._ import com.normation.rudder.domain.logger.NodeLoggerPure import com.normation.rudder.domain.logger.TimingDebugLoggerPure -import com.normation.rudder.domain.queries.And import com.normation.rudder.services.nodes.NodeInfoService.A_MOD_TIMESTAMP import com.normation.rudder.services.nodes.NodeInfoServiceCached.UpdatedNodeEntries import com.normation.rudder.services.nodes.NodeInfoServiceCached.buildInfoMaps -import com.normation.rudder.services.queries.PostFilterNodeFromInfoService import scala.concurrent.duration.FiniteDuration import scala.collection.mutable.{Map => MutMap} @@ -111,14 +107,6 @@ final case class LDAPNodeInfo( trait NodeInfoService { - /** - * Filter the nodeinfos that match the predicate & composition - don't rely on the cache - * used only by the LDAP QueryProcessor. - * foundNodeInfos are the nodeinfo found by ldap query - * allNodeInfos are all the know nodeInfos to date - */ - def getLDAPNodeInfo(foundNodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodeInfos: Set[NodeInfo]): Set[NodeInfo] - /** * Return a NodeInfo from a NodeId. First check the ou=Node, then fetch the other data */ @@ -871,16 +859,7 @@ trait NodeInfoServiceCached extends NodeInfoService with NamedZioLogger with Cac } def getAllNodeInfos():IOResult[Set[NodeInfo]] = withUpToDateCache("all nodeinfos") { cache => - cache.values.map(_._2).toSet.succeed - } - - override def getLDAPNodeInfo(foundNodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodeInfos: Set[NodeInfo]): Set[NodeInfo] = { - // if nodeIds is empty and composition is and, return an empty set; with or, we need to run it in all cases - if (foundNodeInfos.isEmpty && composition == And) { - Set[NodeInfo]() - } else { - PostFilterNodeFromInfoService.getLDAPNodeInfo(foundNodeInfos, predicates, composition, allNodeInfos) - } + cache.view.values.map(_._2).toSet.succeed } def getNodeInfo(nodeId: NodeId): IOResult[Option[NodeInfo]] = withUpToDateCache(s"${nodeId.value} node info") { cache => diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala index 53aa053bea4..54a4de23100 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala @@ -122,15 +122,6 @@ final case class LDAPNodeQuery( */ final case class SubQuery(subQueryId: String, dnType: DnType, objectTypeName: String, filters: Set[ExtendedFilter]) - -final case class LdapQueryProcessorResult( - // list of NodeInfo matching the search - entries : Set[NodeInfo] - // a post filter to run on node info - , nodeFilters: Seq[NodeInfoMatcher] -) - - case class RequestLimits ( val subRequestTimeLimit:Int, val subRequestSizeLimit:Int, @@ -174,40 +165,25 @@ class AcceptedNodesLDAPQueryProcessor( val debugId = if(logger.isDebugEnabled) Helpers.nextNum else 0L val timePreCompute = System.currentTimeMillis -// var initedCache : Option[IOResult[Set[NodeInfo]]] = None -// def lambdaTest() = { -// initedCache match { -// case Some(value) => value -// case None => initedCache = Some((nodeInfoService.getAllNodeInfos().toBox)) -// initedCache.get -// } -// } - for { - // allNodeInfos <- nodeInfoService.getAllNodeInfos().toBox // TODO: make it a lambda again, but a clever one (with a cache) - res <- processor.internalQueryProcessor(query,select,limitToNodeIds,debugId, () => nodeInfoService.getAllNodeInfos).toBox + foundNodes <- processor.internalQueryProcessor(query,select,limitToNodeIds,debugId, () => nodeInfoService.getAllNodeInfos).toBox timeres = (System.currentTimeMillis - timePreCompute) - _ = logger.debug(s"LDAP result: ${res.entries.size} entries obtained in ${timeres}ms for query ${query.toString}") - // nodesInfos = nodeInfoService.getLDAPNodeInfo(res.entries, res.nodeFilters, query.composition, allNodeInfos) - // filterTime = (System.currentTimeMillis - timePreCompute - timeres) - // _ = logger.debug(s"[post-filter:rudderNode] Found ${nodesInfos.size} nodes when filtering for info service existence and properties (${filterTime} ms)") - + _ = logger.debug(s"LDAP result: ${foundNodes.size} entries obtained in ${timeres}ms for query ${query.toString}") } yield { - //filter out Rudder server component if necessary - + //filter out Rudder server component if necessary query.returnType match { case NodeReturnType => // we have a special case for the root node that always never to that group, even if some weird // scenario lead to the removal (or non addition) of them - val withoutServerRole = res.entries.filterNot { case x: NodeInfo => x.node.id.value ==("root") } + val withoutServerRole = foundNodes.filterNot { case x: NodeInfo => x.node.id.value ==("root") } if(logger.isDebugEnabled) { - val filtered = res.entries.filter { case x: NodeInfo => x.node.id.value ==("root") } + val filtered = foundNodes.filter { case x: NodeInfo => x.node.id.value ==("root") } if(!filtered.isEmpty) { logger.debug("[%s] [post-filter:policyServer] %s results".format(debugId, withoutServerRole.size, filtered.mkString(", "))) } } withoutServerRole - case NodeAndPolicyServerReturnType => res.entries + case NodeAndPolicyServerReturnType => foundNodes } } } @@ -226,9 +202,8 @@ class AcceptedNodesLDAPQueryProcessor( } /** - * This is the last step of query, where we are looking for high level properties check - most importantly, + * This is the last step of query, where we are looking for check in NodeInfo - and complex, * the json path and check on node properties. - * See NodeInfoService#getLDAPNodeInfo */ object PostFilterNodeFromInfoService { val logger = LoggerFactory.getLogger("com.normation.rudder.services.queries") @@ -314,22 +289,17 @@ class PendingNodesLDAPQueryChecker( } else { val timePreCompute = System.currentTimeMillis for { - // get the pending node onfos we are considering + // get the pending node infos we are considering allPendingNodeInfos <- nodeInfoService.getPendingNodeInfos().toBox pendingNodeInfos = limitToNodeIds match { case None => allPendingNodeInfos.values.toSet case Some(ids) => allPendingNodeInfos.collect { case (nodeId, nodeInfo) if ids.contains(nodeId) => nodeInfo}.toSet } - res <- checker.internalQueryProcessor(query, Seq("1.1"), limitToNodeIds, 0, () => pendingNodeInfos.succeed).toBox + foundNodes <- checker.internalQueryProcessor(query, Seq("1.1"), limitToNodeIds, 0, () => pendingNodeInfos.succeed).toBox timeres = (System.currentTimeMillis - timePreCompute) - _ = logger.debug(s"LDAP result: ${res.entries.size} entries in pending nodes obtained in ${timeres}ms for query ${query.toString}") - - // nodesInfos = nodeInfoService.getLDAPNodeInfo(res.entries, res.nodeFilters, query.composition, pendingNodeInfos) - //filterTime = (System.currentTimeMillis - timePreCompute - timeres) - //_ = logger.debug(s"[post-filter:rudderNode] Found ${nodesInfos.size} nodes when filtering pending nodes for info service existence and properties (${filterTime} ms)") - + _ = logger.debug(s"LDAP result: ${foundNodes.size} entries in pending nodes obtained in ${timeres}ms for query ${query.toString}") } yield { - res.entries.map(_.node.id) + foundNodes.map(_.node.id) } } } @@ -366,10 +336,9 @@ class InternalLDAPQueryProcessor( /** * - * The high level query processor, with all the - * relevant logics. - * Sub classes should call that method to - * implement process&check method + * The high level query processor, with all the relevant logics. + * It looks in LDAP for infos that are only there + * and in the NodeInfos for eveything else */ def internalQueryProcessor( query : QueryTrait @@ -377,8 +346,8 @@ class InternalLDAPQueryProcessor( , limitToNodeIds: Option[Seq[NodeId]] = None , debugId : Long = 0L , lambdaAllNodeInfos : () => IOResult[Set[NodeInfo]] // this is hackish, to have the list of all node if - // only nodeinfofilters, so that we don't look for all - ) : IOResult[LdapQueryProcessorResult] = { + // only if necessary, to avoid the overall cost of looking for it + ) : IOResult[Set[NodeInfo]] = { @@ -494,7 +463,7 @@ class InternalLDAPQueryProcessor( nq <- normalizedQuery // special case: no query, but we create a dummy one, // identified by noFilterButTakeAllFromCache = true - // in this case, we skip all this part + // in this case, we skip all the ldap part optdms <- { if (nq.noFilterButTakeAllFromCache) { None.succeed @@ -587,8 +556,6 @@ class InternalLDAPQueryProcessor( } } - - // These transformation should be after nodeInfoService.getLDAPNodeInfo inverted = query.transform match { case ResultTransformation.Identity => nodeInfoFiltered case ResultTransformation.Invert => @@ -597,12 +564,9 @@ class InternalLDAPQueryProcessor( logEffect.debug(s"[${debugId}] |- (invert) entries after inversion: ${res.size}") res } - // distinctBy computes the hashcode of the object - // It is really expensive on LDAP entries. - // The dn string is already computed, so the toString is a good alternative (and as also unicity) postFiltered = postFilterNode(inverted, query.returnType, limitToNodeIds) } yield { - LdapQueryProcessorResult(postFiltered, nq.nodeInfoFilters) + postFiltered } } diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala index 25b0e336e6d..97b4baf9e29 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala @@ -61,7 +61,6 @@ import com.normation.rudder.reports.ResolvedAgentRunInterval import com.normation.rudder.reports.GlobalComplianceMode import com.normation.rudder.reports.execution._ import com.normation.rudder.repository.{CategoryWithActiveTechniques, ComplianceRepository, FullActiveTechniqueCategory, RoDirectiveRepository, RoRuleRepository} -import com.normation.rudder.services.nodes.LDAPNodeInfo import com.normation.rudder.services.nodes.NodeInfoService import com.normation.rudder.services.policies.NodeConfigData import com.normation.rudder.services.reports.{CachedFindRuleNodeStatusReports, CachedNodeChangesServiceImpl, DefaultFindRuleNodeStatusReports, NodeChangesServiceImpl, NodeConfigurationService, NodeConfigurationServiceImpl, ReportingServiceImpl, UnexpectedReportInterpretation} @@ -95,7 +94,6 @@ class ReportingServiceTest extends DBCommon with BoxSpecMatcher { } object nodeInfoService extends NodeInfoService { - def getLDAPNodeInfo(foundNodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodeInfos: Set[NodeInfo]): Set[NodeInfo]= ??? def getNodeInfo(nodeId: NodeId) : IOResult[Option[NodeInfo]] = ??? def getNodeInfos(nodesId: Set[NodeId]) : IOResult[Set[NodeInfo]] = ??? def getNode(nodeId: NodeId): Box[Node] = ??? diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala index 59e1be5aaa3..dfe3b5268ec 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala @@ -442,7 +442,7 @@ class TestQueryProcessor extends Loggable { """).openOrThrowException("For tests"), s(2) :: Nil) - testQueries(q1 :: q2 :: q3 :: Nil, true) + testQueries(q1 :: q2 :: q3 :: q3bis:: Nil, true) } @Test def networkInterfaceElementQueries(): Unit = { @@ -1239,7 +1239,7 @@ class TestQueryProcessor extends Loggable { if (doInternalQueryTest) { logger.debug("Testing with expected entries, This test should be ignored when we are looking for Nodes with NodeInfo and inventory (ie when we are looking for property and environement variable") val foundWithLimit = - (internalLDAPQueryProcessor.internalQueryProcessor(query, limitToNodeIds = Some(ids), lambdaAllNodeInfos = (() => nodeInfoService.getAllNodeInfos())).runNow.entries.map { + (internalLDAPQueryProcessor.internalQueryProcessor(query, limitToNodeIds = Some(ids), lambdaAllNodeInfos = (() => nodeInfoService.getAllNodeInfos())).runNow.map { _.node.id }).toSeq.distinct.sortBy( _.value ) diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala index 5c00f42fd68..b355e9f5fa0 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala @@ -42,8 +42,6 @@ import com.normation.inventory.domain.NodeId import com.normation.rudder.domain.nodes.Node import com.normation.rudder.domain.nodes.NodeInfo import com.normation.rudder.domain.policies.RuleId -import com.normation.rudder.domain.queries.CriterionComposition -import com.normation.rudder.domain.queries.NodeInfoMatcher import com.normation.rudder.domain.reports.ComplianceLevel import com.normation.rudder.domain.reports.NodeConfigId import com.normation.rudder.domain.reports.NodeExpectedReports @@ -53,7 +51,6 @@ import com.normation.rudder.reports.GlobalComplianceMode import com.normation.rudder.reports.execution.RoReportsExecutionRepository import com.normation.rudder.repository.FindExpectedReportRepository import com.normation.rudder.repository.ReportsRepository -import com.normation.rudder.services.nodes.LDAPNodeInfo import com.normation.rudder.services.nodes.NodeInfoService import com.normation.rudder.services.policies.NodeConfigData import net.liftweb.common.Box @@ -132,7 +129,6 @@ class CachedFindRuleNodeStatusReportsTest extends Specification { ) object testNodeInfoService extends NodeInfoService { - def getLDAPNodeInfo(foundNodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Set[NodeInfo]) : Set[NodeInfo] = ??? def getNodeInfo(nodeId: NodeId) : IOResult[Option[NodeInfo]] = ??? def getNodeInfos(nodesId: Set[NodeId]) : IOResult[Set[NodeInfo]] = ??? def getNode(nodeId: NodeId): Box[Node] = ??? diff --git a/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala b/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala index 18b7ca249c0..8f94e926c03 100644 --- a/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala +++ b/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala @@ -76,7 +76,6 @@ import com.normation.inventory.domain.AgentType.CfeCommunity import com.normation.zio._ import com.normation.rudder.domain.archives.RuleArchiveId import com.normation.rudder.domain.queries.CriterionComposition -import com.normation.rudder.domain.queries.NodeInfoMatcher import com.normation.rudder.repository.RoRuleRepository import com.normation.rudder.repository.WoRuleRepository import com.normation.rudder.services.nodes.NodeInfoService @@ -1488,7 +1487,7 @@ z5VEb9yx2KikbWyChM1Akp82AV5BzqE80QIBIw== } override def getAllNodes(): IOResult[Map[NodeId, Node]] = getAll().map(_.map(kv => (kv._1, kv._2.node))) - override def getAllNodeInfos():IOResult[Seq[NodeInfo]] = getAll().map(_.values.toSeq) + override def getAllNodeInfos():IOResult[Set[NodeInfo]] = getAll().map(_.values.toSet) override def getAllNodesIds(): IOResult[Set[NodeId]] = getAllNodes().map(_.keySet) override def getAllSystemNodeIds(): IOResult[Seq[NodeId]] = { nodeBase.get.map(_.collect { case (id, n) if(n.info.isSystem) => id }.toSeq ) @@ -1513,7 +1512,6 @@ z5VEb9yx2KikbWyChM1Akp82AV5BzqE80QIBIw== override def getAllNodeInventories(inventoryStatus: InventoryStatus): IOResult[Map[NodeId, NodeInventory]] = getGenericAll(inventoryStatus, _fullInventory(_).map(_.node)) // not implemented yet - override def getLDAPNodeInfo(nodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Set[NodeInfo]): Set[NodeInfo] = ??? override def getNumberOfManagedNodes: Int = ??? override def save(serverAndMachine: FullInventory): IOResult[Seq[LDIFChangeRecord]] = ??? override def delete(id: NodeId, inventoryStatus: InventoryStatus): IOResult[Seq[LDIFChangeRecord]] = ??? @@ -1705,16 +1703,16 @@ z5VEb9yx2KikbWyChM1Akp82AV5BzqE80QIBIw== } } - override def process(query: QueryTrait): Box[Seq[NodeInfo]] = { + override def process(query: QueryTrait): Box[Set[NodeInfo]] = { for { nodes <- nodeInfoService.nodeBase.get matching <- filterForLines(query.criteria, query.composition, nodes.map(_._2).toList).toIO } yield { - matching.map(_.info) + matching.map(_.info).toSet } }.toBox - override def processOnlyId(query: QueryTrait): Box[Seq[NodeId]] = process(query).map(_.map(_.id)) + override def processOnlyId(query: QueryTrait): Box[Set[NodeId]] = process(query).map(_.map(_.id)) } } diff --git a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala index 04d2cfbccb2..a70cc64dc21 100644 --- a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala +++ b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala @@ -74,8 +74,6 @@ import com.normation.rudder.domain.policies.DirectiveUid import com.normation.rudder.domain.policies.ModifyRuleDiff import com.normation.rudder.domain.policies.Rule import com.normation.rudder.domain.policies.RuleId -import com.normation.rudder.domain.queries.CriterionComposition -import com.normation.rudder.domain.queries.NodeInfoMatcher import com.normation.rudder.domain.workflows.ChangeRequestId import com.normation.rudder.git.GitArchiveId import com.normation.rudder.git.GitPath @@ -99,7 +97,6 @@ import com.normation.rudder.repository.ldap.WoLDAPRuleRepository import com.normation.rudder.repository.ldap.ZioTReentrantLock import com.normation.rudder.repository.xml.GitParseTechniqueLibrary import com.normation.rudder.services.eventlog.EventLogFactory -import com.normation.rudder.services.nodes.LDAPNodeInfo import com.normation.rudder.services.nodes.NodeInfoService import com.normation.rudder.services.policies.NodeConfigData import com.normation.rudder.services.policies.TechniqueAcceptationUpdater @@ -164,7 +161,6 @@ class TestMigrateSystemTechniques7_0 extends Specification { val nodeInfoService = new NodeInfoService { override def getAll(): IOResult[Map[NodeId, NodeInfo]] = List(root, relay1).map(x => (x.id, x)).toMap.succeed - override def getLDAPNodeInfo(foundNodeInfos: Set[NodeInfo], predicates: Seq[NodeInfoMatcher], composition: CriterionComposition, allNodesInfos : Set[NodeInfo]): Set[NodeInfo] = ??? override def getNodeInfo(nodeId: NodeId): IOResult[Option[NodeInfo]] = ??? override def getNodeInfos(nodeIds: Set[NodeId]): IOResult[Set[NodeInfo]] = ??? override def getNumberOfManagedNodes: Int = ??? From 34192f1f7078b42da2a04c9f2977f2fe0f29749c Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Tue, 8 Feb 2022 22:04:03 +0100 Subject: [PATCH 11/14] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed --- .../services/nodes/NodeInfoService.scala | 6 ++-- .../services/queries/LdapQueryProcessor.scala | 32 +++++++++---------- .../jdbc/ReportingServiceTest.scala | 2 +- .../CachedFindRuleNodeStatusReportsTest.scala | 2 +- .../com/normation/rudder/MockServices.scala | 2 +- .../TestMigrateSystemTechnique7_0.scala | 2 +- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala index e2d352116f2..e98894948f1 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala @@ -147,7 +147,7 @@ trait NodeInfoService { */ def getAllNodes() : IOResult[Map[NodeId, Node]] - def getAllNodeInfos():IOResult[Set[NodeInfo]] + def getAllNodeInfos():IOResult[Seq[NodeInfo]] /** * Get all systen node ids, for example * policy server node ids. @@ -858,8 +858,8 @@ trait NodeInfoServiceCached extends NodeInfoService with NamedZioLogger with Cac cache.view.mapValues(_._2.node).toMap.succeed } - def getAllNodeInfos():IOResult[Set[NodeInfo]] = withUpToDateCache("all nodeinfos") { cache => - cache.view.values.map(_._2).toSet.succeed + def getAllNodeInfos():IOResult[Seq[NodeInfo]] = withUpToDateCache("all nodeinfos") { cache => + cache.view.values.map(_._2).toSeq.succeed } def getNodeInfo(nodeId: NodeId): IOResult[Option[NodeInfo]] = withUpToDateCache(s"${nodeId.value} node info") { cache => diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala index 54a4de23100..64af37b42d9 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala @@ -211,7 +211,7 @@ object PostFilterNodeFromInfoService { foundNodeInfos: Set[NodeInfo] // the one from the search, need to be a Set for fast "contain" , predicates : Seq[NodeInfoMatcher] , composition : CriterionComposition - , allNodesInfos : Set[NodeInfo] // all the nodeinfo there is + , allNodesInfos : Seq[NodeInfo] // all the nodeinfo there is ): Set[NodeInfo] = { def comp(a: Boolean, b: Boolean) = composition match { case And => a && b @@ -244,7 +244,7 @@ object PostFilterNodeFromInfoService { contains } else { val combined = predicates.reduceLeft(combine) - val validPredicates = combined.matches(nodeInfo) + val validPredicates = combined.matches(nodeInfo) val res = comp(contains, validPredicates) if(logger.isTraceEnabled()) { @@ -256,13 +256,13 @@ object PostFilterNodeFromInfoService { composition match { // TODO: there is surely something we can do here - case Or => allNodesInfos.collect { case nodeinfo if predicate(nodeinfo, predicates, composition) => nodeinfo } + case Or => allNodesInfos.filter { nodeinfo => predicate(nodeinfo, predicates, composition) }.toSet case And => if (predicates.isEmpty) { // paththru foundNodeInfos } else { - foundNodeInfos.collect { case nodeinfo if predicate(nodeinfo, predicates, composition) => nodeinfo } + foundNodeInfos.filter { nodeinfo => predicate(nodeinfo, predicates, composition) } } } @@ -292,8 +292,8 @@ class PendingNodesLDAPQueryChecker( // get the pending node infos we are considering allPendingNodeInfos <- nodeInfoService.getPendingNodeInfos().toBox pendingNodeInfos = limitToNodeIds match { - case None => allPendingNodeInfos.values.toSet - case Some(ids) => allPendingNodeInfos.collect { case (nodeId, nodeInfo) if ids.contains(nodeId) => nodeInfo}.toSet + case None => allPendingNodeInfos.values.toSeq + case Some(ids) => allPendingNodeInfos.collect { case (nodeId, nodeInfo) if ids.contains(nodeId) => nodeInfo}.toSeq } foundNodes <- checker.internalQueryProcessor(query, Seq("1.1"), limitToNodeIds, 0, () => pendingNodeInfos.succeed).toBox timeres = (System.currentTimeMillis - timePreCompute) @@ -345,7 +345,7 @@ class InternalLDAPQueryProcessor( , select : Seq[String] = Seq() , limitToNodeIds: Option[Seq[NodeId]] = None , debugId : Long = 0L - , lambdaAllNodeInfos : () => IOResult[Set[NodeInfo]] // this is hackish, to have the list of all node if + , lambdaAllNodeInfos : () => IOResult[Seq[NodeInfo]] // this is hackish, to have the list of all node if // only if necessary, to avoid the overall cost of looking for it ) : IOResult[Set[NodeInfo]] = { @@ -489,7 +489,7 @@ class InternalLDAPQueryProcessor( case And if query.transform == ResultTransformation.Invert => lambdaAllNodeInfos() case And if optdms.isDefined => lambdaAllNodeInfos() - case _ => Set[NodeInfo]().succeed + case _ => Seq[NodeInfo]().succeed } timefetch <- currentTimeMillis _ <- logPure.debug(s"LDAP result: fetching if necessary all nodesInfos in in nodes obtained in ${timefetch-timeStart} ms for query ${query.toString}") @@ -498,7 +498,7 @@ class InternalLDAPQueryProcessor( // so we skip the last query results <- optdms match { case None if nq.noFilterButTakeAllFromCache => - allNodesInfos.succeed + allNodesInfos.toSet.succeed case None => Set[NodeInfo]().succeed case Some(dms) => @@ -521,8 +521,8 @@ class InternalLDAPQueryProcessor( _ <- logPure.debug(s"[${debugId}] |- (final query) ${rt}") entries <- executeQuery(rt.baseDn, rt.scope, nodeObjectTypes.objectFilter, rt.filter, finalSpecialFilters, select.toSet, nq.composition, debugId) // convert these entries into nodeInfo - nodesId = entries.flatMap(x => x(A_NODE_UUID)) - nodeInfos = allNodesInfos.filter(nodeInfo => nodesId.contains(nodeInfo.node.id.value)) + nodesId = entries.flatMap(x => x(A_NODE_UUID)).toSet + nodeInfos = allNodesInfos.filter(nodeInfo => nodesId.contains(nodeInfo.node.id.value)).toSet } yield nodeInfos). tapError(err => logPure.debug(s"[${debugId}] `-> error: ${err.fullMsg}")). tap(seq => logPure.debug(s"[${debugId}] `-> ${seq.size} results")) @@ -530,7 +530,7 @@ class InternalLDAPQueryProcessor( // No more LDAP query is required here // Do the filtering about non LDAP data here timeldap <- currentTimeMillis - _ <- logPure.debug(s"LDAP result: ${results.size} entries in nodes obtained in ${timeldap-timeStart} ms for query ${query.toString}") + _ <- logPure.debug(s"LDAP result: ${results.size} entries in nodes obtained in ${timeldap-timeStart} ms for query ${query.toString}") nodeInfoFiltered = query.composition match { @@ -539,14 +539,14 @@ class InternalLDAPQueryProcessor( Set[NodeInfo]() case And => // If i'm doing and AND, there is no need for the allNodes here - PostFilterNodeFromInfoService.getLDAPNodeInfo(results, nq.nodeInfoFilters, query.composition, Set()) + PostFilterNodeFromInfoService.getLDAPNodeInfo(results, nq.nodeInfoFilters, query.composition, Seq()) case Or => // Here we need the list of all nodes PostFilterNodeFromInfoService.getLDAPNodeInfo(results, nq.nodeInfoFilters, query.composition, allNodesInfos) } timefilter <- currentTimeMillis - _ <- logPure.debug(s"[post-filter:rudderNode] Found ${nodeInfoFiltered.size} nodes when filtering for info service existence and properties (${timefilter-timeldap} ms)") - _ <- logPure.ifDebugEnabled{ + _ <- logPure.debug(s"[post-filter:rudderNode] Found ${nodeInfoFiltered.size} nodes when filtering for info service existence and properties (${timefilter-timeldap} ms)") + _ <- logPure.ifDebugEnabled{ val filtered = results.map( x => x.node.id.value).diff(nodeInfoFiltered.map( x => x.node.id.value)) if(filtered.nonEmpty) { logPure.debug(s"[${debugId}] [post-filter:rudderNode] ${nodeInfoFiltered.size} results (following nodes not in ou=Nodes,cn=rudder-configuration or not matching filters on NodeInfo: ${filtered.mkString(", ")}") @@ -560,7 +560,7 @@ class InternalLDAPQueryProcessor( case ResultTransformation.Identity => nodeInfoFiltered case ResultTransformation.Invert => logEffect.debug(s"[${debugId}] |- (need to get all nodeIds for inversion) ") - val res = allNodesInfos.diff(nodeInfoFiltered) + val res = allNodesInfos.toSet.diff(nodeInfoFiltered) logEffect.debug(s"[${debugId}] |- (invert) entries after inversion: ${res.size}") res } diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala index 97b4baf9e29..097e82b185f 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/repository/jdbc/ReportingServiceTest.scala @@ -98,7 +98,7 @@ class ReportingServiceTest extends DBCommon with BoxSpecMatcher { def getNodeInfos(nodesId: Set[NodeId]) : IOResult[Set[NodeInfo]] = ??? def getNode(nodeId: NodeId): Box[Node] = ??? def getAllNodes() : IOResult[Map[NodeId, Node]] = ??? - def getAllNodeInfos():IOResult[Set[NodeInfo]] = ??? + def getAllNodeInfos():IOResult[Seq[NodeInfo]] = ??? def getAllNodesIds(): IOResult[Set[NodeId]] = ??? def getAllSystemNodeIds() : IOResult[Seq[NodeId]] = ??? def getPendingNodeInfos(): IOResult[Map[NodeId, NodeInfo]] = ??? diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala index b355e9f5fa0..a8446bf0b83 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/CachedFindRuleNodeStatusReportsTest.scala @@ -134,7 +134,7 @@ class CachedFindRuleNodeStatusReportsTest extends Specification { def getNode(nodeId: NodeId): Box[Node] = ??? def getAllNodes() : IOResult[Map[NodeId, Node]] = ??? def getAllNodesIds(): IOResult[Set[NodeId]] = ??? - def getAllNodeInfos():IOResult[Set[NodeInfo]] = ??? + def getAllNodeInfos():IOResult[Seq[NodeInfo]] = ??? def getAllSystemNodeIds() : IOResult[Seq[NodeId]] = ??? def getPendingNodeInfos(): IOResult[Map[NodeId, NodeInfo]] = ??? def getPendingNodeInfo(nodeId: NodeId): IOResult[Option[NodeInfo]] = ??? diff --git a/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala b/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala index 8f94e926c03..d31e9d3e268 100644 --- a/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala +++ b/webapp/sources/rudder/rudder-rest/src/test/scala/com/normation/rudder/MockServices.scala @@ -1487,7 +1487,7 @@ z5VEb9yx2KikbWyChM1Akp82AV5BzqE80QIBIw== } override def getAllNodes(): IOResult[Map[NodeId, Node]] = getAll().map(_.map(kv => (kv._1, kv._2.node))) - override def getAllNodeInfos():IOResult[Set[NodeInfo]] = getAll().map(_.values.toSet) + override def getAllNodeInfos():IOResult[Seq[NodeInfo]] = getAll().map(_.values.toSeq) override def getAllNodesIds(): IOResult[Set[NodeId]] = getAllNodes().map(_.keySet) override def getAllSystemNodeIds(): IOResult[Seq[NodeId]] = { nodeBase.get.map(_.collect { case (id, n) if(n.info.isSystem) => id }.toSeq ) diff --git a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala index a70cc64dc21..3fb6df04122 100644 --- a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala +++ b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateSystemTechnique7_0.scala @@ -166,7 +166,7 @@ class TestMigrateSystemTechniques7_0 extends Specification { override def getNumberOfManagedNodes: Int = ??? override def getAllNodesIds(): IOResult[Set[NodeId]] = ??? override def getAllNodes(): IOResult[Map[NodeId, Node]] = ??? - override def getAllNodeInfos(): IOResult[Set[NodeInfo]] = ??? + override def getAllNodeInfos(): IOResult[Seq[NodeInfo]] = ??? override def getAllSystemNodeIds(): IOResult[Seq[NodeId]] = ??? override def getPendingNodeInfos(): IOResult[Map[NodeId, NodeInfo]] = ??? override def getPendingNodeInfo(nodeId: NodeId): IOResult[Option[NodeInfo]] = ??? From 7d0e5c7e1f6958b62293e8942a781759e084ae71 Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Tue, 8 Feb 2022 22:09:19 +0100 Subject: [PATCH 12/14] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed --- .../rudder/services/queries/TestQueryProcessor.scala | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala index dfe3b5268ec..35f326ecabb 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala @@ -630,18 +630,6 @@ class TestQueryProcessor extends Loggable { """).openOrThrowException("For tests"), s.filterNot(n => n == s(1)) ) - // test query that doesn't match a software name, ie we want all nodes on which "software 1" is not - // installed (we don't care if there is 0 or 1000 other software) - // THIS DOES NOT WORK DUE TO: https://issues.rudder.io/issues/19137 -// val q12 = TestQuery( -// "q12", -// parser(""" -// { "select":"node", "composition":"or", "where":[ -// { "objectType":"software", "attribute":"cn", "comparator":"notRegex", "value":"Software 1" } -// ] } -// """).openOrThrowException("For tests"), -// s.filterNot(n => n == s(2)) ) - testQueries(q0 :: q1 :: q1_ :: q2 :: q2_ :: q3 :: q3_2 :: q4 :: q5 :: q6 :: q7 :: q8 :: q9 :: q10 :: q11 :: Nil, false) } From 9d5a32137ae672ccd91c86cb58a7baca3755b9bd Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Tue, 8 Feb 2022 23:17:25 +0100 Subject: [PATCH 13/14] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed --- .../rudder/services/queries/LdapQueryProcessor.scala | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala index 64af37b42d9..cbc93fe9628 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala @@ -295,11 +295,18 @@ class PendingNodesLDAPQueryChecker( case None => allPendingNodeInfos.values.toSeq case Some(ids) => allPendingNodeInfos.collect { case (nodeId, nodeInfo) if ids.contains(nodeId) => nodeInfo}.toSeq } - foundNodes <- checker.internalQueryProcessor(query, Seq("1.1"), limitToNodeIds, 0, () => pendingNodeInfos.succeed).toBox + foundNodes <- checker.internalQueryProcessor(query, Seq("1.1"), limitToNodeIds, 0, () => pendingNodeInfos.succeed).toBox timeres = (System.currentTimeMillis - timePreCompute) _ = logger.debug(s"LDAP result: ${foundNodes.size} entries in pending nodes obtained in ${timeres}ms for query ${query.toString}") } yield { - foundNodes.map(_.node.id) + //filter out Rudder server component if necessary + (query.returnType match { + case NodeReturnType => + // we have a special case for the root node that always never to that group, even if some weird + // scenario lead to the removal (or non addition) of them + foundNodes.filterNot { case x: NodeInfo => x.node.id.value ==("root") } + case NodeAndPolicyServerReturnType => foundNodes + }).map(_.node.id) } } } From 1bcb501a9cbd7ed5b46631a4dbfa61bcf5e1c469 Mon Sep 17 00:00:00 2001 From: Nicolas Charles Date: Wed, 9 Feb 2022 09:10:04 +0100 Subject: [PATCH 14/14] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Fixes #20716: Improve dynamic group computation speed Fixes #20716: Improve dynamic group computation speed and fix inverted searched --- .../normation/rudder/services/nodes/NodeInfoService.scala | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala index e98894948f1..0bffee655fe 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala @@ -147,6 +147,12 @@ trait NodeInfoService { */ def getAllNodes() : IOResult[Map[NodeId, Node]] + /** + * Get all nodes + * This returns a Seq for performance reasons - it is much faster + * to return a Seq than a Set, and for subsequent use it is also + * faster + */ def getAllNodeInfos():IOResult[Seq[NodeInfo]] /** * Get all systen node ids, for example