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 #16661: inneficient computation of RuleStatusReports and NodeStatusReports #2754

Conversation

ncharles
Copy link
Member

@ncharles ncharles commented Feb 3, 2020

@ncharles ncharles added the WIP Use that label for a Work In Progress PR that must not be merged yet label Feb 3, 2020
@ncharles
Copy link
Member Author

ncharles commented Feb 3, 2020

this is a WIP. I'll ensure that getNodeStatusReports return merged data, and don't use merge in all method calling it

@ncharles
Copy link
Member Author

ncharles commented Feb 3, 2020

PR updated with a new commit

10 similar comments
@ncharles
Copy link
Member Author

ncharles commented Feb 3, 2020

PR updated with a new commit

@ncharles
Copy link
Member Author

ncharles commented Feb 3, 2020

PR updated with a new commit

@ncharles
Copy link
Member Author

ncharles commented Feb 3, 2020

PR updated with a new commit

@ncharles
Copy link
Member Author

ncharles commented Feb 3, 2020

PR updated with a new commit

@ncharles
Copy link
Member Author

ncharles commented Feb 3, 2020

PR updated with a new commit

@ncharles
Copy link
Member Author

ncharles commented Feb 3, 2020

PR updated with a new commit

@ncharles
Copy link
Member Author

ncharles commented Feb 3, 2020

PR updated with a new commit

@ncharles
Copy link
Member Author

ncharles commented Feb 4, 2020

PR updated with a new commit

@ncharles
Copy link
Member Author

ncharles commented Feb 4, 2020

PR updated with a new commit

@ncharles
Copy link
Member Author

ncharles commented Feb 5, 2020

PR updated with a new commit

@ncharles
Copy link
Member Author

ncharles commented Feb 7, 2020

This version caused a FullGC during policy compliance computation + writting policies
It never happened without this PR

@ncharles
Copy link
Member Author

ncharles commented Feb 7, 2020

PR updated with a new commit

1 similar comment
@ncharles
Copy link
Member Author

ncharles commented Feb 7, 2020

PR updated with a new commit

@ncharles ncharles removed the WIP Use that label for a Work In Progress PR that must not be merged yet label Mar 2, 2020
@ncharles
Copy link
Member Author

ping @fanf

Copy link
Member

@fanf fanf left a comment

Choose a reason for hiding this comment

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

Appart from trivial remark, this seems good!

, complianceLevel.complianceWithoutPending.round
))
}
// global = reportingService.computeComplianceFromReports(reports)
Copy link
Member

Choose a reason for hiding this comment

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

remove comments here and on l208

@ncharles
Copy link
Member Author

PR updated with a new commit

@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/2754
-- Your faithful QA
Kant merge: "Science is organized knowledge. Wisdom is organized life."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/21443/console)

@ncharles
Copy link
Member Author

OK, squash merging this PR

ncharles and others added 6 commits March 13, 2020 15:31
Fixes #16661: inneficient computation of RuleStatusReports and NodeStatusReports
Fixes #16661: inneficient computation of RuleStatusReports and NodeStatusReports
Fixes #16661: inneficient computation of RuleStatusReports and NodeStatusReports
@ncharles ncharles force-pushed the bug_16661/inneficient_computation_of_rulestatusreports_and_nodestatusreports branch from 3c41f78 to 8723a34 Compare March 13, 2020 14:31
@ncharles ncharles merged commit 8723a34 into Normation:branches/rudder/5.0 Mar 13, 2020
@ncharles
Copy link
Member Author

OK, merging this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants