Skip to content
This repository was archived by the owner on Aug 12, 2022. It is now read-only.

feat: Add max_goroutines#184

Merged
yevgenypats merged 7 commits intomainfrom
feat/smart_limiter
Feb 8, 2022
Merged

feat: Add max_goroutines#184
yevgenypats merged 7 commits intomainfrom
feat/smart_limiter

Conversation

@yevgenypats
Copy link
Copy Markdown
Contributor

@yevgenypats yevgenypats commented Feb 7, 2022

This adds another option called max_goroutines to keep the number of goroutines at hand and not OOM when having huge amount of projects or accounts.

@yevgenypats yevgenypats changed the title feat: Support max_parallel_resource_fetch_limit for goroutines feat: Add max_goroutines Feb 7, 2022
@github-actions github-actions bot added feat and removed feat labels Feb 7, 2022
roneli
roneli previously requested changes Feb 8, 2022
Copy link
Copy Markdown
Contributor

@roneli roneli left a comment

Choose a reason for hiding this comment

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

Some CR comments.

@yevgenypats yevgenypats requested a review from roneli February 8, 2022 09:08
Copy link
Copy Markdown
Member

@disq disq left a comment

Choose a reason for hiding this comment

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

LGTM overall with comments

yevgenypats and others added 3 commits February 8, 2022 13:04
Co-authored-by: Kemal <223029+disq@users.noreply.github.com>
@yevgenypats
Copy link
Copy Markdown
Contributor Author

@disq I didn't blindly cast to int64 - https://github.com/cloudquery/cq-provider-sdk/pull/184/files#diff-0e3b5cd2e6abf4633712dbd2d69307b7fbd75bc71844e34306afdcd9840dab64R182 (maybe I didn't push right away as there was a conflict in my local git)

@yevgenypats yevgenypats merged commit 30c9a61 into main Feb 8, 2022
@yevgenypats yevgenypats deleted the feat/smart_limiter branch February 8, 2022 11:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants