fix: remove delay from last request latency call #3579
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sdbondi
force-pushed
the
comms-rpc-improve-last-request-latency
branch
from
cb5dd78 to
7251172
Compare
sdbondi
force-pushed
the
comms-rpc-improve-last-request-latency
branch
from
7251172 to
eb8b815
Compare
hansieodendaal
previously approved these changes
Nov 17, 2021
There was a problem hiding this comment.
Looks really promising, thanks @sdbondi, will revert if I still see some funny behaviour, which I doubt.
stringhandler
previously approved these changes
Nov 17, 2021
aviator-app bot
pushed a commit
that referenced
this pull request
Nov 17, 2021
Description --- - Improved base node connection status feedback to the console wallet UI. - Added disconnect logic for the wallet connectivity service to enable closing bad base node connections if RPC connections cannot be obtained, which will force the connection cycle to restart from scratch, rather than just waiting for the connection to become alive again. - ~~Disabled obtaining latency information after obtaining tip information in the wallet's base node monitor, as it sometimes introduces huge additional delays.~~ ~~_**Note:** Fixing this behaviour is marked for another PR._~~ _**(Fixed in PR #3579)**_ Motivation and Context --- - Connectivity status was not always current in the console wallet under certain edge cases, observed when switching base nodes. - Retrying to obtain an RPC connection from a bad base node connection is not efficient; this sometimes results in the base node staying offline for extended (> 30 minutes) durations. How Has This Been Tested? --- - System-level testing: - Switching base nodes and observing the connection status while (a) mining and (b) monitoring transactions to be mined. - Stress testing `make-it-rain`. - Stress testing `coin-split`. - Stress testing simaltaneous `make-it-rain` and `coin-split`. See sample profiling measurements below depicting the added delay sometimes introduced by obtaining latency information, _**without the fix mentioned above**_: ``` rust 2021-11-16 14:18:07.490078500 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain RPC client 160 ms 2021-11-16 14:18:08.062610200 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain tip info in 572 ms 2021-11-16 14:18:08.062646600 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain latency info in 0 ms 2021-11-16 14:18:08.062944600 [wallet::base_node_service::chain_metadata_monitor] DEBUG Base node ece4c616a156c1e4fe34d94d8c Tip: 52127 (Synced) Latency: 572 ms 2021-11-16 14:18:37.491546100 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain RPC client 0 ms 2021-11-16 14:20:28.971178900 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain tip info in 111479 ms 2021-11-16 14:25:07.794774200 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain latency info in 278823 ms 2021-11-16 14:25:07.795167000 [wallet::base_node_service::chain_metadata_monitor] DEBUG Base node ece4c616a156c1e4fe34d94d8c Tip: 52127 (Synced) Latency: 11011 ms 2021-11-16 14:25:26.789903600 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain RPC client 0 ms 2021-11-16 14:30:35.914526200 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain tip info in 309124 ms 2021-11-16 14:34:04.569556900 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain latency info in 208654 ms 2021-11-16 14:34:04.569945400 [wallet::base_node_service::chain_metadata_monitor] DEBUG Base node ece4c616a156c1e4fe34d94d8c Tip: 52130 (Synced) Latency: 9699 ms 2021-11-16 14:34:24.885048300 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain RPC client 0 ms 2021-11-16 14:34:44.260586300 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain tip info in 19375 ms 2021-11-16 14:34:44.260734900 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain latency info in 0 ms 2021-11-16 14:34:44.261005000 [wallet::base_node_service::chain_metadata_monitor] DEBUG Base node ece4c616a156c1e4fe34d94d8c Tip: 52133 (Synced) Latency: 10712 ms 2021-11-16 14:35:03.552244200 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain RPC client 0 ms 2021-11-16 14:35:22.417825800 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain tip info in 18865 ms 2021-11-16 14:35:22.417869200 [wallet::base_node_service::chain_metadata_monitor] TRACE Obtain latency info in 0 ms 2021-11-16 14:35:22.418075300 [wallet::base_node_service::chain_metadata_monitor] DEBUG Base node ece4c616a156c1e4fe34d94d8c Tip: 52134 (Synced) Latency: 18865 ms ```
* development: feat: improve wallet connectivity status for console wallet (tari-project#3577) v0.21.1 feat: add error codes to LibWallet for CipherSeed errors (tari-project#3578) ci: split cucumber job into two (tari-project#3583) feat(wallet): import utxo’s as EncumberedToBeReceived rather than Unspent (tari-project#3575) docs: rfc 0250_Covenants (tari-project#3574)
sdbondi
dismissed stale reviews from stringhandler and hansieodendaal
via
fde4f9b
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Motivation and Context
Ref #3577
A delay could be experienced when calling get_last_request_latency because a single client runs on a single task and so can only handle one service request at a time.
How Has This Been Tested?
Added unit test