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(python): respect concurrency in worker #2062

Merged
merged 5 commits into from
Jul 17, 2023
Merged

Conversation

roggervalf
Copy link
Collaborator

@roggervalf roggervalf commented Jul 13, 2023

fixes #2063

@roggervalf roggervalf marked this pull request as ready for review July 13, 2023 04:09
@@ -59,21 +59,19 @@ async def run(self):
token_postfix = 0

while not self.closed:
if len(jobs) == 0 and len(self.processing) < self.opts.get("concurrency") and not self.closing:
while len(self.processing) < self.opts.get("concurrency") and not self.closing:
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure this is a proper solution as this could result in multiple calls to BRPOPLPUSH instead of only 1, as we do on the typescript version.

Copy link
Collaborator Author

@roggervalf roggervalf Jul 15, 2023

Choose a reason for hiding this comment

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

Copy link
Contributor

@manast manast Jul 16, 2023

Choose a reason for hiding this comment

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

Yes, the difference is the asyncQueue that avoids the excessive number of calls. It waits until BRPOPLPUSH has timedout or returned a job id. It is a bit tricky to understand the code, but that's the idea.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

at the end, the magic is handled by waiting attribute in worker class

@roggervalf roggervalf requested a review from manast July 17, 2023 16:33
Copy link
Contributor

@manast manast left a comment

Choose a reason for hiding this comment

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

It looks correct to me.

@roggervalf roggervalf merged commit 1b95185 into master Jul 17, 2023
9 checks passed
@roggervalf roggervalf deleted the fix-concurrency-python branch July 17, 2023 20:33
github-actions bot pushed a commit that referenced this pull request Jul 18, 2023
# [4.5.0](v4.4.0...v4.5.0) (2023-07-18)

### Bug Fixes

* **python:** respect concurrency in worker ([#2062](#2062)) fixes [#2063](#2063) ([1b95185](1b95185))

### Features

* **job:** add option for removing children in remove method (python) ([#2064](#2064)) ([841dc87](841dc87))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Concurrency support for Python is not there
2 participants