-
Notifications
You must be signed in to change notification settings - Fork 257
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
[Responsys] Setting timeout to 30 seconds #2693
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2693 +/- ##
=======================================
Coverage 78.52% 78.52%
=======================================
Files 1033 1033
Lines 18702 18702
Branches 3548 3548
=======================================
Hits 14686 14686
Misses 2824 2824
Partials 1192 1192 ☔ View full report in Codecov by Sentry. |
hi @seg-leonelsanches thanks for the PR. |
Hi @joe-ayoub-segment. No problem. Centrifuge team deployed the timeout on their end today. We should be good to go with this PR, then. |
@seg-leonelsanches did you test this on stage? Also, can you link to the Centrifuge PR here? Thanks! |
Hi @marinhero. This was not tested in Stage. The modification in Centrifuge apparently doesn't have an associated PR. |
Okay. How did we confirm this indeed works then? Another question, why are we reducing the PET time from 6 to 3 seconds if we are mentioning that this is the slowest responsys endpoint? Wouldn't we want to increase the time instead of decreasing? |
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.
Please provide test evidence. Also, would love to have more context on the change and its expected effects specially for the PET wait interval. Thanks!
Six seconds is causing this destination to ETimeout all the events in a particular customer. As no other logs are generated for this error, I'm experimenting with decreasing the wait time for this endpoints, since it is the one that takes most of the execution time. |
After deployong in Stage, I was able to test an audience with this destination. However, the ETimeout errors persists. https://app.segment.build/nick-test/destinations/actions-responsys/sources/personas_nick-test-dev/instances/67216bbb86a7770306d97b08/event-delivery?period=past-day |
Some of our customers are facing Timeout errors in their Responsys integration. By default, Centrifuge waits up to 10 seconds for an Action to complete. This integration is taking a much longer time to sync large audiences.
This PR expands the Timeout to 30 seconds, as well as waiting 3 seconds to create a PET (the slowest Responsys endpoint).
Testing