argoproj-labs / argocd-agent

Redefining the multi cluster story of Argo CD
Apache License 2.0
79 stars 15 forks source link

feat: resend events with exponential backoff and wait for ACK #225

Open chetan-rns opened 6 days ago

chetan-rns commented 6 days ago

Note: I think there are multiple ways of designing this and I'm happy to update the approach based on the feedback.

Other choices that were considered:

  1. We send events on a stream and immediately wait for an ACK. Use apimachinery's wait.ExponentialBackOff to resend events until we receive an ACK. However, the events from other resources will be blocked until the current event is processed.
  2. We can improvise the above approach by running the wait.ExponentialBackOff in a separate goroutine for each app. However, each goroutine is shortlived (they only exist until we receive an ACK) and there might be a scenario where the runtime spends more time context switching if there are a large number of apps.
  3. We could potentially solve this using a pool of goroutines (like a boss-worker pattern). But I still went with the current approach (continuously loop and resend) since there is minimal delay and we don't have to wait for the ACK sequentially.

Fixes: https://github.com/argoproj-labs/argocd-agent/issues/117

codecov-commenter commented 6 days ago

Codecov Report

Attention: Patch coverage is 44.24460% with 155 lines in your changes missing coverage. Please review.

Project coverage is 47.21%. Comparing base (72eaea7) to head (2525ac2).

Files with missing lines Patch % Lines
internal/event/event.go 58.49% 62 Missing and 4 partials :warning:
principal/event.go 26.00% 36 Missing and 1 partial :warning:
agent/connection.go 0.00% 24 Missing :warning:
agent/inbound.go 23.80% 14 Missing and 2 partials :warning:
principal/apis/eventstream/eventstream.go 50.00% 9 Missing and 3 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #225 +/- ## ========================================== + Coverage 46.82% 47.21% +0.38% ========================================== Files 57 57 Lines 4952 5151 +199 ========================================== + Hits 2319 2432 +113 - Misses 2452 2529 +77 - Partials 181 190 +9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

jannfis commented 5 days ago

Great stuff, @chetan-rns!

I'm starting a review cycle. Please bear with me, as my reviews are dripping in :)