-
Notifications
You must be signed in to change notification settings - Fork 87
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
Track processed events in Azure provider #1527
Conversation
PR Summary
|
/integration sha=cffd25137016b4b4eae2032990dfc81c8e8fee6f |
1 similar comment
/integration sha=cffd25137016b4b4eae2032990dfc81c8e8fee6f |
⌛ Integration tests are running... Check their status here 👈 |
❌ Oh no! Integration tests have failed |
/integration sha=cffd25137016b4b4eae2032990dfc81c8e8fee6f |
⌛ Integration tests are running... Check their status here 👈 |
❌ Oh no! Integration tests have failed |
/integration sha=79aacd308959f34dcad0106d20bc9cb3949a3f9d |
⌛ Integration tests are running... Check their status here 👈 |
✅ Integration tests have finished successfully! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some nitpicking comments and one a bit more important about what to return when any other error happens while storing the dispatched event
packages/framework-provider-azure/src/library/events-adapter.ts
Outdated
Show resolved
Hide resolved
packages/framework-provider-azure/src/library/events-adapter.ts
Outdated
Show resolved
Hide resolved
@alvaroloes all of your comments have been addressed in commit 8b06598 😉 👍 |
/integration sha=8b06598a412b227c4b93d4152d4ff6bba64a3c5a |
⌛ Integration tests are running... Check their status here 👈 |
✅ Integration tests have finished successfully! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but please take a look at the error "bubble up" comment and the login thing, so that it is easier to debug in prod.
Description
The changes in this PR aim to correct a behavior noticed in the Azure provider, where the Cosmos DB change feed would get triggered more than once for the same event, resulting in the same event being processed twice. This behavior has been seen to happen during load-balancing scenarios.
To correct this duplicate-event behavior, a new processed events container is added to Cosmos DB to store the IDs of events that the event processor has processed. The processor will query this container for the event IDs and verify the event has not been processed (i.e., it can't find its ID in this new processed-events container). If an event with the same ID is found, to avoid duplication, the event processor will ignore it and log a warning indicating the duplication.
The IDs store in the processed events container have a time-to-live (TTL) and will get cleared automatically.
Changes
framework-provider-azure-infrastructure
package adding the new container for processed events.processedEventsTtl
toBoosterConfig
for setting the TTL of the event IDs in the processed events container (defaults to 300 seconds, i.e., 5 minutes).BoosterEventProcessor
in theframework-core
package for querying and storing the processed event IDs.framework-provider-azure
package for interacting with the processed events container.Checks