cli_wallet get_relative_account_history result incorrect when limit > 100 and start is too big #298
Comments
for issue bitshares#298
|
we should not allow a limit greater than 100, pull: #305 fix it. |
|
sample with patch: |
|
I think someone should fix this properly instead of placing arbitrary limits that may cause additional distasteful side effects for those of us that use this daily. |
|
@destenson there is always some limit for pagination in bitshares core. 100 is most of the time the number selected. the issue and fix have nothing to do with if 100 is a good number as a limit for pagination; is actually a bug in the intended action. |
|
Ok, well this may not be the same issue that I've been dealing with for the past year, but my wallet now has 385 keys in it, and overall performance has always been horrendous, lately it's been especially slow and rarely able to recover. Among the things I've noticed is console.log() continuously outputting the following error message repeated dozenes of times per second: It apparently tries to 'get_full_accounts' for every account in the wallet every second, and it simply can't keep up. In a quick & dirty analysis of a log taken over 7 minutes, I found it prints this message 69 times for each unique account, once every 6 seconds on average. It's very bursty though, printing out few hundred errors over the course of a couple seconds then nothing for several seconds & repeat.... My point is, I think my issues are related these pagination issues within bitshares-core. If there is a hard-limit of 100 throughout, then it should handle these situations more gracefully, (without errors, incorrect output, or bad UX). Wallets should probably be prevented from adding more than 100 accounts during creation, or have the ability to be split into smaller wallets. And the code that opens processes these accounts should catch that the list to process is too long far before an assertion in native code does. It should check that the length is within reason first, and if not, not try to keep loading them anyway every 1-5 seconds causing degradation of performance quickly approaching unusable. |
|
@destenson your point is totally valid, it is just that should go here: the confusion is because in this issue(#298) the 100 is for pagination. this pagination is used for pulling big amounts of data in blocks, for example you ask the code for the last 100 results with 0,100. next 100 results with 100, 200, next with 200,300 and so on. In the case of #295 the number is 100 but it is a different thing as the 100 is not acting as a pagination there but as a limitation on how many accounts a user can be getting updates at the same time. lets continue the discussion about it at #295 |
* new pull for issue 298 * Revert "finish sentence in readme" This reverts commit bf76c59. * Revert "Add api-access console log to node startup when present" This reverts commit 80d81ac. * add support for start=0 * changes to rvalues * Update application.cpp * Update application.cpp * Update application.cpp * Update README.md
get_relative_account_historycommand / API in CLI returns incorrect result whenlimitis more than100andstartis too big.For example, the last record of
get_relative_account_history openledger 0 101 12345678command is same to the first record, although the first 100 records are correct.The text was updated successfully, but these errors were encountered: