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

Improve performance of get_ticker API #509

Closed
abitmore opened this issue Nov 27, 2017 · 8 comments
Closed

Improve performance of get_ticker API #509

abitmore opened this issue Nov 27, 2017 · 8 comments

Comments

@abitmore
Copy link
Member

abitmore commented Nov 27, 2017

Currently, get_ticker API walks though the detailed matched/filled order history during exactly last 24 hours (86400 seconds) and aggregates data on the fly, to get the ticker data including total volume, highest price, lowest price, and close price of 24 hours before. It's not a big deal when this API is occasionally called. However, due to its low performance, it may cause high load on API servers if it's regularly called on busy markets, as mentioned in #505.

//Update: see comments below for new proposal.

To improve performance, ticker data should be calculated from bucket data which is aggregated already. However, it is a bit hard to get rolling data of exactly last 86400 seconds. So, I propose to change the definition of "last 24 hours" or "last day" to a time range which includes last 23 full hours and passed time of current hour. For example, * when current time is `2017-11-27T20:20:20`, set time range of ticker data to `[2017-11-26T21:00:00, 2017-11-27T20:20:20)` * if current time is `2017-11-27T20:00:00`, time range of ticker data is `[2017-11-26T20:00:00, 2017-11-27T20:00:00)` * if current time is `2017-11-27T20:00:01`, time range of ticker data is `[2017-11-26T21:00:00, 2017-11-27T20:00:01)`

Pros:

  • With the new definition, ticker data can be get from hourly buckets, so performance would be improved a lot.
  • the result can be verified by checking with hourly chart, so it's easier to explain.

Cons:

  • total volume reported with new definition will be a little lower than before.
@abitmore abitmore added this to the Future Non-Consensus-Changing Release milestone Nov 27, 2017
@oxarbitrage
Copy link
Member

definitely a good idea. i was checking for a way to get the data from the last 23 buckets and by using the old fashion get the exact missing data without sacrificing much performance but it does not look as something doable in a first sight.

let me know if you want me to work on this api change.

@oxarbitrage
Copy link
Member

Note: no need to change get_24_volume as it is already updated to make use of get_ticker. In the past the 2 calls were doing the same whole thing.

@pmconrad
Copy link
Contributor

You can maintain a rolling sum by adding new stuff and subtracting what drops out of the 24-hour-window.

@abitmore abitmore self-assigned this Nov 28, 2017
@abitmore
Copy link
Member Author

Thank you @pmconrad. Maintaining a 24-hour rolling data set seems to be a good idea. So we can have all ticker data pre-calculated when a new blocks are pushed, no longer need to aggregate on the fly.

The main concern would be RAM usage, since we need to store a ticker for each market pair that ever had a trade (at least it will have a latest price). This is not a big deal, by now we have around 2000 assets, so around 2 million market pairs, theoretically we'll need at most 2M * (72 bytes + overhead) ~= 200MB of RAM. Since we don't need to store data for markets that had no trade, the actual RAM usage will be much less, even though there will be more assets in the system. By the way, we don't need to store a copy of history data or bucket data, but only need to store a variable that indicates the minimum history ID of the data set.

Another concern would be that this approach may slightly increase time needed for replay. Also not a big deal IMHO.

I will try to implement in this way.

@abitmore abitmore changed the title Redefine get_ticker API for possible performance improvement Improve performance of get_ticker API Nov 28, 2017
@abitmore abitmore modified the milestones: Future Non-Consensus-Changing Release, Next Non-Consensus-Changing Release - 201712 Nov 28, 2017
@oxarbitrage
Copy link
Member

that will allow us to easy implement an api call to get the top 10 markets by volume as requested by @svk31 here: #171

@abitmore
Copy link
Member Author

@oxarbitrage let's create a new ticket for the top X markets by volume API.

@abitmore
Copy link
Member Author

In current production network, we have around 2200 market pairs that ever had a trade, so I think it only need less than 1MB of RAM to store pre-calculated ticker data.

abitmore added a commit to abitmore/bitshares-core that referenced this issue Nov 30, 2017
abitmore added a commit to abitmore/bitshares-core that referenced this issue Nov 30, 2017
@abitmore
Copy link
Member Author

Fixed with #513, Closing.

@abitmore abitmore removed their assignment Mar 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants