About `Money.ExchangeRatesLite`
Opened this issue · 0 comments
Hi, Kip. It's me, again.
When planning for the changes of #154, I read the code of Money.ExchangeRates
, and I gained more thoughts.
Issues
There are some facts about current ex_money
:
- it always starts an application -
Money.Application
. - it always starts the supervision tree in a fixed hierarchy -
Money.ExchangeRates.Supervisor
. - it can only start one
Money.ExchangeRates.Retriever
.
Every fact above may cause one possible issue:
ex_money
is running as an application, and the configuration is a global, which makes it impossible to use it for different exchange rates services, such as https://openexchangerates.org/ and https://exchangeratesapi.io/, etc. (Although this situation is very rare.)- If we want to move
Money.ExchangeRates.Supervisor
to another supervision tree, we have to stop it first, then start it again in another place. Money.ExchangeRates.Retriever
is implemented as a GenServer. And, one GenServer will become a bottleneck in the case of large number of messages. In this case, we would typically start multiple GenServers, but the current implementation is not flexible to implement that.
Solution 1
In order to resolve above issues, I tried to implement a new version of Money.ExchangeRates
. And, in order to avoid introducing breaking changes, I rename my implementation to Money.ExchangeRatesLite
.
When implementing that, I divided the work into four parts:
- a multi-adapter,business-agnostic HTTP client. (when implement it, I deliberately avoided using GenServer)
- a multi-adapter,business-agnostic cache. (when implement it, I deliberately avoided using GenServer, too. Because ETS is good for concurrency, wrapping it with GenServer weakens its capabilities.)
- a multi-adapter retriever
- assemble above components into
Money.ExchangeRatesLite
, and implement it as a GenServer.
To show the code and how to use it, I created a PR and a demo repo:
- The PR: #156 (shouldn't be merged, because it has some issues that I'll mention later)
- The demo repo: https://github.com/plastic-gun/elixir-ex_money-demo-1
With this implementation, above issues can be resolved:
- it is easy to create different exchange rates module for different exchange rates services.
- it is easy start
Money.ExchangeRatesLite
in any supervisor tree. BecauseMoney.ExchangeRatesLite
is a simple GenServer, there's no application or supervisor. - it is easy to create a pool manager for
Money.ExchangeRatesLite
. Because, again,Money.ExchangeRatesLite
is a simple GenServer.
But it has drawbacks:
Money.ExchangeRatesLite
is scheduling retrieve tasks. Running a GenServer individually works fine. However, when running multiple GenServers, they are not aware of each other and always schedule multiple retrieve tasks. For example, if we create a pool with 5Money.ExchangeRatesLite
workers, there will be 5 scheduled retrieve tasks running in the background.
It is possible to eliminate these drawbacks with more code, but I think that would introduce too many assumptions and make :ex_money
less flexible.
So, I don't plan to invest time in this solution any more.
Solution 2
I hope :ex_money
provides a lightweight solution, which allow developers to build or choose their own:
- supervision tree
- caching mechanism
- ...
But, how? The method is to provide just functions.
To show the code and how to use it, I created a PR and a demo repo:
- The PR: #157
- The demo repo: https://github.com/plastic-gun/elixir-ex_money-demo-2
This implementation:
- provides only functions and a helper macro to retrieve exchange rates.
- doesn't provide cache for exchange rates.
- doesn't schedule tasks for retrieving exchange rates internally.
Why? Because:
- functions are the most flexible thing for a library.
- developers has their own preferred method to build cache. And, for an existing system, it should already include an existing cache mechanism. If
:ex_money
provide a caching mechanism, developers would have to modify their code to meet the requirements of:ex_money
. - as mentioned in solution 1, scheduled tasks can cause issues when creating a pool. Therefore, it's better to separate them, and let the developers to decide how to schedule tasks.
What do you think, kip? Is solution2 a good solution from your perspective? Feel free to comment ;).