dgraph icon indicating copy to clipboard operation
dgraph copied to clipboard

perf(concurrency): cancel remaining goroutines when has error

Open eileenaaa opened this issue 5 months ago • 3 comments

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

  • [x] 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 avatar Aug 15 '25 03:08 eileenaaa

Can you instead use errGroup to do the same thing?

ghost avatar Aug 15 '25 05:08 ghost

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?

eileenaaa avatar Aug 15 '25 08:08 eileenaaa

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.

ghost avatar Aug 16 '25 00:08 ghost