-
Notifications
You must be signed in to change notification settings - Fork 36
5sec Timeout - Performance Issue #16
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
Comments
@sagarvsh Thanks for reaching out, yes, we have actually ran into similar issues from time to time as well with this when devs aren't following certain best practices or when they aren't committing coding regularly. One thing to mention right away is squash merging. If you squash merge then this causes SEDATED to rescan all the changes in the commits again. If possible, avoid squash merging, or if you require squashing then maybe do it more often. By doing this, it reduces the amount of content that is scanned in by SEDATED in that 5 second window. Here are some other suggestions to watch out for especially for repos you repeatedly have issues with:
Another option to help avoid the 5 second issue, one of the changes we intentionally made was how we are handling whitelisted commits. Previously, if a commit was whitelisted we still ran the grep command (expensive) on the contents of that commit. Now though, in the lasted version of the code we no longer grep if the commit is whitelisted. Here is what that means. If you push 5 huge commits and they are all whitelisted, you will not hit the 5 second window because we don't bother interrogating the huge content in your 5 commits. With that said, an alternative approach to repo whitelisting, is to just do normal commit level whitelisting on the pushes that contain a bunch of code. |
@sagarvsh glad to hear you are using SEDATED! 😊 Just curious, what types of files are being pushed and/or lines of code in the commit(s) that is causing the push to hit the 5s timeout? images/pdfs/zips/bundle.js? We may be able to optimize if we can find a common culprit. |
Thank you @SimeonCloutier This really helps. I will test this out and update back. |
@denniskennedy We are working very hard to clear out the sensitive data from our code base and this tool is a boon to prevent new sensitive data from getting in. Here is one example where we have 5s rule issue. |
@sagarvsh In this example you mentioned, is a developer building up 17k modified lines of code (I would assume over the course of many months) and then many months later pushing this up to github? If not, can you explain the the flow of pushing/pulling/merging the code that is occurring in this example? |
@SimeonCloutier I think I was found the issue.
I will start collecting more metrics which might help. |
@sagarvsh FYI, we just released a new version of SEDATED®, with lots of improvements (see below). |
Perfect, let me try it today and let you know. |
Hi Team,
We have implemented SEDATED in our Organization and it's been very successful. Thank you for this. One challenge we are facing is on the performance, many times, even with medium sized changes in the push or when we push the changes to Master branch, we easily end up with 5sec rule timeout. For now, we are whitelisting the repo till the push is complete and revert it.
Any recommendations or better work around to deal with this issue, as the current process is slowing down or blocking the development process even when there is no sensitive data in the code.
Thank you
Sagar
The text was updated successfully, but these errors were encountered: