feat: add transactions.mempool.lockedTransactions and chainlocks.blockHeight stats #6237
Conversation
PastaPastaPasta
force-pushed
the
stats-add-basic-islock-clsig
branch
from
8f47ceb to
ab3598b
Compare
|
Guix Automation has began to build this PR tagged as v21.1.0-devpr6237.ab3598b1. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v21.1.0-devpr6237.ab3598b1. The image should be on dockerhub soon. |
|
Guix Automation has began to build this PR tagged as v21.1.0-devpr6237.763e4fcc. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v21.1.0-devpr6237.763e4fcc. The image should be on dockerhub soon. |
|
Guix Automation has began to build this PR tagged as v21.1.0-devpr6237.63241c25. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v21.1.0-devpr6237.63241c25. The image should be on dockerhub soon. |
|
Guix Automation has began to build this PR tagged as v21.1.0-devpr6237.2f26f788. A new comment will be made when the image is pushed. |
|
Applied |
src/llmq/instantsend.cpp
Outdated
| auto time_diff = [&] () -> int64_t { | ||
| LOCK(cs_timingsTxSeen); | ||
| if (auto it = timingsTxSeen.find(islock->txid); it != timingsTxSeen.end()) { | ||
| // This is the normal case where we received the TX before the islock | ||
| return GetTimeMillis() - it->second; | ||
| } | ||
| // But if we received the islock and don't know when we got the tx, then say 0, to indicate we received the islock first. | ||
| return 0; | ||
| }(); | ||
| statsClient.timing("islock_ms", time_diff); |
There was a problem hiding this comment.
any concerns about doing this here? kinda concerned about potential performance implications doing this in this thread. Any thoughts?
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v21.1.0-devpr6237.2f26f788. The image should be on dockerhub soon. |
|
pls see 7aa0a48 + clang format is complaining |
4faf35f chore: add `stats` as a pull request header scope (Kittywhiskers Van Gogh) Pull request description: ## Additional Information Would allow tagging #5167, #6267 and #6237 as `feat(stats)`. ## Breaking Changes None. ## Checklist: - [x] I have performed a self-review of my own code **(note: N/A)** - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ **(note: N/A)** ACKs for top commit: PastaPastaPasta: utACK 4faf35f UdjinM6: utACK 4faf35f thephez: utACK 4faf35f Tree-SHA512: 25cc28792351852b9e920d980b8814d93274bc53d22ce63fb1a5bf32821b10902d22b384a408e1c4a7b97239e53e235c45ac008d0f82e1afe5d6071b392acb47
|
This pull request has conflicts, please rebase. |
| @@ -254,6 +254,9 @@ class CInstantSendManager : public CRecoveredSigsListener | |||
| mutable Mutex cs_pendingRetry; | |||
| std::unordered_set<uint256, StaticSaltedHasher> pendingRetryTxs GUARDED_BY(cs_pendingRetry); | |||
|
|
|||
| mutable Mutex cs_timingsTxSeen; | |||
| std::unordered_map<uint256, int64_t, StaticSaltedHasher> timingsTxSeen GUARDED_BY(cs_timingsTxSeen); | |||
There was a problem hiding this comment.
it seems as this map can indefinitely grow and eat all significant amount of RAM
| if (auto it = timingsTxSeen.find(tx->GetHash()); it == timingsTxSeen.end()) { | ||
| timingsTxSeen[tx->GetHash()] = GetTimeMillis(); | ||
| } |
There was a problem hiding this comment.
let's avoid double lookup on map here:
| if (auto it = timingsTxSeen.find(tx->GetHash()); it == timingsTxSeen.end()) { | |
| timingsTxSeen[tx->GetHash()] = GetTimeMillis(); | |
| } | |
| timingsTxSeen.try_emplace(tx->GetHash(), GetTimeMillis()); |
emplace() and try_emplace() are similar, but difference in performance - one is expecting success by default, other is expecting failure by default
There was a problem hiding this comment.
proof that it works:
$ cat a.cpp
#include <iostream>
#include <unordered_map>
using namespace std;
int main() {
unordered_map<int, int> m;
if (m.try_emplace(2, 3).second) cout << "OK" << endl; else cout << "FAIL" << endl;
if (m.try_emplace(3, 4).second) cout << "OK" << endl; else cout << "FAIL" << endl;
if (m.try_emplace(2, 5).second) cout << "OK" << endl; else cout << "FAIL" << endl;
for (auto i : m) {
cout << i.first << ' ' << i.second << endl;
}
}
$ ./a
OK
OK
FAIL
3 4
2 3
…ead of `ConnectBlock`
PastaPastaPasta
force-pushed
the
stats-add-basic-islock-clsig
branch
from
2f26f78 to
3e52422
Compare
|
Guix Automation has began to build this PR tagged as v22.1.0-devpr6237.3e524228. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v22.1.0-devpr6237.3e524228. The image should be on dockerhub soon. |
|
Guix Automation has began to build this PR tagged as v22.1.0-devpr6237.35745503. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v22.1.0-devpr6237.35745503. The image should be on dockerhub soon. |
Issue being fixed or feature implemented
Add basic lockedTransactions count and chain locked block height to stats
What was done?
How Has This Been Tested?
Will be tested once this has a docker image
Breaking Changes
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.