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

Blocks file diagnostic tool #1595

Closed
wants to merge 2 commits into from
Closed

Conversation

ronnno
Copy link
Contributor

@ronnno ronnno commented Jul 13, 2020

  • depends on Filesystem metrics and Histogram bug fix #1594
  • add a thin diagnostics_parser.go to wrap the codec. The alternative to using this wrapper is to make Codec public. Which is better? @noambergIL
  • analyze_blocks.go is a command line tool which receives a blocks file, scans all blocks and prints analysis. Currently only a very minimal analysis is done (histogram of block sizes - and one which does not contain all blocks - very basic)

Notice - we can delete the parser class if we make the codec public, the UI tool is easily extendable to answer any question about all blocks in any blocks file.

Tests are missing - need feedback before finalizing

@noambergIL
Copy link
Contributor

This is a main that creates a histogram of block sizes ? i am not sure why should it be in ONG ? and why expose internal stuff ?
this could be a tool outside that reads blocks from file and uses membuf to diagnose

i don't think it belongs to ONG base code

@ronnno
Copy link
Contributor Author

ronnno commented Jul 15, 2020

I agree - it should not be in ONG and that is also why I used _main in the folder. it's not part of the compilation. its an illustration of how you would use the codec or the DiagnosticsParse - it can be removed to a different repo.

that is not the main point. the point is how do we externalize the codec to other apps. there are three options:

  1. make Codec capitalized
  2. create a wrapper such as DiagnosticParse which is capitalized
  3. Copy the Codec to a different repo - and either live with the duplicity of code or convert ONG to use that external dependency.

After implementing DiagnosticParser here - when my aim was to avoid production code changes as much as possible. I now start to think that duplicating the parser code is really the best option with the least friction and dependency to ONG.

What do you think?

@ronnno
Copy link
Contributor Author

ronnno commented Jul 16, 2020

reached the conclusion that it's best to capitalize (make public) the codec and reference it from an outside repo for the diagnostic parser

@ronnno ronnno closed this Jul 16, 2020
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.

2 participants