Skip to content

Optimize data workers #1820

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 29 commits into from
Nov 9, 2023
Merged

Optimize data workers #1820

merged 29 commits into from
Nov 9, 2023

Conversation

garrrikkotua
Copy link
Contributor

@garrrikkotua garrrikkotua commented Nov 6, 2023 •
edited by ghost
Loading

Changes proposed ✍️

What

🤖 Generated by Copilot at b5d4613

Increase the concurrency and add locking mechanisms for activity processing in the data sink and integration data workers. This will improve the efficiency and reliability of the services that handle activities from the data source.
​

🤖 Generated by Copilot at b5d4613

Oh, we're the jolly coders of the MAX_CONCURRENT_PROCESSING three
We pull and push the data from the source to the sea
We don't need no old functions that are lying in the dust
We use our custom locking with the mighty Redis trust

Why

How

🤖 Generated by Copilot at b5d4613

  • Add custom locking functions for concurrent processing of activities (link)
  • Comment out unused locking functions from @crowd/redis (link)
  • Increase the maximum number of activities that can be processed at the same time by the integration_data_worker service (link)

Checklist ✅

  • Label appropriately with Feature, Improvement, or Bug.
  • Add screenshots to the PR description for relevant FE changes
  • New backend functionality has been unit-tested.
  • API documentation has been updated (if necessary) (see docs on API documentation).
  • Quality standards are met.

themarolt
themarolt reviewed Nov 7, 2023
View reviewed changes
services/apps/data_sink_worker/src/jobs/processOldResults.ts Outdated
@@ -30,13 +32,13 @@ export const processOldResultsJob = async (

const loadNextBatch = async (): Promise<string[]> => {
return await processWithLock(redis, 'process-old-results', 5 * 60, 3 * 60, async () => {
const resultIds = await repo.getOldResultsToProcess(5)
const resultIds = await repo.getOldResultsToProcess(MAX_CONCURRENT_PROMISES)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would load like 10 at once and process them MAX_CONCURRENT_PROMISES at a time.

themarolt
themarolt reviewed Nov 7, 2023
View reviewed changes
@garrrikkotua garrrikkotua merged commit adf86df into main Nov 9, 2023
@garrrikkotua garrrikkotua deleted the improve/optimize-data-workers branch November 9, 2023 15:44
sausage-todd pushed a commit that referenced this pull request Nov 9, 2023
@garrrikkotua
Optimize data workers (#1820)
0e57f23 
Co-authored-by: Uroš Marolt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@themarolt themarolt themarolt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@garrrikkotua @themarolt