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 ZFS.md #1694

Merged
merged 23 commits into from
Jan 22, 2025
Merged

Add ZFS.md #1694

merged 23 commits into from
Jan 22, 2025

Conversation

kinrokinro
Copy link
Contributor

@kinrokinro kinrokinro commented Sep 3, 2024

Overview

Add the documentation on how to run a DA node with ZFS compression

Summary by CodeRabbit

  • New Features
    • Added a comprehensive guide for setting up a DA node with on-the-fly compression using ZFS.
    • Included detailed instructions on hardware requirements, environment setup, and advanced configuration options for optimizing storage efficiency.
    • Introduced an optional section for enabling on-the-fly compression with ZFS in the bridge node documentation.

Copy link
Contributor

coderabbitai bot commented Sep 3, 2024

Walkthrough

The 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 how-to-guides/bridge-node.md file. Furthermore, a new guide has been introduced in how-to-guides/zfs.md, detailing the setup of a Data Availability (DA) node with on-the-fly compression using ZFS. This guide includes hardware requirements, installation steps, and advanced tuning options for users.

Changes

Files Change Summary
how-to-guides/bridge-node.md New section added: "Optional: enable on-fly compression with ZFS."
how-to-guides/zfs.md New document added providing a comprehensive guide for setting up a DA node with ZFS compression, including requirements and detailed instructions.

Poem

🐇 In the meadow where data flows,
A bridge node now with ZFS glows.
Compression on-the-fly, oh what a sight,
Storage efficiency, taking flight!
With each command, we hop and cheer,
Optimizing nodes, the future is here! 🌟


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 name



Also 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 -->

nodes/bridge-node.md Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 9c2fb73 and eb9a83d.

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

@jcstein jcstein requested a review from Wondertan September 4, 2024 15:17
@jcstein
Copy link
Member

jcstein commented Sep 4, 2024

hmm @Wondertan I think we'd probably advise not to include this hack in docs?

@Wondertan
Copy link
Member

@jcstein, mentioning that in the docs may be helpful, especially if storage issues are pressing for node runners, as it's ~ a 2x reduction.

@Wondertan
Copy link
Member

@kinrokinro, for how long have you been running the node with compression on? Do you see it running stable?

@kinrokinro
Copy link
Contributor Author

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.

@kinrokinro
Copy link
Contributor Author

Currently we're running both our bridges on testnet and mainnet on zfs, so our metrics is available in your OTEL

@jcstein
Copy link
Member

jcstein commented Sep 4, 2024

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

@mogoll92
Copy link

mogoll92 commented Sep 9, 2024

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.

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.

@jcstein
Copy link
Member

jcstein commented Sep 9, 2024

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?

@mogoll92
Copy link

mogoll92 commented Sep 9, 2024

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.
I found an interesting discussion regarding ZFS levels, it gives overall info about ZFS levels. Here is:
https://www.reddit.com/r/zfs/comments/sxx9p7/a_simple_real_world_zfs_compression_speed_an/

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.

@kinrokinro
Copy link
Contributor Author

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?

Yes, we use zstd-3 compression level

@jcstein
Copy link
Member

jcstein commented Sep 10, 2024

@kinrokinro do you want to add this to the menu? or think that it is okay as is, linked in the "optional" section?

@kinrokinro
Copy link
Contributor Author

@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

@mogoll92
Copy link

mogoll92 commented Sep 10, 2024

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%.

@kinrokinro
Copy link
Contributor Author

It would be great to add here that processor is more important than RAM here, tried with 2x Intel Xeon Silver 4108 but it does fly, have to request more powerful. Bridge node just syncs too slow and CPU is always more that 90% while RAM (64GB) about 50%.

Perhaps it because of more powerful compression you choose

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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:

  1. Add a hyphen to "bare metal" to make it "bare-metal" when used as a compound adjective.
  2. 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:

  1. Remove the colon from the heading to adhere to the Markdown style guide.
  2. Specify the language for fenced code blocks to improve syntax highlighting.
  3. 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-linux Create 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

Commits

Files that changed from the base of the PR and between eb9a83d and 804a79e.

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-linux Create 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)

@mogoll92
Copy link

