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

Add new analysis --count-blocks #3418

Merged
merged 1 commit into from
Oct 13, 2021
Merged

Conversation

EncodePanda
Copy link
Contributor

cabal run ouroboros-consensus-cardano:db-analyser --\
  --db db_mainnet_with_alonzo_40217333 cardano \
  --configByron dbacfg/mainnet-byron-genesis.json \
  --configShelley dbacfg/mainnet-shelley-genesis.json \
  --nonce 1a3be38bcbb7911969283716ad7aa550250226b76a61fc51cc9a9a35d9276d81 \
  --configAlonzo dbacfg/mainnet-alonzo-genesis.json \
  --only-immutable-db \
  --analyse-from 39842441 \
  --count-blocks

About to count number of blocks processed...
Counted: 16378

=> Analysis blk
countBlocks (AnalysisEnv { db, registry, initLedger, limit }) = do
putStrLn $ "About to count number of blocks processed..."
counted <- processAll db registry GetBlock initLedger limit 0 process
Copy link
Contributor

@nfrisby nfrisby Oct 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't necessarily need GetBlock here. It could be any of the following ctors, since it's unused.

https://github.com/input-output-hk/ouroboros-network/blob/c82309f403e99d916a76bb4d96d6812fb0a9db81/ouroboros-consensus/src/Ouroboros/Consensus/Storage/Common.hs#L148-L167

In particular, it could be GetPure () -- ie "get nothing". If we're wanting this command to identify eg malformed files (eg things that don't successfully deserialize), then that wouldn't work. But I don't get the sense that's what this command is for.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Will make a change here.

Copy link
Contributor

@nfrisby nfrisby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the motivation for this specific command, but it doesn't seem unreasonable and the implementation here is fine. (I suggested one simplification.)

Also, right now this also counts EBBs. Is that right? Does it matter? Should we be explicit about it? If needed: there is a BlockComponent constructor that would let us simultaneously count the number of EBBs, so this command could report both counts.

So: I have clicked Approve.

If we accumulate too many of these "trivial" commands for our liking, we may wish to remember that there are lots of other ways this same query could be answered. EG reporting the first and last block no of some other "more significant" but not "too much slower" command and then printing the difference at the end.

@EncodePanda
Copy link
Contributor Author

I don't know the motivation for this specific command, but it doesn't seem unreasonable and the implementation here is fine. (I suggested one simplification.)

There two motivations:

  1. I wanted to have a clean PR showing how to add a new analysis into the tool (so I could add that to docs)
  2. I actually need to learn how many blocks I see while running the tool for various snapshots, thus this command was helpful and that is why I decided to add it permanently

@EncodePanda EncodePanda force-pushed the EncodePanda/CAD-3383/count-blocks branch from 0634a3a to dca81ef Compare October 11, 2021 08:55
@EncodePanda
Copy link
Contributor Author

changed BlockComponent to GetPure ()

Copy link
Contributor

@nfrisby nfrisby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Thanks!

@EncodePanda
Copy link
Contributor Author

bors merge

iohk-bors bot added a commit that referenced this pull request Oct 11, 2021
3418: Add new analysis --count-blocks r=EncodePanda a=EncodePanda

```
cabal run ouroboros-consensus-cardano:db-analyser --\
  --db db_mainnet_with_alonzo_40217333 cardano \
  --configByron dbacfg/mainnet-byron-genesis.json \
  --configShelley dbacfg/mainnet-shelley-genesis.json \
  --nonce 1a3be38bcbb7911969283716ad7aa550250226b76a61fc51cc9a9a35d9276d81 \
  --configAlonzo dbacfg/mainnet-alonzo-genesis.json \
  --only-immutable-db \
  --analyse-from 39842441 \
  --count-blocks

About to count number of blocks processed...
Counted: 16378
```

Co-authored-by: EncodePanda <paul.szulc@gmail.com>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 11, 2021

Build failed:

@EncodePanda EncodePanda force-pushed the EncodePanda/CAD-3383/count-blocks branch 3 times, most recently from 5fa3dc3 to 36812cf Compare October 13, 2021 09:15
@EncodePanda EncodePanda force-pushed the EncodePanda/CAD-3383/count-blocks branch from 36812cf to 36134cb Compare October 13, 2021 10:10
@EncodePanda
Copy link
Contributor Author

bors merge

iohk-bors bot added a commit that referenced this pull request Oct 13, 2021
3418: Add new analysis --count-blocks r=EncodePanda a=EncodePanda

```
cabal run ouroboros-consensus-cardano:db-analyser --\
  --db db_mainnet_with_alonzo_40217333 cardano \
  --configByron dbacfg/mainnet-byron-genesis.json \
  --configShelley dbacfg/mainnet-shelley-genesis.json \
  --nonce 1a3be38bcbb7911969283716ad7aa550250226b76a61fc51cc9a9a35d9276d81 \
  --configAlonzo dbacfg/mainnet-alonzo-genesis.json \
  --only-immutable-db \
  --analyse-from 39842441 \
  --count-blocks

About to count number of blocks processed...
Counted: 16378
```

Co-authored-by: EncodePanda <paul.szulc@gmail.com>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 13, 2021

Build failed:

@EncodePanda
Copy link
Contributor Author

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 13, 2021

@iohk-bors iohk-bors bot merged commit a6f90a7 into master Oct 13, 2021
@iohk-bors iohk-bors bot deleted the EncodePanda/CAD-3383/count-blocks branch October 13, 2021 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants