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

feat: Add Longhorn V2 Data Engine #653

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tserong
Copy link
Contributor

@tserong tserong commented Oct 21, 2024

This adds a new page under Advanced describing how to enable the Longhorn v2 Data Engine, along with its prerequisites and current limitations.

A couple of questions:

  • Regarding the limitations, should this be worded differently?
    • Do I need to add anything about how future versions of Longhorn and Harvester will have more features added?
    • Do I need to go into more detail about the implications of some of the limitations (e.g.: as VMs with V2 volumes can't be live migrated, so will need to be shut down and restarted when upgrading Harvester)?
  • Is there enough detail in "Using the Longhorn V2 Data Engine"? Is it sufficiently obvious that the V2 Data Engine is only available for new additional volumes, and existing volumes, VM images, and VM root volumes remain using the V1 engine?

(We should also update the screenshots and description on the Multi-Disk Management page, but this also probably needs to include information about @Vicente-Cheng's LVM work, so I haven't included that as part of this PR.)

Related issue: harvester/harvester#5274

Copy link

github-actions bot commented Oct 21, 2024

Name Link
🔨 Latest commit fc2355e
😎 Deploy Preview https://67244d9a3963d7a51e6cc952--harvester-preview.netlify.app


If you wish to selectively disable the Longhorn V2 data engine on some nodes (for example on nodes with less CPU and/or RAM), go to the **Hosts** screen and add the label `node.longhorn.io/disable-v2-data-engine` with value `true` to those nodes.

2. Go to the **Hosts** screen and add extra disks to each node as described in [Multi-disk Management](../host/host.md#multi-disk-management). Set the additional disk's `Provisioner` to `Longhorn V2 (CSI)`.
Copy link
Member

Choose a reason for hiding this comment

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

We can also add some screenshots in the multidisk management page for LVM/v2 volumes.

Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

@tserong The structure looks good. I suggested wording improvements and dropped some comments below the suggestions. I'll answer your questions after I've had time to step back and revisit the content with fresher eyes.

docs/advanced/longhorn-v2.md Outdated Show resolved Hide resolved
docs/advanced/longhorn-v2.md Outdated Show resolved Hide resolved
docs/advanced/longhorn-v2.md Outdated Show resolved Hide resolved
docs/advanced/longhorn-v2.md Outdated Show resolved Hide resolved
docs/advanced/longhorn-v2.md Outdated Show resolved Hide resolved
docs/advanced/longhorn-v2.md Outdated Show resolved Hide resolved
docs/advanced/longhorn-v2.md Outdated Show resolved Hide resolved
docs/advanced/longhorn-v2.md Outdated Show resolved Hide resolved
docs/advanced/settings.md Outdated Show resolved Hide resolved
@jillian-maroket
Copy link
Contributor

jillian-maroket commented Oct 23, 2024

Here are my answers:

Do I need to add anything about how future versions of Longhorn and Harvester will have more features added?

No. The "Experimental" label and how the Limitations section is worded already imply that the feature set is incomplete.

Do I need to go into more detail about the implications of some of the limitations (e.g.: as VMs with V2 volumes can't be live migrated, so will need to be shut down and restarted when upgrading Harvester)?

Such details could be useful. We can probably reuse some content from this section of the doc.

Is there enough detail in "Using the Longhorn V2 Data Engine"?

This section leans heavily on other doc topics (Multi-disk Management, Creating a StorageClass). Linking is the correct approach. We just need to ensure that those topics actually contain the information that the user needs.

Is it sufficiently obvious that the V2 Data Engine is only available for new additional volumes, and existing volumes, VM images, and VM root volumes remain using the V1 engine?

We can add an intro paragraph to "Using the Longhorn V2 Data Engine" that contains these details. We can also convert the related paragraph in the "Limitations" section to an "Important" note.

@tserong
Copy link
Contributor Author

tserong commented Oct 31, 2024

Thanks @jillian-maroket, I've updated this now to include all your feedback, which I've squashed into the first commit. I've also added two other commits, one to make important info boxes, and the second to add a note about an obscure issue with disk size. I'm not 100% sure if this is the best place to put it, because the issue only occurs when testing with non-NVMe disks, and we already said that local NVMe disks are a requirement (it probably really belongs in the Longhorn docs - see my comments in the commit message at 314ff94)

tserong and others added 3 commits November 1, 2024 12:27
Related issue: harvester/harvester#5274

Signed-off-by: Tim Serong <tserong@suse.com>
Co-authored-by: Jillian <67180770+jillian-maroket@users.noreply.github.com>
Signed-off-by: Tim Serong <tserong@suse.com>
This one is a little obscure.  When testing with non-NVMe disks,
Longhorn uses the aio bdev driver, which assumes the block size
is 4096 bytes, and thus requires the disk to be an even multiple
of that figure.  We hit this in testing as described in
harvester/harvester#6861.

Ordinarily one should be using NVMe disks, which end up using the
nvme bdev driver, which picks up the actual block size of the
underlying device and everything just works.  This subtlety
(nvme bdev driver working regardless of disk size, but aio bdev
driver requiring 4096 byte blocks) is apparently not documented
anywhere else (e.g. it's not mentioned in the Longhorn docs)
which is why I'm adding it here, just in case anyone else hits
it and gets confused.

Signed-off-by: Tim Serong <tserong@suse.com>
@tserong
Copy link
Contributor Author

tserong commented Nov 1, 2024

(I missed the "Technical Preview" -> "Experimental" change in the settings page yesterday :-/)

Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

Thanks, @tserong. I made a few edits to the note you added but the doc looks fab. Should be good to go soon.

docs/advanced/longhorn-v2.md Outdated Show resolved Hide resolved
Co-authored-by: Jillian <67180770+jillian-maroket@users.noreply.github.com>
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.

3 participants