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

fix: avoid concurrent data collectors #296

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

luislhl
Copy link
Contributor

@luislhl luislhl commented Jan 5, 2024

Acceptance Criteria

We should make sure we won't run the DataCollector daemon's logic concurrently in the asyncio event loop.

Each execution should wait for the previous one to finish before starting.

The way we did it before left the possibility of having several async tasks in the event loop at once, in case they took more than 1 second to finish.

For instance, supposing each of them took 5 seconds to finish, we would have 5 tasks running concurrently.

By looking at the number of concurrent executions in the Lambda that is called by this daemon, we can see that sometimes we had even 9 concurrent executions:

image

I suspect this could be related to high memory consumption in the daemon, but I'm not 100% sure. The fix looked beneficial anyway.

Security Checklist

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@luislhl luislhl requested a review from r4mmer as a code owner January 5, 2024 22:08
@luislhl luislhl self-assigned this Jan 5, 2024
@luislhl luislhl changed the base branch from main to dev January 5, 2024 22:09
@luislhl luislhl requested a review from jansegre January 5, 2024 22:09
@luislhl luislhl merged commit 3c07334 into dev Jan 10, 2024
4 checks passed
@luislhl luislhl deleted the fix/avoid-concurrent-data-collectors branch January 10, 2024 15:47
This was referenced Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants