fix: Print `StateRecord`s corresponding to the range of a state part generated #8798

nikurt · 2023-03-24T23:49:03Z

To help us understand why creation of certain parts is as slow as 10 second, look at the corresponding StateRecords for the first and the last node of the state part.

Based on #8794

The dump needs to be of the state at the beginning of the last block of the previous epoch.

Using `get_state_response_header()` instead of reimplementing it.

…p` also print the beginning and end of corresponding StateRecords.

akhi3030 · 2023-03-27T06:16:56Z

@marcelo-gonzalez: can you review please?

The dump needs to be of the state at the beginning of the last block of the previous epoch.

Using `get_state_response_header()` instead of reimplementing it.

tools/state-viewer/src/state_parts.rs

marcelo-gonzalez · 2023-03-28T03:51:31Z

tools/state-viewer/src/state_parts.rs

+    for item in trie.iter().unwrap() {
+        if let Ok((key, value)) = item {
+            let key_len = key.len();
+            if let Some(sr) = StateRecord::from_raw_key_value(key, value) {


Totally fine if this is what's actually useful, but is printing out the first and last state record exactly required? looks like it's just to print it out as debugging info, so it feels to me like quite a bit of work to call StateRecord::from_raw_key_value on every node just to print only that out in the end. For the debugging purposes here would it do to just call Trie::print_recursive() w/ a low max depth?

After trying this code out, I simplified:

Removed the node key length

Print only the first state record, not the last

print_recursive() would be good, but I don't want too much output to stderr.

Are we still on track to increase protocol version each release?

Making two changes to fuzzer at once -- adding 1.32.1 and 1.33.0 releases.

This was added in #6778, but it's honestly really not needed. There's been some discussion that it's easier to just run mock-node directly, and that the benchmarks don't add a ton of value. Also it's not great to have added binary files to the tree.

ClientActor::syncing_info() prints log messages when the bool it returns indicating whether syncing is needed doesn't match the current syncing state. The problem is that this doesn't always match up to when the client actually changes its sync status. This function is called in sync_wait_period(), which is just meant to tell how long to wait until trying to call run_sync_step() again. The result is that when all peers have disconnected from a node, it prints terrible log spam like: ``` 2023-03-25T01:20:27.598443Z WARN client: Sync: no peers available, disabling sync 2023-03-25T01:20:27.678527Z WARN client: Sync: no peers available, disabling sync 2023-03-25T01:20:27.701984Z WARN client: Sync: no peers available, disabling sync ... ... ``` and you get similar log spam right after node finds a peer after it had none previously: ``` 2023-03-25T01:30:50.957743Z INFO client: Sync: height: 14, peer id/height: ed25519:GyUjwR2XmFHrhsQsrW69NLKnZym6mVYqB4BLcHudv6gx/100, enabling sync 2023-03-25T01:30:50.976113Z INFO client: Sync: height: 14, peer id/height: ed25519:GyUjwR2XmFHrhsQsrW69NLKnZym6mVYqB4BLcHudv6gx/100, enabling sync 2023-03-25T01:30:51.057412Z INFO client: Sync: height: 14, peer id/height: ed25519:GyUjwR2XmFHrhsQsrW69NLKnZym6mVYqB4BLcHudv6gx/100, enabling sync ... ... ``` To fix it we can just have ClientActor::syncing_info() return a type that tells if we need to sync, and only print a log message in run_sync_step() when we actually change the sync status

* fix: Epoch Sync implementation is delayed * fix: Epoch Sync implementation is delayed --------- Co-authored-by: near-bulldozer[bot] <73298989+near-bulldozer[bot]@users.noreply.github.com>

… be (#8810) Fix: #8809

This PR builds on top of #8792. It continues the work mainlining #8323. Ignore the first three commits to review only the changes actually brought by this PR

This version has added support for revoked ssh keys and has revoked the leaked github key. Seems like a thing to be better-safe-than-sorry about.

State Sync remains important functionality that needs to be tested, and which is expected to work on networks that are smaller than testnet/mainnet TODO: Run Nayduck before merging

#8818) First off, we should always use `safe_add_gas` when adding gas to avoid bugs and for code consistency, even in cases when we know there won't be an overflow. Moreover, in this particular case, I actually wasn't able to strictly prove that the value never overflows as it depends on the send fees set in the config. This is technically a protocol change. In practice, we don't expect this addition to ever overflow and if this actually happens in practice, I argue that it's better to loudly fail rather than corrupt the state of the network by acting on the overflowed value of the `total_gas_burnt`.

* protocol_upgrade_num_epochs * reduce_wait_for_missing_block * announce_account_horizon epoch_sync_enabled is also useless, will follow-up, untangling it is too involved to be included in the same PR. #7842

Using `get_state_response_header()` instead of reimplementing it.

nikurt · 2023-03-30T10:59:38Z

@akhi3030 please take a look

…generated (#8798) To help us understand why creation of certain parts is as slow as 10 second, look at the corresponding `StateRecord`s for the first and the last node of the state part. Based on #8794

…generated (near#8798) To help us understand why creation of certain parts is as slow as 10 second, look at the corresponding `StateRecord`s for the first and the last node of the state part. Based on near#8794

nikurt and others added 10 commits March 24, 2023 12:11

fix: State dump creates dumps at the correct point

fea54f4

The dump needs to be of the state at the beginning of the last block of the previous epoch.

Metrics

428234c

Merge branch 'master' into nikurt-dump-off-by-one-block

4eef20c

fix

313d441

fmt

c28321e

Merge branch 'master' into nikurt-dump-off-by-one-block

074e289

Merge branch 'master' into nikurt-dump-off-by-one-block

6cb38f7

Implemented a suggestion from @marcelo-gonzalez.

76564db

Using `get_state_response_header()` instead of reimplementing it.

Merge branch 'master' into nikurt-dump-off-by-one-block

d663fb5

fix: When dumping state parts using `neard view-state state-parts dum…