mogoll92 commented Sep 10, 2024

It would be great to add here that processor is more important than RAM here, tried with 2x Intel Xeon Silver 4108 but it does fly, have to request more powerful. Bridge node just syncs too slow and CPU is always more that 90% while RAM (64GB) about 50%.

Perhaps it because of more powerful compression you choose

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).

@jcstein
Copy link
Member

jcstein commented Sep 12, 2024

so this baead03 should resolve the issue you were facing @mogoll92 ?

@mogoll92
Copy link

mogoll92 commented Sep 12, 2024

so this baead03 should resolve the issue you were facing @mogoll92 ?

not completely, unfortunately.
With more synced data performance of data node has been decreasing. I thought that 2x Intel Xeon Gold 6130 solved that issue as it more powerful and I didn't see huge load on it, around 50% with +/- 10%. On screenshot you see that node reached 1.5m blocks very fast, but after it had been started degrading in performance. I assume that now it's because of NVMe disks are on PCIe-3 that why node stuck on I/O and it makes load on processor and system overall. I should mentioned that I kept testnet bridge on the server with 2x Intel Xeon Silver 4108 and disks supports PCIe-3 without zfs and node was syncing very well and working as well. Now I rent AMD EPYC 7313P and drives with PCIe-4, will try with that setup.
bad_perfromance

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.
Screenshot 2024-09-12 at 15 36 59

@mindstyle85 mindstyle85 reopened this Jan 21, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 36cc78f and 7671a39.

📒 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)

how-to-guides/zfs.md Outdated Show resolved Hide resolved
how-to-guides/zfs.md Outdated Show resolved Hide resolved
how-to-guides/zfs.md Outdated Show resolved Hide resolved
how-to-guides/zfs.md Outdated Show resolved Hide resolved
@jcstein
Copy link
Member

jcstein commented Jan 21, 2025

@kinrokinro can you please quickly review coderabbits suggestions before we merge?

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 run zpool 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 -->

how-to-guides/zfs.md Show resolved Hide resolved
how-to-guides/zfs.md Show resolved Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 issue

Add 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 suggestion

Add 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: ...ession zstd-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

📥 Commits

Reviewing files that changed from the base of the PR and between 6e31a4d and 53fa8b7.

📒 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)

how-to-guides/zfs.md Show resolved Hide resolved
how-to-guides/zfs.md Outdated Show resolved Hide resolved
how-to-guides/zfs.md Outdated Show resolved Hide resolved
kinrokinro and others added 4 commits January 21, 2025 23:11
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Very edge case, but just let it be
@mogoll92
Copy link

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.

@kinrokinro
Copy link
Contributor Author

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?

@kinrokinro
Copy link
Contributor Author

kinrokinro commented Jan 21, 2025

@jcstein coderabbit issues resolved, there is nothing to add from my side for now

@kinrokinro
Copy link
Contributor Author

kinrokinro commented Jan 21, 2025

We've migrated our mainnet bridge to the zstd-3 ZFS, no issues for now. I don't think there will be a huge difference on zstd-3 in terms of performance, but it still will reduce the disk space usage as it should.

@mogoll92
Copy link

mogoll92 commented Jan 21, 2025

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.

how-to-guides/zfs.md Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 suggestion

Add 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

📥 Commits

Reviewing files that changed from the base of the PR and between 53fa8b7 and 3a1486c.

📒 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 suggestion

Complete 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 issues

Likely invalid or redundant comment.

🧰 Tools
🪛 Markdownlint (0.37.0)

15-15: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

how-to-guides/zfs.md Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 suggestion

Add 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 empty

Verify 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

📥 Commits

Reviewing files that changed from the base of the PR and between 3a1486c and 050e424.

📒 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)

how-to-guides/zfs.md Outdated Show resolved Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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:

  1. Specify the data size and type being compared
  2. Note that results may vary based on data characteristics
  3. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 050e424 and 6817122.

📒 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)

how-to-guides/zfs.md Show resolved Hide resolved
how-to-guides/zfs.md Show resolved Hide resolved
@jcstein jcstein merged commit d0d3963 into celestiaorg:main Jan 22, 2025
3 of 4 checks passed
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.

7 participants