feat(engine, replica): v2 volume expand #8022 by davidcheng0922 · Pull Request #386 · longhorn/longhorn-spdk-engine

davidcheng0922 · 2025-07-25T06:09:11Z

Which issue(s) this PR fixes:

longhorn/longhorn#8022

What this PR does / why we need it:

support v2 volume expansion, only for nvme frontend

Special notes for your reviewer:

Additional documentation or context

v2-expansion.webm
v2-expansion-with-live-IO.webm

codecov · 2025-07-25T07:54:23Z

Codecov Report

❌ Patch coverage is 0% with 586 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.67%. Comparing base (4d9b75a) to head (f0e90a9).
⚠️ Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/spdk/engine.go	0.00%	438 Missing ⚠️
pkg/spdk/replica.go	0.00%	88 Missing ⚠️
pkg/spdk/server.go	0.00%	31 Missing ⚠️
pkg/client/client.go	0.00%	29 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##            main    #386      +/-   ##
========================================
- Coverage   0.77%   0.67%   -0.10%     
========================================
  Files         24      24              
  Lines       9866   11273    +1407     
========================================
  Hits          76      76              
- Misses      9783   11190    +1407     
  Partials       7       7

Flag	Coverage Δ
unittests	`0.67% <0.00%> (-0.10%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

PhanLe1010 · 2025-07-31T22:58:44Z

+		// partial success
+		// for the fail one, we set it to ERR
+		aggregatedErr := map[string]string{}
+		for replicaName, err := range failedReplica {
+			e.ReplicaStatusMap[replicaName].Mode = types.ModeERR
+			aggregatedErr[replicaName] = err.Error()
+		}
+
+		e.log.WithFields(logrus.Fields{
+			"engineName":     e.Name,
+			"volumeName":     e.VolumeName,
+			"failedReplicas": aggregatedErr,
+		}).Error("Some replicas failed to expand and have been marked as ERR")


Good idea to mark the failed to expand replica as failed.

Quick question, is this possible to revert the replica expansion?

Yes, go-spdk-helper bdev-lvol resize support to shrink size.
But it will be dangerous, I think it will directly remove the blobs without any data protection.

During the expansion, as we suspend the I/O, I think it will be ok, but I don't think we have to revert the size; instead only contain the successful ones.

If it is needed for other rpc call, maybe need a snapshot before doing it and it will become more complicated for revert.

it will be dangerous

Why is the operation dangerous?

The dangerous means if we allow shrink in some cases, I'm afraid the data will be erased and the fs is broken.
If we can make sure the data is not used, it might be ok.

davidcheng0922 · 2025-08-11T01:57:04Z

Blocker:

ublk expansion process
the current workable expansion process for nvme is

suspend --noflush=false --nolockfs=false
bdev-raid delete
controller detach
resize lvol
controller attach
bdev-raid create
reload / resume dm

For ublk,

suspend --noflush=false --nolockfs=false -> success
bdev-raid delete -> the raid is deleted, but timeout, no response from spdk (not sure the reason, maybe ublk kernel driver hang the response)
controller detach -> although we ignore the timeout issue previous, it still hang in this step

Interesting things, if I manually execute command in the same order
the bdev-raid will not timeout, but still hang in controller detach
maybe the resource is not fully released

Tried method :

Add backoff and time to check if it is race condition
Stop ublk disk before/after suspension without removing the dm

To brief, my guess,
When suspend succeeds, the kernel believes the ublk device is idle. The SPDK application is blocked because it can't free up resources that the kernel's ublk driver is still holding.
nvme can work maybe is because of network block device
I'm not sure about my understanding is correct or not, any comment is welcomed

Currently, because ublk is not the priority, I will stop here, and do more research in the future.

mergify · 2025-08-26T21:20:44Z

This pull request is now in conflict. Could you fix it @davidcheng0922? 🙏

PhanLe1010 · 2025-08-28T01:17:23Z

+
+	// Suspend IO if frontend is active
+	if e.Frontend == types.FrontendSPDKTCPBlockdev && e.Endpoint != "" {
+		if err := e.initiator.Suspend(false, false); err != nil {


Just want to double check. This function means dm device suspension, right?

yeah, it actually calls
dmsetup suspend --noflush=false --nolockfs=false

derekbit · 2025-09-09T15:28:16Z


+func (c *SPDKClient) ReplicaExpand(name string, size uint64) error {
+	if name == "" {
+		return fmt.Errorf("failed to delete SPDK replica: missing required parameter")


Suggested change

return fmt.Errorf("failed to delete SPDK replica: missing required parameter")

return fmt.Errorf("failed to expand replica: missing required parameter")

Signed-off-by: David Cheng <davidcheng0922@gmail.com>

- disconnect nvme target & stop expose bdev before deleting raid Signed-off-by: David Cheng <davidcheng0922@gmail.com>

Signed-off-by: David Cheng <davidcheng0922@gmail.com>

- Replace backoff with retry.RetryOnConflict for update retry - Use handleNvmeTcpFrontend instead of reconnectNvmeTcpFrontend - Remove unnecessary check in replica expand - Refactor finishExpansion for clarity and separation of concerns Signed-off-by: David Cheng <davidcheng0922@gmail.com>

…and polish log Signed-off-by: David Cheng <davidcheng0922@gmail.com>

derekbit

LGTM

derekbit · 2025-09-10T11:47:38Z

@davidcheng0922 Thank you. You can update longhorn-instance-manager PR.

davidcheng0922 requested a review from a team July 25, 2025 06:09

davidcheng0922 changed the title ~~Issue 8022 volume expansion~~ feat(engine, replica): v2 volume expand #8022 Jul 25, 2025

This was referenced Jul 31, 2025

feat(proxy): implement V2 VolumeExpand #8022 longhorn/longhorn-instance-manager#1036

Merged

feat: support v2 volume expansion #8022 longhorn/longhorn-manager#3949

Merged

PhanLe1010 reviewed Jul 31, 2025

View reviewed changes

Comment thread pkg/spdk/engine.go Outdated

davidcheng0922 force-pushed the issue-8022-volume-expansion branch 2 times, most recently from 02a8b79 to ac856a7 Compare August 1, 2025 09:44

shuo-wu reviewed Aug 2, 2025

View reviewed changes

davidcheng0922 force-pushed the issue-8022-volume-expansion branch 2 times, most recently from d5ab71e to 89173b4 Compare August 11, 2025 01:28

davidcheng0922 force-pushed the issue-8022-volume-expansion branch from 89173b4 to 159083a Compare August 20, 2025 03:10

davidcheng0922 requested review from PhanLe1010, derekbit and shuo-wu August 21, 2025 07:53

davidcheng0922 force-pushed the issue-8022-volume-expansion branch from 159083a to e3152e4 Compare August 21, 2025 08:21

shuo-wu reviewed Aug 26, 2025

View reviewed changes

davidcheng0922 force-pushed the issue-8022-volume-expansion branch 3 times, most recently from f0b8910 to dcf99a0 Compare August 27, 2025 09:48

davidcheng0922 requested a review from shuo-wu August 27, 2025 09:52

PhanLe1010 reviewed Aug 28, 2025

View reviewed changes

davidcheng0922 force-pushed the issue-8022-volume-expansion branch 3 times, most recently from 0dbf28f to 9d8a90d Compare August 28, 2025 07:52

derekbit reviewed Aug 28, 2025

View reviewed changes

Comment thread pkg/client/client.go

derekbit reviewed Aug 28, 2025

View reviewed changes

Comment thread pkg/client/client.go Outdated

Comment thread pkg/spdk/replica.go Outdated

Comment thread pkg/spdk/replica.go Outdated

davidcheng0922 requested a review from c3y1huang September 4, 2025 02:10

derekbit force-pushed the issue-8022-volume-expansion branch from 8970b27 to 5736962 Compare September 6, 2025 03:21

c3y1huang reviewed Sep 9, 2025

View reviewed changes

Comment thread pkg/spdk/engine.go Outdated

Comment thread pkg/spdk/engine.go

shuo-wu reviewed Sep 9, 2025

View reviewed changes

Comment thread pkg/spdk/replica.go Outdated

Comment thread pkg/spdk/engine.go Outdated

Comment thread pkg/spdk/engine.go Outdated

Comment thread pkg/spdk/engine.go

davidcheng0922 force-pushed the issue-8022-volume-expansion branch 2 times, most recently from 98f0ae2 to 7ca6e43 Compare September 9, 2025 08:19

davidcheng0922 requested review from c3y1huang and shuo-wu September 9, 2025 09:47

derekbit reviewed Sep 9, 2025

View reviewed changes

shuo-wu reviewed Sep 9, 2025

View reviewed changes

Comment thread pkg/spdk/engine.go Outdated

Comment thread pkg/spdk/engine.go

derekbit force-pushed the issue-8022-volume-expansion branch from 7ca6e43 to 6b71171 Compare September 10, 2025 00:20

davidcheng0922 force-pushed the issue-8022-volume-expansion branch from 6b71171 to 4206442 Compare September 10, 2025 07:22

davidcheng0922 added 8 commits September 10, 2025 15:24

feat(engine, client): add EngineExpand and ReplicaExpand methods

c59a569

Signed-off-by: David Cheng <davidcheng0922@gmail.com>

fix(engine,replica): replicas size check & change expand process order

9f3887d

- disconnect nvme target & stop expose bdev before deleting raid Signed-off-by: David Cheng <davidcheng0922@gmail.com>

fix(engine): use e.disconnectTarget instead and fix small bug

91eebab

Signed-off-by: David Cheng <davidcheng0922@gmail.com>

fix(engine,replica): code refactor

dc094b4

Signed-off-by: David Cheng <davidcheng0922@gmail.com>

fix(engine,replica): improve error handling and message

b0f5877

Signed-off-by: David Cheng <davidcheng0922@gmail.com>

chore(mod,vendor): add k8s client-go for retry.RetryOnConflict

da07bd0

Signed-off-by: David Cheng <davidcheng0922@gmail.com>

fix(engine,replica): refactor handleFrontend, improve error handling …

f0e90a9

…and polish log Signed-off-by: David Cheng <davidcheng0922@gmail.com>

davidcheng0922 force-pushed the issue-8022-volume-expansion branch from 4206442 to f0e90a9 Compare September 10, 2025 07:24

davidcheng0922 requested review from derekbit and shuo-wu September 10, 2025 07:56

derekbit approved these changes Sep 10, 2025

View reviewed changes

derekbit merged commit d7f9a69 into longhorn:main Sep 10, 2025
8 of 11 checks passed

This was referenced Sep 11, 2025

feat(proxy): implement V2 VolumeExpand #8022 (backport #1036) longhorn/longhorn-instance-manager#1077

Merged

feat: support v2 volume expansion #8022 (backport #3949) longhorn/longhorn-manager#4076

Merged

davidcheng0922 mentioned this pull request Sep 11, 2025

[FEATURE] v2 supports volume expansion longhorn/longhorn#8022

Closed

	return fmt.Errorf("failed to delete SPDK replica: missing required parameter")
	return fmt.Errorf("failed to expand replica: missing required parameter")

Conversation

davidcheng0922 commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue(s) this PR fixes:

What this PR does / why we need it:

Special notes for your reviewer:

Additional documentation or context

Uh oh!

codecov Bot commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

PhanLe1010 Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

davidcheng0922 Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

derekbit Aug 31, 2025

Choose a reason for hiding this comment

Uh oh!

davidcheng0922 Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davidcheng0922 commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mergify Bot commented Aug 26, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PhanLe1010 Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

davidcheng0922 Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

derekbit Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davidcheng0922 commented Jul 25, 2025 •

edited

Loading

codecov Bot commented Jul 25, 2025 •

edited

Loading

davidcheng0922 commented Aug 11, 2025 •

edited

Loading