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

Initialize a "preflight guide" #879

Merged
merged 2 commits into from
Jan 22, 2024
Merged

Initialize a "preflight guide" #879

merged 2 commits into from
Jan 22, 2024

Conversation

amesgen
Copy link
Member

@amesgen amesgen commented Jan 12, 2024

Prompted by a suggestion by @RenateEilers, this PR initializes a "preflight guide" that strives to outline various steps (potentially with some dependencies) with the goal to build understanding of the overall Cardano system, with a focus on tasks that are relevant for working in the Consensus team.

I also opened a companion epic at #887 which contains ideas for further steps.

Rendered

@amesgen amesgen self-assigned this Jan 12, 2024
@amesgen amesgen added documentation Improvements or additions to documentation no changelog labels Jan 12, 2024
Copy link
Member

@dnadales dnadales left a comment

Choose a reason for hiding this comment

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

We should think about how to link this document to other "entry point" documents, such as our CONTRIBUTING guidelines.

docs/website/contents/for-developers/PreflightGuide.md Outdated Show resolved Hide resolved
docs/website/contents/for-developers/PreflightGuide.md Outdated Show resolved Hide resolved
@amesgen amesgen mentioned this pull request Jan 16, 2024
3 tasks
@amesgen amesgen marked this pull request as ready for review January 16, 2024 15:57
@amesgen amesgen requested a review from a team as a code owner January 16, 2024 15:57
@amesgen
Copy link
Member Author

amesgen commented Jan 16, 2024

We should think about how to link this document to other "entry point" documents, such as our CONTRIBUTING guidelines.

WDYT about a0f7a05?

Copy link
Contributor

@jasagredo jasagredo left a comment

Choose a reason for hiding this comment

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

This whole file looks awesome!
"What I Wish I Knew When Working In Consensus"

@amesgen amesgen added this pull request to the merge queue Jan 22, 2024
@amesgen amesgen removed this pull request from the merge queue due to a manual request Jan 22, 2024
@amesgen amesgen added this pull request to the merge queue Jan 22, 2024
Merged via the queue into main with commit fe245ac Jan 22, 2024
14 checks passed
@amesgen amesgen deleted the amesgen/preflight branch January 22, 2024 13:43
Copy link
Contributor

@jorisdral jorisdral left a comment

Choose a reason for hiding this comment

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

Late to the party: very nice!

[Magic Wormhole]: https://github.com/magic-wormhole/magic-wormhole
[db-analyser]: https://github.com/IntersectMBO/ouroboros-consensus/blob/main/ouroboros-consensus-cardano/README.md#db-analyser
[Cardano ledger]: https://github.com/IntersectMBO/cardano-ledger
[Glossary]: https://ouroboros-consensus.cardano.intersectmbo.org/docs/for-developers/Glossary
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it also be valid to provide a relative path to the file instead of a link to the hosted website? When the website is hosted, it will resolve the relative paths correctly (I think)

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, I didn't think of that for some reason. I am too lazy to open a PR for that, let's 🙏 that we don't migrate our website to a different place soon 😄

Here, "validity" refers most prominently to the ledger rules ("Does this person actually have enough money to make that payment?") of the [Cardano ledger].
On disk, the file name of a ledger snapshot usually contains the slot number of its tip point.

### Running db-analyser passes
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember that we note somewhere in the docs that you should probably use db-analyser with a copy of an immutable DB, because it might mess with the contents a bit

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I locally always use db-analyser directly with the ImmDB of my local node, didn't have problems so far, but that doesn't mean there are no problems. Did you have a specific db-analyser functionality in mind that might be destructive?

Another point: Unless one uses some copy-on-write filesystem (I am still using boring ext4), using a copy of the ImmDB seems quite wasteful, given its current size of >160GB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation no changelog
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

6 participants