Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fixes #14924: Cleanup unreferenced software #2231

Conversation

ncharles
Copy link
Member

Full(Unit)
case Failure(msg, exception, chain) => mutSetSoftwares = Failure(msg, exception, chain) // otherwise the time is wrong
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

this ain't pretty, yet its more efficient
[2019-05-23 13:00:06] DEBUG com.normation.inventory.ldap.core.SoftwareServiceImpl - All softwares id in nodes fetched: 37953 softwares id in 10029ms, compared to what is in the TODO below
[2019-05-23 12:55:17] DEBUG com.normation.inventory.ldap.core.SoftwareServiceImpl - All softwares id in nodes fetched: 37953 softwares id in 15197ms

Copy link
Member Author

Choose a reason for hiding this comment

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

this code is surely non idiomatic, and i fail to improve it

@ncharles ncharles force-pushed the bug_14924/cleanup_unreferenced_software branch from fe7bf28 to 4bb4a0d Compare May 23, 2019 11:49
@ncharles
Copy link
Member Author

Commit modified

@@ -146,7 +146,7 @@ along with Rudder. If not, see <http://www.gnu.org/licenses/>.
are doing.
-->

<root level="info">
<root level="debug">
Copy link
Member

Choose a reason for hiding this comment

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

That should not be modified

@@ -360,7 +360,7 @@ along with Rudder. If not, see <http://www.gnu.org/licenses/>.
================
This logger is in charge of all scheduled jobs and batches
-->
<logger name="scheduledJob" level="info" additivity="false">
<logger name="scheduledJob" level="debug" additivity="false">
Copy link
Member

Choose a reason for hiding this comment

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

neither that

@ncharles ncharles force-pushed the bug_14924/cleanup_unreferenced_software branch from 4bb4a0d to 4517d59 Compare May 31, 2019 09:36
@ncharles
Copy link
Member Author

Commit modified


batchedNodes = nodes.grouped(50)

_ = batchedNodes.foreach { nodeEntries: Seq[LDAPEntry] =>
Copy link
Member

Choose a reason for hiding this comment

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

it should not be a foreach, else we loose the maybe Failure: res <- sequence(batchedNodes) { .... }

Copy link
Member Author

Choose a reason for hiding this comment

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

i tried using a sequence on batchedNodes, without success (it's an iterator, I cannot sequence it)

Copy link
Member

@fanf fanf Jun 3, 2019

Choose a reason for hiding this comment

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

You can do:

      batchedNodes         = con.searchSub(acceptedDit.NODES.dn.getParent, IS(OC_NODE), A_NODE_UUID).grouped(50).toSeq

      res <- sequence(batchedNodes) { nodeEntries: Seq[LDAPEntry] =>

You still process nodes 50 by 50, and don't hold more node ids than previously

@ncharles ncharles force-pushed the bug_14924/cleanup_unreferenced_software branch from db1e062 to fd62862 Compare June 11, 2019 11:04
@ncharles
Copy link
Member Author

Commit modified

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/2231
-- Your faithful QA
Kant merge: "Happiness is not an ideal of reason, but of imagination."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/11167/console)

@fanf
Copy link
Member

fanf commented Jun 12, 2019

OK, merging this PR

@fanf fanf merged commit fd62862 into Normation:branches/rudder/5.0 Jun 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants