-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Initial Refactor for speed control #4986
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm !
- suggested 2 possible changes
- if i am not wrong this lays a foundation for handler / controller logic which will be implemented in follow up PRs ( but as of now this is a No-OP behaviour wise )
More thoughts
- Looking at implementation i wonder if we could have created all these sizedpools from a single type/method like
types.Options
or maybe a option to register a optional handler when intializing a sizedpool created a new sizedpool ex:
swg := NewSizedPool(concurrency).RegisterController("js-pool", autoModeController)
and we would have a autoModeController running in goroutine continiously monitering memory and follow a pre-defined algorithm to rebalance different sizedpools at different locations ( ex: decrease payloadconcurrency first and observe change etc )
that way this could be later on extended to implement a global nuclei level smart rate limit controller
Actually it's a NO-OP only for usages from CLI, while via SDK a change within the input settings, causes the internal pools to follow the new value within The centralizing of speed control was the central thought at #4919, so that the various resizable pools would have referred to a central unique controller, with the advantage that references were cleaned up automatically, while with the approach you suggested, while easier to implement, requires a very careful removal of handlers, since they are tied to a global go routine. As we have seen this is the source of endless memory issues. The biggest problem with this approach is the logic scattered into ultra-wrapped callbacks with unclear intended closures and circular references. |
Proposed changes
Due to the impact of internal changes (remove callbacks, move components from static to component-field and pass via dep-injection) the original PR is put On Hold, and a lighter approach for speed control is implemented here. The speed control will be performed with the following points:
RateLimitPerMinute
and addingRateLimitDuration
types.Option
options:BulkSize
Threads
PayloadConcurrency
JsConcurrency
RateLimit
RateLimitDuration
In order to allow autotuning it's first necessary to resolve internal design issues to allow smooth communication between the growing amount of components. As a follow up we can expose the speed control capability via local http endpoint (maybe the same metrics/stats one)
Checklist