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

add simple pagination to get_asset_holders #312

Merged
merged 1 commit into from Jun 14, 2017

Conversation

oxarbitrage
Copy link
Member

asset_api call get_asset_holders was lacking a pagination, this was causing a single call from client to get over 10k results freezing the application. i added start and limit parameters in order to get the results by chunks of 100 results.

can be called by:

> {"id":1, "method":"call", "params":[2,"get_asset_holders",["1.3.0", 0, 0]]}

> {"id":1, "method":"call", "params":[2,"get_asset_holders",["1.3.0", 0, 100]]}

> {"id":1, "method":"call", "params":[2,"get_asset_holders",["1.3.0", 101, 100]]}

and so on. as the asset_api calls are relative new i think it is safe to add this 2 arguments now.

@vikramrajkumar vikramrajkumar merged commit b307557 into bitshares:master Jun 14, 2017
vikramrajkumar added a commit that referenced this pull request Jun 14, 2017
@abitmore
Copy link
Member

abitmore commented Jun 15, 2017

Will this break old API calls? If yes, best to use a new API.

@abitmore
Copy link
Member

and so on. as the asset_api calls are relative new i think it is safe to add this 2 arguments now.

OK.. Just saw this. Make your own decision.

However,

  1. the design / implementation is inefficient / poor performance when querying with a big page number. I'd recommend avoid doing it this way. Instead, pass in an element and query following X elements. For account balance object, currently need to pass in tuple<asset_type, balance, owner>.
  2. When found a zero balance, break. account_balance_index is ordered descending by balance. Actually I'd suggest that we remove the account balance object from database when balance is zero, but perhaps it need a new ticket.

@oxarbitrage oxarbitrage deleted the get_asset_holders branch November 15, 2018 23:23
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

Successfully merging this pull request may close these issues.

None yet

3 participants