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

[3.2] Remove hardcoded 10ms limit for chain_api_plugin calls #96

Merged
merged 5 commits into from
Sep 12, 2022

Conversation

heifner
Copy link
Member

@heifner heifner commented Sep 1, 2022

Previously http-max-response-time-ms controlled a limit on time for converting internal representation (variant) to a JSON string. Now http-max-response-time-ms will work as described, "Maximum time for processing a request.". It will put a limit on the time for the complete http call for the chain_api_plugin calls specified below.

http-max-response-time-ms now supports -1 for unlimited.

Similar to abi-serializer-max-time-ms, http-max-response-time-ms is time the main thread is blocked. Be careful setting this very large on public API nodes as it does specify a max time that the main thread can be busy servicing an API request and not validating an incoming block. A block production node should not enable the chain_api_plugin as these limits can interfere with block production windows. If chain_api_plugin is enabled on a block production node, make sure to set both http-max-response-time-ms & abi-serializer-max-time-ms to small values to help avoid interference with the block production window.

  • http-max-response-time-ms - Controls maximum time of execution of the following chain_api_plugin calls.
    • /v1/chain/get_activated_protocol_features
    • /v1/chain/get_table_rows
    • /v1/chain/get_table_by_scope
    • /v1/chain/get_producers
    • /v1/chain/get_scheduled_transactions

The above API calls now all support an optional time_limit_ms defaulting to 10ms to provide previous behavior. Also cleos commands get table, get table scope, and get_producers now take a --time-limit which allows for overriding the default 10ms limit. This time limit will be used as before to limit the returned output. Note the nodeos configured http-max-response-time-ms will take precedence and hard fail any http request taking longer than http-max-response-time-ms . Also --time-limit will be ignored by any nodeos that does not have these PR changes.

Example cleos:

./cleos system listproducers --help
List producers
Usage: ./cleos system listproducers [OPTIONS]

Options:
  -h,--help                   Print this help message and exit
  -j,--json                   Output in JSON format
  -l,--limit UINT             The maximum number of rows to return
  -L,--lower TEXT             Lower bound value of key, defaults to first
  --time-limit UINT           Limit time of execution in milliseconds, defaults to 10ms

This enables setting http-max-response-time-ms to -1 on a local nodeos and specifying --limit 999999 --time-limit 999999 to cleos get table allowing users to retrieve complete table output without multiple calls to get table. Note this will block the main thread during the complete call.

Resolves eosnetworkfoundation/mandel#819

@matthewdarwin
Copy link

A block production node should not enable the chain_api_plugin as these limits can interfere with block production windows.

As far as I know, there is no practical way to monitor a node other than enabling chain_api_plugin to run /v1/chain/get_info requests, even on a producing node. Of course get_info returns very quickly, so not an issue related to this change. Just commenting on the part of "no enable the chain api".

…luding time for main thread and http thread. Puts limit on complete execution including conversion to JSON.
…l_features, get_table_rows, get_table_by_scope, get_producers, get_scheduled_transactions
@heifner heifner added the OCI Work exclusive to OCI team label Sep 6, 2022
@heifner heifner merged commit 0b1c8ab into main Sep 12, 2022
@heifner heifner deleted the http-max-response branch September 12, 2022 17:52
@lparisc lparisc added the documentation Improvements or additions to documentation label Feb 21, 2023
greg7mdp added a commit that referenced this pull request Dec 15, 2023
131/150 Test   #98: producer_schedule_hs_unit_test_eos-vm-jit ..........***Failed    1.36 sec
132/150 Test   #97: producer_schedule_hs_unit_test_eos-vm ..............***Failed    1.57 sec
133/150 Test   #96: producer_schedule_hs_unit_test_eos-vm-oc ...........***Failed    2.86 sec
144/150 Test   #21: api_unit_test_eos-vm-oc ............................***Failed   79.26 sec
146/150 Test   #23: api_unit_test_eos-vm-jit ...........................***Failed   96.61 sec
150/150 Test   #22: api_unit_test_eos-vm ...............................***Failed  467.14 sec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation OCI Work exclusive to OCI team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Honor http-max-response-time-ms for complete http request
4 participants