Bitshares-core: !!! GrapheneApi error: get_full_accounts [Array[1], true]

[fundon]

!!! GrapheneApi error:  get_full_accounts [Array[1], true] 
Object {code: 1, message: "Assert Exception: std::distance(_subscribed_accounts.begin(), _subscribed_accounts.end()) < 100: ", data: Object}

{"code":1,"message":"Assert Exception: std::distance(_subscribed_accounts.begin(), _subscribed_accounts.end()) < 100: ","data":{"code":10,"name":"assert_exception","message":"Assert Exception","stack":[{"context":{"level":"error","file":"database_api.cpp","line":621,"method":"get_full_accounts","hostname":"","thread_name":"th_a","timestamp":"2017-05-31T15:07:18"},"format":"std::distance(_subscribed_accounts.begin(), _subscribed_accounts.end()) < 100: ","data":{}},{"context":{"level":"warn","file":"websocket_api.cpp","line":122,"method":"on_message","hostname":"","thread_name":"th_a","timestamp":"2017-05-31T15:07:18"},"format":"","data":{"call.method":"call","call.params":[2,"get_full_accounts",[["1.2.175587"],true]]}}]}}

[fundon]

I have some accounts, when switch an account, throw the error and loading all the time.

[abitmore]

More than 100 accounts? That's in the message.

[abitmore]

Perhaps this is an UI issue? Pinging @svk31

[svk31]

No I think this is an issue with the new subscriptions in the database api added by elmato.

[elmato]

Yes, we left a limit of 100 subscribed account per session which seems
reasonable enough.

@Fangdung do you want to subscribe to all the accounts?

On Jun 1, 2017 05:07, "Sigve Kvalsvik" [e-mail] wrote:

No I think this is an issue with the new subscriptions in the database api
added by elmato.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/bitshares/bitshares-core/issues/295#issuecomment-305421059,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAOUovt315kEzIp5iXiZO3xEprctL0WFks5r_nFIgaJpZM4Nrxgy
.

[elmato]

@Fangdung how many accounts do you have in your wallet?

On Jun 1, 2017 10:38, "Matias Romeo" [e-mail] wrote:

Yes, we left a limit of 100 subscribed account per session which seems
reasonable enough.

@Fangdung do you want to subscribe to all the accounts?

On Jun 1, 2017 05:07, "Sigve Kvalsvik" [e-mail] wrote:

No I think this is an issue with the new subscriptions in the database api
added by elmato.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/bitshares/bitshares-core/issues/295#issuecomment-305421059,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAOUovt315kEzIp5iXiZO3xEprctL0WFks5r_nFIgaJpZM4Nrxgy
.

[fundon]

@elmato I don't subscribe account more than 100.

[elmato]

@fundon can you give us a way to reproduce the error?

[abitmore]

@svk31 yes, I guess the subscription is done by UI, imho the new API limit is reasonable, best if UI can adapt to it.

[abitmore]

When switching to an account, the UI tries to fetch history, there are other accounts in the history, so.. is it a UI issue?

[svk31]

Well, in some cases the ui needs to use get_full_account because it's the easiest way to get a lot of account data, but it doesn't necessarily want to subscribe to those accounts. I can check if it's possible to reduce the dependance on get_full_account.

[fundon]

@abitmore @svk31 @elmato

1. I have tow computers: iMac and Macbook Pro, then sometimes use the notebook, sometimes use the computer, open the Bitshares client.

1. I subscribe some accounts and markets on iMac or Macbook.

2. Now the Bitshares client still throws that error, I can not transfer to any accounts.

[fundon]

@abitmore @svk31 @elmato
Please fix this big issue.
Currently, I can not transfer to anyone. 😭

[fundon]

@xeroc @wackou @oxarbitrage
Please look at this issue. Thanks.

[fundon]

I have 99 accounts.

[oxarbitrage]

please try pointing your wallet to the the following node for a test:

ws://23.94.69.140:8090

node is compiled increasing the accounts that you can be subscribed from <100 to <200.

FC_ASSERT( std::distance(_subscribed_accounts.begin(), _subscribed_accounts.end()) < 200 );

if this solve your problem we can discuss how much will be a good number to have here.

let me know after tested.

[fundon]

@oxarbitrage

Thank you very much!
It's working for me!

[fundon]

if this solve your problem we can discuss how much will be a good number to have here.

The UI should inform the user what happened.

[oxarbitrage]

is the UI already blocking you to create more than 100 accounts ?

