Skip to content

Conversation

eileenaaa
Copy link
Contributor

@eileenaaa eileenaaa commented Aug 15, 2025

Description

This PR optimizes goroutine execution in multi-task scenarios:

  • Uses errgroup.WithContext to manage multiple goroutines and propagate cancellation signals.
  • When any goroutine encounters an error, errgroup automatically cancels the remaining goroutines, allowing them to exit early.
  • Reduces unnecessary computation and resource usage when partial failures occur.

This improves system responsiveness and reduces wasted CPU cycles in error cases.

Checklist

  • Code compiles correctly and linting passes locally
  • For all code changes, an entry added to the CHANGELOG.md file describing and linking to
    this PR
  • Tests added for new functionality, or regression tests for bug fixes added as applicable

@eileenaaa eileenaaa requested a review from a team as a code owner August 15, 2025 03:23
@harshil-goel
Copy link
Contributor

Can you instead use errGroup to do the same thing?

@eileenaaa
Copy link
Contributor Author

eileenaaa commented Aug 15, 2025

Can you instead use errGroup to do the same thing?

Yes, that works. I’ve implemented the same logic using an errgroup with context—please review code again.

Also, a quick question: since we’re using egCtx (from errgroup.WithContext(ctx)),
it already inherits all features from the parent ctx, including cancellation and timeouts.
In handleUidPostings, there’s still a check on the parent ctx every 100 iterations (around line 812 in worker/task.go).
Given that, is this check still necessary, or is there a specific reason for keeping it?

@harshil-goel
Copy link
Contributor

If we don't have that check, it could take a long time for a thread to finish as this one thread with no errors could take long when other threads have been cancelled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants