Improve performance of get_account_history_operations API

[abitmore]

The get_account_history API and get_account_history_operations API are inefficient, which can be seen as DoS vulnerabilities for API servers especially full-full nodes.

• get_account_history API
• get_account_history_operations API

[abitmore]

This may break some client applications. So please be careful. Perhaps implement a mechanism for API servers to refuse to serve such API, if there is not one already.

[abitmore]

Actually perhaps the performance can be improved by getting use of the by_op index defined in operation_history_object, not sure why we didn't do so.

[abitmore]

Actually the performance issue about get_account_history_operations API is more serious, since it's not as easy to fix as get_account_history API due to lack of direct index on operation type.

[oxarbitrage]

All the history_api calls including the 2 of this issue should be replaced by their elasticsearch versions by checking if the plugin is active. If so, a database endpoint will be available for the node and return the same data as current calls maintaining all clients compatible. If the plugin is not active don't return anything.

I understand not all full nodes will change the account_history_plugin for the elasticsearch_plugin right away so a temporal fix maybe worth it.

Ill start with get_account_history as it is easier and for get_account_history_operations ideas are welcome.

will like to know what @pmconrad think too.

[pmconrad]

I'm with the UNIX philosophy: One tool for one job.

Integrating an ES proxy into the witness_node makes things unnecessarily complex. If clients need a common endpoint to access both history and live chain data, set a common proxy before both witness_node and ES.

[oxarbitrage]

okay, got it, lets fix. starting on get_account_history now.

[oxarbitrage]

any ideas for get_account_history_operations ?

current implementation do the same as get_account_history and insert only if the operation match the requested type. we can do the same with the better get_account_history we have now but it will still be inefficient.

ideally we should have an composite key index that can track account from account_transaction_history_object and operation_type(can be obtained by doing op.which()) from operation_history_object but i am not sure if this is possible,

suggestions welcome.

[abitmore]

Since get_account_history_operations is useful to an extent, I tend to add an index for it rather than deprecate it. However, if we do this, we need more RAM to run API nodes. Not decided yet, anyway, we can try and get some data (e.g. how much more RAM is needed) for final decision.

[oxarbitrage]

one thing that should work is to add a new field operation_type to the account_transaction_history_object and populate it on account_history_plugin.cpp with op_id(db).op.which().

that should allow us to create new tag by_op_type in operation_history_object.hpp, something like:

         ordered_unique< tag<by_op_type>,
            composite_key< account_transaction_history_object,
               member< account_transaction_history_object, account_id_type, &account_transaction_history_object::account>,
               member< account_transaction_history_object, uint32_t, &account_transaction_history_object::operation_type>
            >
         >,

RAM of course will be increased by saving an additional integer field(operation_type) on each created object.

This option(or similar) was discarded in the past mainly because the data is already in the blockchain so it will be redundant to add again, however, an index that can get data from 2 different objects is something i was not able to find any example on how to do.

if that is possible we can maybe do a good refactor without increasing ram much.

[abitmore]

Partially done. Moving to next milestone.

[jmjatlanta]

however, an index that can get data from 2 different objects is something i was not able to find any example on how to do.

Perhaps I can help. It sounds like you want to build a composite key based on values from 2 different objects, not just the account_transaction_history object, correct? I haven't looked at your situation in detail, but I have a little experience with key extractors. Take a look at https://github.com/bitshares/bitshares-core/pull/1019/files#diff-1ad134c975af5bd661e554a2a77f4767

Perhaps that is something to build on. I hope it helps.

[oxarbitrage]

Many thanks for the comment. i am going to take a look as if i can solve it that way it will be the most efficient as we will use data already available in the blockchain. i have a solution that actually adds a new field to account_transaction_history object but it does not convince me.

resuming the work here to give a try to john suggestion.

if this works, it can also be of value in other issues.

[ryanRfox]

@oxarbitrage I set the status label to 2 Discussion Needed and left it in the To Do bucket on the Feature Release project board. It was not clear from your comment above if you are actively working this, or just plan to in this sprint. Anyway, please update labels and move it to In Development when you are ready. Thanks