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

OpenExchangeRate being hit 2x as often as retrieve_every configuration. #152

Closed
dbernheisel opened this issue Oct 4, 2023 · 5 comments
Closed

Comments

@dbernheisel
Copy link

The ExchangeRate Retriever seems to have duplicate timers scheduled whenever retrieve_every is configured, this results in the OpenExchangeRates (of whichever configured api module) to fetch rates 2x as often.

I believe it's happening here:

if is_integer(config.retrieve_every) do
log(config, :info, log_init_message(config.retrieve_every))
schedule_work(0)
schedule_work(config.retrieve_every)
end

Which both land here:

def handle_info(:latest_rates, config) do
retrieve_latest_rates(config)
schedule_work(config.retrieve_every)
{:noreply, config}
end

Let's try to track it, and let's assume retrieve_every is 1000ms.

  1. 0ms - We initialize the Retriever genserver.
  2. 0ms - schedule_work(0) casts a message to immediately fetch exchange rates. This is put into mailbox. Expected to process immediately after initializing.
  3. 0ms - schedule_work(1000) casts another message to fetch exchange rates 1000ms from now.
  4. 1ms - Genserver has initialized
  5. 2ms - The first :latest_rates message is processed in the mailbox, the API is immediately hit and now we schedule_work(1000) again, expected around 1002ms (now + 1000ms).
  6. 1000ms - The second message in the mailbox is now processed, we hit the API again and schedule another timer for 1000ms, expected at 2000ms (now + 1000ms).
  7. 1002ms - The third message in the mailbox is now processed, and we hit the API yet again and schedule another timer for 100ms, expected at 2002ms (now + 1000ms).
  8. and so forth. We have 2 duplicate timers at this point past startup.

One solution I've done to avoid this issue is to store a reference of the timer in the genserver state, and whenever scheduling work, you cancel the timer and reset it.

@kipcole9
Copy link
Owner

kipcole9 commented Oct 4, 2023

@dbernheisel thanks again for a fabulously detailed report. I will dig into this later today. Clearly not acceptable to be consuming at twice the expected rate.

@kipcole9
Copy link
Owner

kipcole9 commented Oct 7, 2023

I changed

   schedule_work(0) 
   schedule_work(config.retrieve_every)

to be simply:

   schedule_work(0) 

And then let it run for a few iterations and I believe there is now only a single event loop.

Would you consider trying this by configuring ex_money from github? If you see the correct behaviour then I'll publish a new release right away.

Of course please re-open if the issue isn't fixed as you see it.

@dbernheisel
Copy link
Author

Yep. I'll report back on Monday

@dbernheisel
Copy link
Author

Confirmed fixed. Thanks so much.

@kipcole9
Copy link
Owner

kipcole9 commented Oct 9, 2023

I've published ex_money version 5.15.1 with the following changelog entry:

Bug Fixes

Thanks again for the report!

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

No branches or pull requests

2 participants