-
Notifications
You must be signed in to change notification settings - Fork 390
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 ZFS.md #1694
Add ZFS.md #1694
Conversation
WalkthroughThe documentation has been enhanced with two significant updates. An additional section titled "Optional: enable on-fly compression with ZFS" has been added to the existing Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
nodes/zfs.md (2)
25-25
: Use a hyphen for the compound adjective "Bare Metal".-1. Bare Metal server with decent amount of RAM (64GB+) +1. Bare-Metal server with decent amount of RAM (64GB+)Tools
LanguageTool
[uncategorized] ~25-~25: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ia-bridge/ ``` ::: ## Requirements: 1. Bare Metal server with decent amount of RAM (64GB+...(EN_COMPOUND_ADJECTIVE_INTERNAL)
31-33
: Specify the language for shell command code blocks.For better syntax highlighting and readability, specify the language as "sh" for the following code blocks:
- Lines 31-33
- Lines 37-38
- Lines 42-43
- Lines 47-48
- Lines 53-54
- Lines 59-60
- Lines 64-65
- Lines 72-74
- Lines 77-79
- Lines 82-84
- Lines 96-97
Example:
-``` +```sh lsblk --nodeps -o nameAlso applies to: 37-38, 42-43, 47-48, 53-54, 59-60, 64-65, 72-74, 77-79, 82-84, 96-97 </blockquote></details> </blockquote></details> <details> <summary>Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>Commits</summary> Files that changed from the base of the PR and between 95dc48c08af2229039346c9dbb7f25720b0aab54 and 9c2fb73e883a8710626e20548081cf55f90f4388. </details> <details> <summary>Files selected for processing (2)</summary> * nodes/bridge-node.md (1 hunks) * nodes/zfs.md (1 hunks) </details> <details> <summary>Additional context used</summary> <details> <summary>LanguageTool</summary><blockquote> <details> <summary>nodes/zfs.md</summary><blockquote> [uncategorized] ~25-~25: If this is a compound adjective that modifies the following noun, use a hyphen. Context: ...ia-bridge/ ``` ::: ## Requirements: 1. Bare Metal server with decent amount of RAM (64GB+... (EN_COMPOUND_ADJECTIVE_INTERNAL) </blockquote></details> <details> <summary>nodes/bridge-node.md</summary><blockquote> [grammar] ~209-~209: The word “setup” is a noun. The verb is spelled with a white space. Context: ...ith ZFS Follow the [tutorial on how to setup your DA node to use on-fly compression ... (NOUN_VERB_CONFUSION) </blockquote></details> </blockquote></details> <details> <summary>Markdownlint</summary><blockquote> <details> <summary>nodes/zfs.md</summary><blockquote> 24-24: Punctuation: ':' Trailing punctuation in heading (MD026, no-trailing-punctuation) --- 28-28: Punctuation: ':' Trailing punctuation in heading (MD026, no-trailing-punctuation) --- 11-11: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 18-18: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </blockquote></details> </blockquote></details> </details> <details> <summary>Additional comments not posted (2)</summary><blockquote> <details> <summary>nodes/zfs.md (1)</summary><blockquote> `1-97`: **Documentation looks great!** The documentation provides a clear and comprehensive guide for setting up ZFS compression on a DA node. The step-by-step instructions, command examples for different network environments, and additional notes make it easy for users to follow along and implement ZFS compression on their nodes. Great work on putting together this helpful resource! <details> <summary>Tools</summary> <details> <summary>LanguageTool</summary><blockquote> [uncategorized] ~25-~25: If this is a compound adjective that modifies the following noun, use a hyphen. Context: ...ia-bridge/ ``` ::: ## Requirements: 1. Bare Metal server with decent amount of RAM (64GB+... (EN_COMPOUND_ADJECTIVE_INTERNAL) </blockquote></details> <details> <summary>Markdownlint</summary><blockquote> 24-24: Punctuation: ':' Trailing punctuation in heading (MD026, no-trailing-punctuation) --- 28-28: Punctuation: ':' Trailing punctuation in heading (MD026, no-trailing-punctuation) --- 11-11: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 18-18: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </blockquote></details> </details> </blockquote></details> <details> <summary>nodes/bridge-node.md (1)</summary><blockquote> `205-209`: **LGTM!** The addition of the optional section for enabling on-the-fly compression using ZFS is a valuable enhancement to the bridge node setup instructions. It provides users with additional options for optimizing their node performance. <details> <summary>Tools</summary> <details> <summary>LanguageTool</summary><blockquote> [grammar] ~209-~209: The word “setup” is a noun. The verb is spelled with a white space. Context: ...ith ZFS Follow the [tutorial on how to setup your DA node to use on-fly compression ... (NOUN_VERB_CONFUSION) </blockquote></details> </details> </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- nodes/bridge-node.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- nodes/bridge-node.md
hmm @Wondertan I think we'd probably advise not to include this hack in docs? |
@jcstein, mentioning that in the docs may be helpful, especially if storage issues are pressing for node runners, as it's ~ a 2x reduction. |
@kinrokinro, for how long have you been running the node with compression on? Do you see it running stable? |
For a month and yes, it running stable, our bridge is always in sync and everything looks pretty good to me, that's the reason why I recommend to add this option to the docs. Anyways we always can mark it as "not recommended" if there is any concern from your side. BTW zstd-3 is a light compression, max is zstd-19. |
Currently we're running both our bridges on testnet and mainnet on zfs, so our metrics is available in your OTEL |
thank you for the context and feedback! I think it is safe to recommend this if @Wondertan approves. And then I will work into the sidebar menu |
I think zstd-3 is optimal as with more zstd-x more time is needed to compress so with bigger load node could be behind sync all the time. |
confirming with @kinrokinro that this uses zstd-3? |
according to the doc that @kinrokinro wrote - yes, it uses zstd-3 and with this compression he achieved 2.05x ratio. According to it level more than zstd-10 leads to crazy time downgrade and ratio isn't so huge to sacrifice with performance. I'm going to try zstd-5 with my mocha bridge node to see if no issues with performance and what compression ratio I will get comparing to zstd-3, probably just after full sync ask to share @kinrokinro and compare with what I got. |
Yes, we use |
@kinrokinro do you want to add this to the menu? or think that it is okay as is, linked in the "optional" section? |
This should be optional of course |
It would be great to add here that processor is more important than RAM here, tried with 2x Intel Xeon Silver 4108 but it doesn't fly, have to request more powerful. Bridge node just syncs too slow and CPU is always more than 90% while RAM (64GB) about 50%. |
Perhaps it because of more powerful compression you choose |
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.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
nodes/zfs.md (2)
24-27
: Add a hyphen to "bare metal" and remove the colon.The requirements section provides clear hardware specifications. However, please make the following minor changes:
- Add a hyphen to "bare metal" to make it "bare-metal" when used as a compound adjective.
- Remove the colon at the end of the heading to adhere to the Markdown style guide.
Apply this diff:
-## Requirements: +## Requirements -1. A bare metal server with a substantial amount of RAM (64GB or more) and a modern CPU (latest generation EPYC or Xeon with a clock speed of 2.1GHz or higher and 32 threads or higher is recommended) +1. A bare-metal server with a substantial amount of RAM (64GB or more) and a modern CPU (latest generation EPYC or Xeon with a clock speed of 2.1GHz or higher and 32 threads or higher is recommended)Tools
LanguageTool
[uncategorized] ~25-~25: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...-bridge/ ``` ::: ## Requirements: 1. A bare metal server with a substantial amount of RAM...(EN_COMPOUND_ADJECTIVE_INTERNAL)
Markdownlint
24-24: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
28-97
: Address the static analysis hints and remove the colon from the heading.The guide section is well-structured and provides clear instructions for setting up ZFS compression. The command examples for different network environments are helpful. However, please address the following static analysis hints:
- Remove the colon from the heading to adhere to the Markdown style guide.
- Specify the language for fenced code blocks to improve syntax highlighting.
- Add a comma after "In case of using a snapshot" to improve readability.
Apply this diff:
-## Guide: +## Guide Get your disk name: ```sh @@ -89,7 +89,7 @@ celestia bridge start --node.store /celestia/bridge/.celestia-bridge-arabica-11 ::: :::tip NOTE -It is recommended to sync from scratch. In case of using a snapshot it is important to have your local route to `--data.store` identical to one in a snapshot. +It is recommended to sync from scratch. In case of using a snapshot, it is important to have your local route to `--data.store` identical to one in a snapshot. ::: After completing the steps above, you can begin syncing your DA node.Tools
LanguageTool
[uncategorized] ~45-~45: You might be missing the article “a” here.
Context: ... apt install zfsutils-linuxCreate ZFS pool:
sh zpool create $ZFS_POOL_NAME...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~89-~89: A comma might be missing here.
Context: ...o sync from scratch. In case of using a snapshot it is important to have your local rout...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Markdownlint
28-28: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- nodes/zfs.md (1 hunks)
Additional context used
LanguageTool
nodes/zfs.md
[uncategorized] ~25-~25: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...-bridge/ ``` ::: ## Requirements: 1. A bare metal server with a substantial amount of RAM...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~45-~45: You might be missing the article “a” here.
Context: ... apt install zfsutils-linuxCreate ZFS pool:
sh zpool create $ZFS_POOL_NAME...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~89-~89: A comma might be missing here.
Context: ...o sync from scratch. In case of using a snapshot it is important to have your local rout...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Markdownlint
nodes/zfs.md
24-24: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
28-28: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
11-11: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
18-18: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (1)
nodes/zfs.md (1)
1-23
: Introduction section looks good!The introduction provides a clear and informative overview of the purpose and benefits of using ZFS compression for a DA node. The note effectively illustrates the storage savings achieved with ZFS compression compared to EXT4 without compression.
Tools
Markdownlint
11-11: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
18-18: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Will provide here much more info on issues that I got with charts, just still in syncing state. But after switching from 2x Intel Xeon Silver 4108 to 2x Intel Xeon Gold 6130 my node is flying very well, just want to see final result on compression ratio with level what we set (zstd-5). |
not completely, unfortunately. Also adding to decreasing in performance, on the screenshot below you can see that node progresses with 20k blocks every 3 hours now (sadly). Hope more capable I/O drives will solve it. |
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.
Actionable comments posted: 4
🧹 Nitpick comments (4)
how-to-guides/zfs.md (3)
47-48
: Add recommended ZFS pool options for performance.The pool creation command should include additional recommended options for optimal performance.
-zpool create -o ashift=12 $ZFS_POOL_NAME /dev/nvme0n1 +zpool create \ + -o ashift=12 \ + -o autoexpand=on \ + -o autoreplace=on \ + -O atime=off \ + -O compression=zstd-3 \ + $ZFS_POOL_NAME /dev/nvme0n1
30-33
: Add disk requirement validation.Add a step to verify disk size and type before proceeding.
Get your disk name: ```sh lsblk --nodeps -o name + +# Verify disk size and type +lsblk --nodeps -o name,size,type,model--- `62-65`: **Add explanation for compression level choice.** Based on the PR discussion, explain why zstd-3 is recommended over higher levels. ```diff Enable compression: +:::tip +We recommend `zstd-3` as it provides a good balance between compression ratio and performance. Higher compression levels (e.g., zstd-19) may cause increased CPU usage and potentially affect node synchronization. +::: ```sh zfs set compression=zstd-3 $ZFS_POOL_NAME/$ZFS_DATASET_NAME
</blockquote></details> <details> <summary>how-to-guides/bridge-node.md (1)</summary><blockquote> `206-209`: **Enhance section with compression benefits.** Add context about when users might want to consider ZFS compression. ```diff ### Optional: enable on-fly compression with ZFS +ZFS compression can significantly reduce storage requirements for your bridge node, potentially achieving a 2x reduction in disk usage. This is particularly useful for nodes with storage constraints or those looking to optimize storage costs. However, ensure your system meets the hardware requirements before proceeding. + Follow the [tutorial on how to set up your DA node to use on-fly compression with ZFS](./zfs.md).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
how-to-guides/bridge-node.md
(1 hunks)how-to-guides/zfs.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
how-to-guides/zfs.md
[uncategorized] ~25-~25: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...-bridge/ ``` ::: ## Requirements: 1. A bare metal server with a substantial amount of RAM...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~89-~89: Possible missing comma found.
Context: ...o sync from scratch. In case of using a snapshot it is important to have your local rout...
(AI_HYDRA_LEO_MISSING_COMMA)
[duplication] ~101-~101: Possible typo: you repeated a word.
Context: ...an try the following steps: ### Disable Auto-Trim Auto-trim disabling can improve I/O performance, ...
(ENGLISH_WORD_REPEAT_RULE)
🪛 Markdownlint (0.37.0)
how-to-guides/zfs.md
24-24: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
28-28: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
11-11: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
18-18: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
@kinrokinro can you please quickly review coderabbits suggestions before we merge? |
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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
how-to-guides/zfs.md (3)
11-21
: Add language specifiers to code blocks.The code blocks should specify their language for proper syntax highlighting.
-``` +```bash $ zfs get compressratio celestia && du -h /celestia/bridge/.celestia-bridge NAME PROPERTY VALUE SOURCE celestia compressratio 2.05x - 2.6T /celestia/bridge/.celestia-bridge-
+
bash
$ du -h ~/.celestia-bridge/
5.0T /home/ubuntu/.celestia-bridge/🧰 Tools
🪛 Markdownlint (0.37.0)
11-11: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
18-18: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
24-24
: Remove trailing punctuation from heading.Remove the colon from the heading to follow Markdown best practices.
-## Requirements: +## Requirements🧰 Tools
🪛 Markdownlint (0.37.0)
24-24: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
46-48
: Add error handling guidance.Provide guidance on common errors during pool creation and how to resolve them.
Create ZFS pool: ```sh zpool create -o ashift=12 $ZFS_POOL_NAME /dev/nvme0n1 + +# Verify pool creation was successful +zpool status $ZFS_POOL_NAME
+:::tip Troubleshooting
+If you encounter permission errors, ensure you're running the command with sudo.
+If the disk is in use, you may need to runzpool labelclear /dev/nvme0n1
first.
+:::</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 65de6dd281ac79f49f431d7102a87643f06ded55 and 6e31a4d422e84a7d0234c5321c6bf11e06e30466. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `how-to-guides/zfs.md` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>how-to-guides/zfs.md</summary> [uncategorized] ~25-~25: If this is a compound adjective that modifies the following noun, use a hyphen. Context: ...-bridge/ ``` ::: ## Requirements: 1. A bare metal server with a substantial amount of RAM... (EN_COMPOUND_ADJECTIVE_INTERNAL) --- [uncategorized] ~89-~89: A comma might be missing here. Context: ...o sync from scratch. In case of using a snapshot it is important to have your local rout... (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA) --- [duplication] ~101-~101: Possible typo: you repeated a word. Context: ...an try the following steps: ### Disable Auto-Trim Auto-trim disabling can improve I/O performance, ... (ENGLISH_WORD_REPEAT_RULE) --- [uncategorized] ~108-~108: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around. Context: ...off $ZFS_POOL_NAME ``` :::tip NOTE You always can trim maually: `sudo zpool trim $ZFS_POO... (AI_EN_LECTOR_REPLACEMENT_WORD_ORDER) </details> </details> <details> <summary>🪛 Markdownlint (0.37.0)</summary> <details> <summary>how-to-guides/zfs.md</summary> 24-24: Punctuation: ':' Trailing punctuation in heading (MD026, no-trailing-punctuation) --- 28-28: Punctuation: ':' Trailing punctuation in heading (MD026, no-trailing-punctuation) --- 11-11: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 18-18: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (4)</summary><blockquote> <details> <summary>how-to-guides/zfs.md (4)</summary> `7-8`: **Add performance impact disclaimer.** Based on the PR discussion, we should add a clear disclaimer about potential performance implications of ZFS compression. --- `24-26`: **Enhance hardware requirements section.** The CPU requirement should be more specific about compression overhead. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [uncategorized] ~25-~25: If this is a compound adjective that modifies the following noun, use a hyphen. Context: ...-bridge/ ``` ::: ## Requirements: 1. A bare metal server with a substantial amount of RAM... (EN_COMPOUND_ADJECTIVE_INTERNAL) </details> <details> <summary>🪛 Markdownlint (0.37.0)</summary> 24-24: Punctuation: ':' Trailing punctuation in heading (MD026, no-trailing-punctuation) </details> </details> --- `99-100`: **Add warning about fine-tuning risks.** The introduction to fine-tuning should include a stronger warning about potential risks. --- `111-119`: **Strengthen warning about sync disabled.** The current warning about sync disabled needs to be more explicit about data loss risks. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 3
♻️ Duplicate comments (2)
how-to-guides/zfs.md (2)
29-32
:⚠️ Potential issueAdd disk validation steps.
Before proceeding with disk operations, it's crucial to verify the disk is empty and safe to use.
Get your disk name: ```sh lsblk --nodeps -o name + +# Verify disk is empty (should show no partitions) +lsblk /dev/nvme0n1 + +# Verify disk is not mounted +mount | grep nvme0n1 + +# Check if disk contains any data +sudo dd if=/dev/nvme0n1 bs=1M count=1 | hexdump -C
124-128
: 🛠️ Refactor suggestionAdd persistence configuration for prefetch setting.
The prefetch setting will not persist across system reboots.
### Disable prefetch Disabling reduces memory usage but can slow down performance for sequential read workloads. ```sh echo 1 | sudo tee /sys/module/zfs/parameters/zfs_prefetch_disable + +# For persistence across reboots, add to /etc/modprobe.d/zfs.conf: +echo "options zfs zfs_prefetch_disable=1" | sudo tee -a /etc/modprobe.d/zfs.conf
🧹 Nitpick comments (2)
how-to-guides/zfs.md (2)
18-25
: Enhance hardware requirements section.Consider adding storage requirements and hyphenating "bare metal".
## Requirements: 1. A bare metal server with: +1. A bare-metal server with: - RAM: 64GB or more - CPU: Latest generation EPYC or Xeon with: - Clock speed: 2.1GHz or higher - Threads: 32 or higher - Note: Additional CPU overhead is required for ZFS compression + - Storage: + - NVMe SSD recommended for optimal performance + - Minimum capacity based on network: + - Mainnet Beta: 2TB+ + - Testnet: 1TB+ 2. At least one empty disk (with no filesystem)🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...essionzstd-3
: ## Requirements: 1. A bare metal server with: - RAM: 64GB or more ...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint (0.37.0)
18-18: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
103-107
: Clarify auto-trim section and fix duplication.The auto-trim section contains word duplication and could be clearer about the implications.
-### Disable Auto-Trim -Auto-trim disabling can improve I/O performance, but may lead to increased SSD wear over time. +### Disable Auto-Trim +Disabling auto-trim can improve I/O performance by reducing background operations. However, this may lead to: +- Increased SSD wear over time +- Reduced SSD performance as blocks fill up +- Need for manual trim scheduling🧰 Tools
🪛 LanguageTool
[duplication] ~103-~103: Possible typo: you repeated a word.
Context: ...an try the following steps: ### Disable Auto-Trim Auto-trim disabling can improve I/O performance, ...(ENGLISH_WORD_REPEAT_RULE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
how-to-guides/zfs.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
how-to-guides/zfs.md
[uncategorized] ~19-~19: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ession zstd-3
: ## Requirements: 1. A bare metal server with: - RAM: 64GB or more ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~44-~44: You might be missing the article “a” here.
Context: ... apt install zfsutils-linux Create ZFS pool:
sh zpool create -o ashift=12 $...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~88-~88: A comma might be missing here.
Context: ...o sync from scratch. In case of using a snapshot it is important to have your local rout...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[duplication] ~103-~103: Possible typo: you repeated a word.
Context: ...an try the following steps: ### Disable Auto-Trim Auto-trim disabling can improve I/O performance, ...
(ENGLISH_WORD_REPEAT_RULE)
🪛 Markdownlint (0.37.0)
how-to-guides/zfs.md
7-7: null
Multiple headings with the same content
(MD024, no-duplicate-heading)
18-18: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
27-27: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Very edge case, but just let it be
I would mark compression as optional since we are using shwap in mainnet and testnet, and there is no so huge numbers in saving storage. |
Could you add? |
@jcstein coderabbit issues resolved, there is nothing to add from my side for now |
We've migrated our mainnet bridge to the |
We've done the same and works very well, numbers less but still about 20-30% in saving storage. I meant maybe to make optional as maybe somebody doesn't want to compression on as it's required a bit stronger hardware during syncing. Anyway, up to you, just a suggestion. |
Co-authored-by: Josh Stein <[email protected]>
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
how-to-guides/zfs.md (1)
152-156
: 🛠️ Refactor suggestionAdd persistence configuration for prefetch setting.
The prefetch setting will not persist across system reboots.
Disabling reduces memory usage but can slow down performance for sequential read workloads. ```sh echo 1 | sudo tee /sys/module/zfs/parameters/zfs_prefetch_disable + +# For persistence across reboots, add to /etc/modprobe.d/zfs.conf: +echo "options zfs zfs_prefetch_disable=1" | sudo tee -a /etc/modprobe.d/zfs.conf</blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (3)</summary><blockquote> <details> <summary>how-to-guides/zfs.md (3)</summary><blockquote> `7-9`: **Enhance the warning message with specific performance implications.** Based on the PR discussion, we should provide more specific details about performance implications. ```diff :::warning -Using ZFS compression may impact node performance depending on your hardware configuration. Ensure your system meets the recommended requirements before proceeding. This is an optional optimization that may not be suitable for all deployments. +Using ZFS compression may impact node performance depending on your hardware configuration: +- CPU: Additional overhead for compression/decompression operations +- Memory: Increased RAM usage for ZFS's adaptive replacement cache (ARC) +- Storage: Potential I/O performance impact during high load + +While this optimization can significantly reduce storage requirements, it should be thoroughly tested in a non-production environment first. This is an optional configuration that may not be suitable for all deployments. :::
28-35
: Add monitoring recommendations for resource usage.To help users ensure their hardware meets the requirements during operation, we should add monitoring guidance.
- Clock speed: 2.1GHz or higher - Threads: 32 or higher - Note: Additional CPU overhead is required for ZFS compression + +Monitor your resource usage with: +- CPU: `top` or `htop` to watch compression overhead +- Memory: `arc_summary` to monitor ZFS ARC usage +- I/O: `zpool iostat` to track ZFS pool performance🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...-bridge/ ``` ::: ## Requirements: 1. A bare metal server with: - RAM: 64GB or more ...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint (0.37.0)
28-28: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
116-116
: Fix grammatical issues for better readability.Several minor grammar issues need attention:
-It is recommended to sync from scratch. In case of using a snapshot it is important to have your local route to `--data.store` identical to one in a snapshot. +It is recommended to sync from scratch. When using a snapshot, it is important to have your local route to `--data.store` identical to the one in the snapshot. -### Disable Auto-Trim -Auto-trim disabling can improve I/O performance, but may lead to increased SSD wear over time. +### Disable Auto-Trim +Disabling auto-trim can improve I/O performance but may lead to increased SSD wear over time. -You always can trim maually: `sudo zpool trim $ZFS_POOL_NAME` +You can always trim manually: `sudo zpool trim $ZFS_POOL_NAME`Also applies to: 131-132, 138-138
🧰 Tools
🪛 LanguageTool
[uncategorized] ~116-~116: A comma might be missing here.
Context: ...o sync from scratch. In case of using a snapshot it is important to have your local rout...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
how-to-guides/zfs.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
how-to-guides/zfs.md
[uncategorized] ~29-~29: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...-bridge/ ``` ::: ## Requirements: 1. A bare metal server with: - RAM: 64GB or more ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~72-~72: You might be missing the article “a” here.
Context: ... apt install zfsutils-linux Create ZFS pool:
sh zpool create -o ashift=12 $...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~116-~116: A comma might be missing here.
Context: ...o sync from scratch. In case of using a snapshot it is important to have your local rout...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[duplication] ~131-~131: Possible typo: you repeated a word.
Context: ...an try the following steps: ### Disable Auto-Trim Auto-trim disabling can improve I/O performance, ...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~138-~138: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ...off $ZFS_POOL_NAME ``` :::tip NOTE You always can trim maually: `sudo zpool trim $ZFS_POO...
(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
[uncategorized] ~143-~143: Possible missing comma found.
Context: ...abling sync provides faster write speeds but significantly increases the risk of dat...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint (0.37.0)
how-to-guides/zfs.md
28-28: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
37-37: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
15-15: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
22-22: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
how-to-guides/zfs.md (1)
13-15
: 🛠️ Refactor suggestionComplete the tip note with compression level details.
Based on the PR discussion about compression levels, we should provide more context about why zstd-3 is recommended.
:::tip NOTE -ZFS, compression `zstd-3`: +ZFS with compression level `zstd-3` is recommended because: +- Provides optimal balance between compression ratio and CPU usage +- Higher levels (e.g., zstd-19) may cause increased processing time +- Lower levels may not provide sufficient compression +- Based on user experience, zstd-3 keeps nodes in sync without performance issuesLikely invalid or redundant comment.
🧰 Tools
🪛 Markdownlint (0.37.0)
15-15: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
how-to-guides/zfs.md (1)
162-166
: 🛠️ Refactor suggestionAdd persistence configuration for prefetch setting.
The prefetch setting will reset after system reboot:
### Disable prefetch Disabling reduces memory usage but can slow down performance for sequential read workloads. ```sh echo 1 | sudo tee /sys/module/zfs/parameters/zfs_prefetch_disable + +# For persistence across reboots, add to /etc/modprobe.d/zfs.conf: +echo "options zfs zfs_prefetch_disable=1" | sudo tee -a /etc/modprobe.d/zfs.conf</blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (4)</summary><blockquote> <details> <summary>how-to-guides/zfs.md (4)</summary><blockquote> `13-26`: **Enhance the compression ratio example with context.** Based on the PR discussion, add context about why `zstd-3` was chosen and what users can expect: ```diff :::tip NOTE -ZFS, compression `zstd-3`: +ZFS with compression level `zstd-3` provides an optimal balance of compression and performance: +- Chosen for its balance of CPU usage vs compression ratio +- Higher levels (>10) offer diminishing returns +- Lower CPU overhead compared to higher compression levels + +Real-world compression results:
🧰 Tools
🪛 Markdownlint (0.37.0)
15-15: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
22-22: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
28-35
: Clarify bare metal requirement and add memory allocation guidance.The requirements section should explain why bare metal is preferred and provide guidance on memory allocation for ZFS:
## Requirements: 1. A bare metal server with: + > Why bare metal? Virtual environments may introduce additional overhead and unpredictable I/O performance. - RAM: 64GB or more + > Memory allocation guidance: + - Reserve 1GB RAM per 1TB of storage for ZFS Address Resolution Table (ART) + - Additional RAM improves ZFS's Adaptive Replacement Cache (ARC) performance - CPU: Latest generation EPYC or Xeon with: - Clock speed: 2.1GHz or higher - Threads: 32 or higher - Note: Additional CPU overhead is required for ZFS compression🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...-bridge/ ``` ::: ## Requirements: 1. A bare metal server with: - RAM: 64GB or more ...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint (0.37.0)
28-28: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
44-52
: Improve disk validation example.The disk validation commands should show example output to help users interpret results:
Verify disk is empty (should show no partitions): ```sh -lsblk YOUR_DISK_NAME (/dev/nvme0n1 or /dev/sda i.e.) +# Example: lsblk /dev/nvme0n1 +# Expected output for an empty disk: +# NAME MAJ:MIN RM SIZE RO TYPE +# nvme0n1 259:0 0 1.8T 0 disk + +# If you see entries with 'part' type, the disk is not emptyVerify disk is not mounted:
-mount | grep YOUR_DISK_NAME +# Example: mount | grep nvme0n1 +# Expected output: no results if disk is not mounted--- `125-127`: **Clarify snapshot usage with ZFS.** The note about snapshots needs more context and guidance: ```diff :::tip NOTE -It is recommended to sync from scratch. In case of using a snapshot it is important to have your local route to `--data.store` identical to one in a snapshot. +It is recommended to sync from scratch when using ZFS for the first time. If you must use a snapshot: +1. Ensure your local path matches the snapshot's `--data.store` path exactly +2. The snapshot must be from a ZFS-based node with compatible compression settings +3. Verify the snapshot's integrity before proceeding :::
🧰 Tools
🪛 LanguageTool
[uncategorized] ~126-~126: A comma might be missing here.
Context: ...o sync from scratch. In case of using a snapshot it is important to have your local rout...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
how-to-guides/zfs.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
how-to-guides/zfs.md
[uncategorized] ~29-~29: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...-bridge/ ``` ::: ## Requirements: 1. A bare metal server with: - RAM: 64GB or more ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~72-~72: You might be missing the article “a” here.
Context: ... apt install zfsutils-linux Create ZFS pool:
sh zpool create -o ashift=12 $...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~126-~126: A comma might be missing here.
Context: ...o sync from scratch. In case of using a snapshot it is important to have your local rout...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[duplication] ~141-~141: Possible typo: you repeated a word.
Context: ...an try the following steps: ### Disable Auto-Trim Auto-trim disabling can improve I/O performance, ...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~148-~148: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ...off $ZFS_POOL_NAME ``` :::tip NOTE You always can trim maually: `sudo zpool trim $ZFS_POO...
(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
🪛 Markdownlint (0.37.0)
how-to-guides/zfs.md
28-28: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
37-37: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
15-15: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
22-22: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 2
🧹 Nitpick comments (4)
how-to-guides/zfs.md (4)
13-26
: Enhance the compression ratio example with more context.The compression ratio example would be more valuable with additional context:
- Specify the data size and type being compared
- Note that results may vary based on data characteristics
- Include the time period over which this data was collected
:::tip NOTE -ZFS, compression `zstd-3`: +ZFS with compression level `zstd-3` (after 1 month of node operation): ```sh $ zfs get compressratio celestia && du -h /celestia/bridge/.celestia-bridge NAME PROPERTY VALUE SOURCE celestia compressratio 1.22x - 1.3T /celestia/bridge/.celestia-bridge-EXT4, no compression:
+EXT4 without compression (same node, same time period):$ du -h ~/.celestia-bridge/ 1.8T /home/ubuntu/.celestia-bridge/
+Note: Actual compression ratios may vary based on your node's data characteristics and workload patterns.
:::<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.37.0)</summary> 15-15: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 22-22: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `28-35`: **Enhance storage requirements specification.** The requirements should include specific storage recommendations based on the compression ratio example. ```diff ## Requirements: 1. A bare-metal server with: - RAM: 64GB or more - CPU: Latest generation EPYC or Xeon with: - Clock speed: 2.1GHz or higher - Threads: 32 or higher - Note: Additional CPU overhead is required for ZFS compression + - Storage: + - NVMe SSD recommended for optimal performance + - Minimum capacity: 2TB (provides ~2.4TB effective capacity with typical compression ratio) + - RAID configuration recommended for production deployments
🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...-bridge/ ``` ::: ## Requirements: 1. A bare metal server with: - RAM: 64GB or more ...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint (0.37.0)
28-28: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
108-121
: Improve network-specific path examples.The network-specific paths should follow a consistent pattern and include a note about permissions.
```sh [Mainnet Beta] -# Add flag --node.store /celestia/bridge/.celestia-bridge to your command, example: +# Add flag --node.store /$ZFS_POOL_NAME/$ZFS_DATASET_NAME/.celestia-bridge to your command, example: celestia bridge start --metrics.tls=true --metrics --metrics.endpoint otel.celestia.observer --p2p.metrics --node.store /celestia/bridge/.celestia-bridge-# Add flag --node.store /celestia/bridge/.celestia-bridge-mocha-4 to your command, example: +# Add flag --node.store /$ZFS_POOL_NAME/$ZFS_DATASET_NAME/.celestia-bridge-mocha-4 to your command, example: celestia bridge start --metrics.tls=true --metrics --metrics.endpoint otel.mocha.celestia.observer --p2p.metrics --node.store /celestia/bridge/.celestia-bridge-mocha-4 --p2p.network mocha
-# Add flag --node.store /celestia/bridge/.celestia-bridge-arabica-11 to your command, example: +# Add flag --node.store /$ZFS_POOL_NAME/$ZFS_DATASET_NAME/.celestia-bridge-arabica-11 to your command, example: celestia bridge start --node.store /celestia/bridge/.celestia-bridge-arabica-11 --p2p.network arabica
+:::tip NOTE
+Ensure the celestia user has write permissions to the ZFS dataset:
+sh +sudo chown -R celestia:celestia /$ZFS_POOL_NAME/$ZFS_DATASET_NAME +
+:::--- `125-127`: **Clarify sync recommendation.** The sync recommendation should provide more detail about snapshot considerations. ```diff :::tip NOTE -It is recommended to sync from scratch. In case of using a snapshot it is important to have your local route to `--data.store` identical to one in a snapshot. +It is recommended to sync from scratch when using ZFS compression for optimal results. If using a snapshot: +1. Ensure your local path matches the snapshot's `--data.store` path exactly +2. Verify the snapshot was created with compatible ZFS settings +3. Be aware that compression ratios may differ from a fresh sync :::
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
how-to-guides/zfs.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
how-to-guides/zfs.md
[uncategorized] ~29-~29: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...-bridge/ ``` ::: ## Requirements: 1. A bare metal server with: - RAM: 64GB or more ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[duplication] ~141-~141: Possible typo: you repeated a word.
Context: ...an try the following steps: ### Disable Auto-Trim Auto-trim disabling can improve I/O performance, ...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~153-~153: Possible missing comma found.
Context: ...abling sync provides faster write speeds but significantly increases the risk of dat...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint (0.37.0)
how-to-guides/zfs.md
28-28: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
37-37: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
15-15: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
22-22: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
Overview
Add the documentation on how to run a DA node with ZFS compression
Summary by CodeRabbit