c8a8d00

…p` also print the beginning and end of corresponding StateRecords.

nikurt requested a review from a team as a code owner March 24, 2023 23:49

nikurt requested a review from akhi3030 March 24, 2023 23:49

Comment

11aec51

akhi3030 requested a review from marcelo-gonzalez March 27, 2023 06:17

nikurt and others added 6 commits March 27, 2023 11:08

Merge branch 'master' into nikurt-analyze-state-dump-contents

b217358

fix: State dump creates dumps at the correct point

b5832ec

The dump needs to be of the state at the beginning of the last block of the previous epoch.

Metrics

9e54cb3

Implemented a suggestion from @marcelo-gonzalez.

c8704e7

Using `get_state_response_header()` instead of reimplementing it.

merge

ddce6e6

merge

9dfc745

robin-near reviewed Mar 27, 2023

View reviewed changes

tools/state-viewer/src/state_parts.rs Outdated Show resolved Hide resolved

marcelo-gonzalez approved these changes Mar 28, 2023

View reviewed changes

posvyatokum and others added 7 commits March 29, 2023 13:04

bump protocol version for 1.33.0 (#8803)

ec6aa9b

Are we still on track to increase protocol version each release?

Fuzz 1.33.0 (#8801)

dba637a

Making two changes to fuzzer at once -- adding 1.32.1 and 1.33.0 releases.

fix: Epoch Sync implementation is delayed (#8807)

70fe1e6

* fix: Epoch Sync implementation is delayed * fix: Epoch Sync implementation is delayed --------- Co-authored-by: near-bulldozer[bot] <73298989+near-bulldozer[bot]@users.noreply.github.com>

fix: Make the logging layer dynamically configure-able, as it used to…

94de451

… be (#8810) Fix: #8809

Build code introduced in #8792 (#8796)

245072e

This PR builds on top of #8792. It continues the work mainlining #8323. Ignore the first three commits to review only the changes actually brought by this PR

nagisa and others added 10 commits March 29, 2023 13:04

Upgrade to rust 1.68.2 (#8820)

81d6189

This version has added support for revoked ssh keys and has revoked the leaked github key. Seems like a thing to be better-safe-than-sorry about.

fix: Update integration and pytests to enable state sync (#8815)

e3b70cf

State Sync remains important functionality that needs to be tested, and which is expected to work on networks that are smaller than testnet/mainnet TODO: Run Nayduck before merging

fix: Remove unused config fields (#8808)

9e7db47

* protocol_upgrade_num_epochs * reduce_wait_for_missing_block * announce_account_horizon epoch_sync_enabled is also useless, will follow-up, untangling it is too involved to be included in the same PR. #7842

Implemented a suggestion from @marcelo-gonzalez.

b1f7a26

Using `get_state_response_header()` instead of reimplementing it.

Metrics

526ea48

simplify

726dc18

simplify

f393dbc

simplify

c934359

Merge branch 'master' into nikurt-analyze-state-dump-contents

60b78ed

nikurt added the S-automerge label Mar 30, 2023

Merge master into nikurt-analyze-state-dump-contents

5ed8ae8

nikurt removed the S-automerge label Mar 30, 2023

nikurt requested review from marcelo-gonzalez and robin-near March 30, 2023 10:59

akhi3030 approved these changes Mar 30, 2023

View reviewed changes

nikurt added the S-automerge label Mar 30, 2023

near-bulldozer bot added 2 commits March 30, 2023 13:16

Merge master into nikurt-analyze-state-dump-contents

6357931

Merge refs/heads/master into nikurt-analyze-state-dump-contents

295dd39

near-bulldozer bot merged commit 2438277 into master Mar 30, 2023

near-bulldozer bot deleted the nikurt-analyze-state-dump-contents branch March 30, 2023 13:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Print `StateRecord`s corresponding to the range of a state part generated #8798

fix: Print `StateRecord`s corresponding to the range of a state part generated #8798

nikurt commented Mar 24, 2023 •

edited

Loading

akhi3030 commented Mar 27, 2023

marcelo-gonzalez Mar 28, 2023

nikurt Mar 29, 2023

nikurt commented Mar 30, 2023

fix: Print StateRecords corresponding to the range of a state part generated #8798

fix: Print StateRecords corresponding to the range of a state part generated #8798

Conversation

nikurt commented Mar 24, 2023 • edited Loading

akhi3030 commented Mar 27, 2023

marcelo-gonzalez Mar 28, 2023

Choose a reason for hiding this comment

nikurt Mar 29, 2023

Choose a reason for hiding this comment

nikurt commented Mar 30, 2023

fix: Print `StateRecord`s corresponding to the range of a state part generated #8798

fix: Print `StateRecord`s corresponding to the range of a state part generated #8798

nikurt commented Mar 24, 2023 •

edited

Loading