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 overview of the tasks of a caught-up node #1215

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

amesgen
Copy link
Member

@amesgen amesgen commented Aug 13, 2024

Closes #1057

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

amesgen commented Aug 13, 2024

Rendered

@amesgen amesgen force-pushed the amesgen/node-tasks-overview branch 3 times, most recently from 5c30a4f to 0572d0b Compare August 13, 2024 15:20
@amesgen amesgen marked this pull request as ready for review August 13, 2024 15:21
@amesgen amesgen force-pushed the amesgen/node-tasks-overview branch from 0572d0b to 75dfc10 Compare August 13, 2024 18:57
VolatileDB --> CopyToImmutableDB
CopyToImmutableDB --> ImmutableDB
GarbageCollect -- "cleans up" --> VolatileDB
LedgerDB --> CreateLedgerSnapshot
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it is aligned with the ImmutableDB, but it should be aligned with the volatiledb. Maybe it is not possible with mermaid?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I don't think Mermaid has enough flexibility for stuff like this. Maybe it is possible to do sth with invisible arrows, but I don't think it is too terrible.

docs/website/contents/for-developers/NodeTasks.md Outdated Show resolved Hide resolved
docs/website/contents/for-developers/NodeTasks.md Outdated Show resolved Hide resolved
docs/website/contents/for-developers/NodeTasks.md Outdated Show resolved Hide resolved
docs/website/contents/for-developers/NodeTasks.md Outdated Show resolved Hide resolved
@amesgen amesgen force-pushed the amesgen/node-tasks-overview branch from 75dfc10 to 0b3adba Compare August 14, 2024 09:01
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'm sorry for the delay! I hadn't realized I didn't send my first review pass. I've now read the whole thing.

docs/website/contents/for-developers/NodeTasks.md Outdated Show resolved Hide resolved
docs/website/contents/for-developers/NodeTasks.md Outdated Show resolved Hide resolved
docs/website/contents/for-developers/NodeTasks.md Outdated Show resolved Hide resolved

7. In parallel, chain selection for B continues by validating B.

Time budget: average ~30ms, upper bound ~500ms.
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does 500ms come from? EG the June attack exceeded this -- what's the scope being considered here? Perhaps it's none-buggy-block validation 😬

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing that out, I chose that number somewhat arbitrarily. According to the recent db-analyser pass, there are only 1596 blocks with block application mutator time higher than 500ms, out of which 1574 (all but 22 blocks) are due to the ref script DoS.

I added a "based on empirical data" caveat.

@amesgen amesgen force-pushed the amesgen/node-tasks-overview branch from 0b3adba to e290eed Compare September 4, 2024 16:34
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.

This is really helpful. Thank you for taking the time to write this up 🫶

docs/website/contents/for-developers/NodeTasks.md Outdated Show resolved Hide resolved
docs/website/contents/for-developers/NodeTasks.md Outdated Show resolved Hide resolved
docs/website/contents/for-developers/NodeTasks.md Outdated Show resolved Hide resolved
@amesgen amesgen force-pushed the amesgen/node-tasks-overview branch from e290eed to d594504 Compare September 9, 2024 10:21
@amesgen amesgen force-pushed the amesgen/node-tasks-overview branch from d594504 to 562071e Compare September 9, 2024 12:33
@amesgen amesgen enabled auto-merge September 9, 2024 15:51
@amesgen amesgen added this pull request to the merge queue Sep 9, 2024
Merged via the queue into main with commit 5a1409b Sep 9, 2024
18 checks passed
@amesgen amesgen deleted the amesgen/node-tasks-overview branch September 9, 2024 17:21
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.

Compose an overview of the tasks of a node, with a focus on performance
4 participants