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

Feat/17-retry mechanism #49

Merged
merged 8 commits into from
Nov 27, 2024
Merged

Conversation

gurkanguray
Copy link
Contributor

Implement Exponential Retries

This PR implements exponential backoff retry mechanisms throughout the codebase using the cenkalti/backoff library, as suggested in #17.

Changes

  • Added new internal/retry package with:

    • Generic retry functionality with exponential backoff
    • Type-safe retry operations via TypedRetrier
    • Configurable retry parameters
  • Implemented retries in critical operations:

    • Database operations in persist package
    • Admin token operations
    • Secret operations
    • Initialization operations

Implementation Details

  • Created standardized retry configurations with sensible defaults
  • Added proper context handling and cancellation support
  • Implemented comprehensive logging of retry attempts
  • Maintained type safety through generic implementations

Testing

  • Added unit tests for retry mechanisms
  • Verified retry behavior in database operations
  • Tested timeout and cancellation scenarios

Related Issues

Closes #17

Copy link
Contributor

@v0lkan v0lkan left a comment

Choose a reason for hiding this comment

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

I have two change requests:

  • instead of logging use a callback function
  • move the module under pkg

Other than that, it looks amazing 🤤 .

Thanks for your contributions 🎊 .

func(err error, duration time.Duration) {
totalDuration += duration
// log the error, duration and total duration
log.Log().Debug("Retrying operation after error", "error", err.Error(), "duration", duration, "total duration", totalDuration.String())
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of an explicit log, can we have a callback function (passed as an argument)? As in:

notify(err, duration, totalDuration)

That will make this module more generic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a notify option 🙌🏻

Verified

This commit was signed with the committer’s verified signature. The key has expired.
addaleax Anna Henningsen
@v0lkan v0lkan merged commit 54ab369 into spiffe:main Nov 27, 2024
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.

implement exponential retries
2 participants