-
Notifications
You must be signed in to change notification settings - Fork 56
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
doc: added deletion overview with very high level chart. #288 #295
base: main
Are you sure you want to change the base?
Conversation
All the comments has been resolved !! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one more change :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes! 👍
@bari12 Can you verify your requested changes are completed? |
It's good. Thanks @panta-123 and @voetberg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case future changes need to be done, it would be great to have the source code for the flowcharts as part of the documentation repo. Maybe we could use Mermaid.js to handle this?
website/static/img/reaper.png
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"get list of all replica with tombstone" -> "get list of all replicas with tombstone"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two identical "get list of all replicas with tombstone" blocks; the chart could be simplified so that there is only one block, to which you can either get via "Is greedy? YES" or "needed_free_space >= 0"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is another step for needed_free_space so that will complicate things if pointrd to single blocks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rdimaio , The mermaid.js lets create another issue and handle all the chart there.
Added the deletion overveiw in different chapter.
Few things I am not clear.
When setting TOMBSTONE by cleaner and undertaker , what is the value it sets. I look through the code and not clear to me.
The chart are very crude and may need some imporvement suggestion needed here.