i think you may have 100 accounts, the main one and 99 more. the core was working for 99 accounts. i created a patch: https://github.com/bitshares/bitshares-core/issues/306 that allows to be subscribed to 100 accounts instead of 99 to be in sync with the UI.

i am not sure if the UI is already blocking the account creation after the user creates additional 99 accounts. if this is not the case we are going to need to open an issue in the bitshares-ui repo(https://github.com/bitshares/bitshares-ui).

[destenson]

This has been an issue for me for quite some time. I have more than 100 accounts in my wallet and some aren't even accessible. The UI also has horrendous performance when there are many accounts in the wallet. Lately, I'd been using an older backup with fewer users, but then I needed to access one of my accounts that wasn't in it. Now after loading a newer wallet, it won't let me switch to it... I can't access any of my accounts at this moment! It would be nice if someone would come up with some handy tools to split wallet files or manage large numbers of accounts. I know there aren't too many of us with over 100, but I'm sure it affects each one of us that do quite negatively.

[oxarbitrage]

@elmato what can be the consequences of changing the _subscribed_accounts.begin(), _subscribed_accounts.end()) < 100 to just _subscribed_accounts.begin(), _subscribed_accounts.end())

@fundon was able get over his problems by just increasing the node from subscribing from 99 to 100 but @destenson has over 350 accounts(see: https://github.com/bitshares/bitshares-core/issues/298#issuecomment-307581171).

the UI seems to be not blocking after 100 so i guess we should at least consider removing that limitation.

[bowdus]

Hi.
When will this issue fixed? I am using the light wallet, have 99 accounts and cant send to other accounts anymore because of "unknown account" in "TO" field.

Or how can i temporary unsubscribe from some of the 99 accounts?

Thx

[bowdus]

Block Explorer shows no account names too:

[svk31]

@destenson It's not surprsing that performance is bad with that many accounts inside a wallet since the GUI has to fetch API data for every one of those accounts and also keep it in sync. That's just not a use case that was ever considered and frankly I don't think it's something even worth spending time trying to improve. When you have that many accounts you will also run into this backend limitation no matter what I do on the frontend side.

That being said, there is certainly a case of get_full_accounts being used too liberally in the GUI, causing the API to return this error for users that don't necessarily have that many accounts. That's more of a consequence of how Dan L. designed the object structure and the API, where you're basically forced to either use get_full_accounts or make 10-20 separate API calls to get the data you need. Like I said above I'll see if I can make some tweaks so it's used less, but I'm on holiday at the moment so it's low priority for me.

For those running into issues, a quick workaround is to take the private active key of the account you want to use, and login to https://bitshares.org/wallet using the Account model using your account name plus the private key as your password. That will let you use the account.

[bowdus]

@destenson It's not surprsing that performance is bad with that many accounts inside a wallet since the GUI has to fetch API data for every one of those accounts and also keep it in sync. That's just not a use case that was ever considered and frankly I don't think it's something even worth spending time trying to improve. When you have that many accounts you will also run into this backend limitation no matter what I do on the frontend side.

If the user CAN create as many accounts he wants in client/wallet then he WILL DO and it should even work without problems. The end user is not interested whats going on in background. If that many accounts are causing performance problems in client then there should be a upper account limit or another way to limit the api traffic. Maybe unsubsribe from accounts i am not interested at the moment, and because one an account is created there is no way to remove/delete from wallet.

Thx for the workaround.

[abitmore]

The unfollow/ignore button should have some effect.

[abitmore]

A possible behavior change: no matter what the subscription limitation is, whether or not it's able to subscribe to a new account, the account data that requested by this API will be returned. Or say, let the subscription fail silently. Although the client may not know if the subscription has succeeded, in most cases it won't block the UI. @svk31: thoughts?

[bowdus]

Dear Devs

Will this issue be fixed with the 201708-bitsharesui worker?
I can not send anything in wallet since weeks because of this bug.

Thx

[destenson]

All they have to do to fix this is ease the restriction on hard 100 account subscription limit, OR use the API as provided and turn off auto-subscription when calling get_full_accounts fails. That call has two arguments, one to ID or name the account, and one to select auto-subscribe.

As is, auto-subscribe flag is hard-coded to always be true in the GUI. The bugs related to this issue have only occurred for me when auto-subscribe is on. When I turn it off, performance & usability returns. I submitted a simple PR two weeks ago (https://github.com/bitshares/bitsharesjs/pull/3) that stops hard-coding that argument so they can begin to prevent auto-subscription from the GUI, but it doesn't look like anyone's looked at it yet.

Granted, it will take some thought about some things, like when's the most appropriate time to disable auto-subscription, and how to detect the problem cleanly, which accounts to disable, etc. but it wouldn't be prohibitively difficult if they wanted to fix it.

[destenson]

I just realized also that the test: FC_ASSERT( std::distance(_subscribed_accounts.begin(), _subscribed_accounts.end()) <= 100 ); may be flawed as well.

The assumption is that distance will give you the number of items in the container. But, _subscribed_accounts is implemented as a std::set<>, which is "usually implemented as red-black trees" (according to http://en.cppreference.com/w/cpp/container/set). The std::distance function (according to the same site http://en.cppreference.com/w/cpp/iterator/distance) will give undefined results if given a random-access iterator. And as far as I can tell, std::set is not guaranteed to provide a non- random-access iterator. To eliminate that possibility, since it is a tree not a linear structure, and since it knows how many items are in it already, the assertion should just be changed to use std::set<>::size().

FC_ASSERT(_subscribed_accounts.size() <= MAX_ACCT_SUBSCRIPTIONS)

[abitmore]

I think the distance call also kills performance.

[destenson]

Certainly nowhere near as efficient as grabbing a variable pre-loaded with value it's looking for. And since, when this occurs, the client is in a forever event loop that make it keep trying every 5 seconds, it's probably best not to have it do any unnecessary work while it fields the unending stream of requests.

[oxarbitrage]

@destenson i have a node up for testing the distance() to size() change at #364 . please point your wallet to: 23.94.69.140:9999
it is currently syncing but it should be ready in about 30 mins. pls give it a try and let me know if you notice differences.
thanks!

[destenson]

I just tried it, but I think that the web wallet doesn't allow non-wss connections unless they're local. I can try hacking the db & forcing it to ws if it's still up.

[destenson]

Ok, using the electron app, it connected right away. And I didn't notice a difference, except the error message uses .size() now :) I really wasn't expecting any difference for this problem, it was just a likely inefficiency on the server that could potentially affect other people who might not even have many accounts.

[destenson]

@oxarbitrage, if you're up for it a couple of things that would be interesting to try are, eliminate the assert altogether & see what happens, or instead of throwing an assert once it gets full, to bypass the auto-subscribe path and assume that argument is false so it doesn't try to subscribe any more accounts.. I've done the second one manually on the client side & that "fixes" the performance problems once the get_full_accounts calls stop failing.

No matter what, I think the client side needs to be fixed to handle the error appropriately instead of trying forever to no avail.

I just realized too, that you don't have to have a lot of accounts in your wallet. If you're only following a lot of accounts, the issue can also occur. I just confirmed that the assertion occurs after following severall accounts. The performance doesn't degrade immediately, but I think it gets worse as you continue following more and more accounts.

[oxarbitrage]

pls try 23.94.69.140:9999 again. removed the assert and changed it to subscribe only to the first 100 accounts https://github.com/bitshares/bitshares-core/pull/364/commits/dbf51aa2c7d700808933f8632969525fcee8b8ec

[bowdus]

pls try 23.94.69.140:9999 again. removed the assert and changed it to subscribe only to the first 100 accounts dbf51aa

Yes, for me its working, other users are showing.

[destenson]

@oxarbitrage, For a quick hack fix, It works well. There's little, if any performance degradation and no obvious features missing or not working. All accounts appear where they should. There could be unknown side effects of having some accounts unsubscribed, but I haven't noticed any and, overall it works really well.

In future versions of the GUI, It might be nice to let a user choose which accounts & objects get subscribed to dynamically.. Like extending the "follow" concept to include accounts you do and don't have keys for, and assets and markets. Maybe even others' trades. It could be cool to be able to subscribe to the second-lowest ask in a market list and possibly be alerted when a price level is reached or when a certain open order gets filled.

[oxarbitrage]

it is an improve, i think we should merge it. if @abitmore agree will do so.

[abitmore]

This workaround should work in most cases. People who have more than 100 accounts in the wallet may still be unable to receive updates of some accounts, which should be handled in UI.

[abitmore]

I just realized too, that you don't have to have a lot of accounts in your wallet. If you're only following a lot of accounts, the issue can also occur.

I think the client automatically subscribes to all the account names appeared in the client. For example, when you occasionally clicked into openledger-wallet or poloniexwallet's account which has a history of transferring to many different accounts.

[oxarbitrage]

with the last changes in core and UI i am closing the issue.