Skip to content

<fix>[storage]: Atomic modification API for MDS nodes. #3233

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.

Sign up for GitHub

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

Closed
zstack-robot-2 wants to merge 1 commit into from
+118 −0
Closed

<fix>[storage]: Atomic modification API for MDS nodes. #3233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions ...main/java/org/zstack/header/storage/addon/primary/APIUpdateExternalPrimaryStorageMsg.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ public class APIUpdateExternalPrimaryStorageMsg extends APIUpdatePrimaryStorageM
@APIParam(required = false, maxLength = 255, validValues = {"Vhost", "Scsi", "Nvme", "CBD", "file"})
private String defaultProtocol;

@APIParam(required = false)
private String oldConfig;

public String getConfig() {
return config;
}
Expand All @@ -35,13 +38,22 @@ public void setDefaultProtocol(String defaultProtocol) {
this.defaultProtocol = defaultProtocol;
}

public String getOldConfig() {
return oldConfig;
}

public void setOldConfig(String oldConfig) {
this.oldConfig = oldConfig;
}

public static APIUpdateExternalPrimaryStorageMsg __example__() {
APIUpdateExternalPrimaryStorageMsg msg = new APIUpdateExternalPrimaryStorageMsg();
msg.setUuid(uuid());
msg.setName("My Primary Storage");
msg.setDescription("New description");
msg.setDefaultProtocol("Vhost");
msg.setConfig("{\"pools\":[{\"name\":\"pool1\",\"aliasName\":\"pool-high\"}]}");
msg.setOldConfig("{\"pools\":[{\"name\":\"pool2\",\"aliasName\":\"pool-high\"}]}");
return msg;
}
}
9 changes: 9 additions & 0 deletions ...rg/zstack/header/storage/addon/primary/APIUpdateExternalPrimaryStorageMsgDoc_zh_cn.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,15 @@ doc {
optional true
since "5.0.0"
}
column {
name "oldConfig"
enclosedIn "updateExternalPrimaryStorage"
desc "用于原子更新的预期旧值,为 null 则代表跳过预检.只有当服务器上的当前配置与此值完全相等时,更新才会生效;否则将拒绝请求以防止并发冲突."
location "body"
type "String"
optional true
since "5.5.0"
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions sdk/src/main/java/org/zstack/sdk/UpdateExternalPrimaryStorageAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ public Result throwExceptionIfError() {
@Param(required = false, validValues = {"Vhost","Scsi","Nvme","CBD","file"}, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false)
public java.lang.String defaultProtocol;

@Param(required = false, nonempty = false, nullElements = false, emptyString = true, noTrim = false)
public java.lang.String oldConfig;

@Param(required = true, nonempty = false, nullElements = false, emptyString = true, noTrim = false)
public java.lang.String uuid;

Expand Down
23 changes: 23 additions & 0 deletions storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.zstack.storage.addon.primary;

import com.google.common.collect.Interner;
import com.google.common.collect.Interners;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang.StringUtils;
import org.springframework.beans.factory.annotation.Autowire;
Expand Down Expand Up @@ -71,6 +73,7 @@ public class ExternalPrimaryStorage extends PrimaryStorageBase {

protected final PrimaryStorageNodeSvc node;
protected final PrimaryStorageControllerSvc controller;
private static final Interner<String> externalPsUpdateLock = Interners.newWeakInterner();

private ExternalPrimaryStorageVO externalVO;
private LinkedHashMap selfConfig;
Expand Down Expand Up @@ -172,6 +175,18 @@ protected void handleApiMessage(APIMessage msg) {
}
}

private boolean compareAndSetConfig(String expectedConfig, String config) {
synchronized (externalPsUpdateLock.intern(String.format("ExternalPrimaryStorage-update-%s", self.getUuid()))) {
String updateSql = "update ExternalPrimaryStorageVO set config = :config where uuid = :uuid and config = :expectedConfig";
int updatedRows = SQL.New(updateSql)
.param("config", config)
.param("uuid", self.getUuid())
.param("expectedConfig", expectedConfig)
.execute();
return updatedRows > 0;
}
}

private void handle(APIUpdateExternalPrimaryStorageMsg msg) {
APIUpdateExternalPrimaryStorageEvent evt = new APIUpdateExternalPrimaryStorageEvent(msg.getId());
if (msg.getName() != null) {
Expand All @@ -193,6 +208,14 @@ private void handle(APIUpdateExternalPrimaryStorageMsg msg) {
externalVO.setConfig(config);
needReconnect = true;
}
if (msg.getOldConfig() != null) {
boolean success = compareAndSetConfig(msg.getOldConfig(), externalVO.getConfig());
if (!success) {
evt.setError(operr(ORG_ZSTACK_STORAGE_ADDON_PRIMARY_10040,"Failed to update ExternalPrimaryStorage[uuid:%s], config has been modified by another operation", externalVO.getUuid()));
bus.publish(evt);
return;
}
}
externalVO = dbf.updateAndRefresh(externalVO);

if (needReconnect) {
Expand Down
69 changes: 69 additions & 0 deletions ...groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsPrimaryStorageCase.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import org.zstack.testlib.SubCase
import org.zstack.utils.data.SizeUnit
import org.zstack.utils.gson.JSONObjectUtil

import java.util.concurrent.CyclicBarrier
import java.util.concurrent.atomic.AtomicInteger

/**
Expand Down Expand Up @@ -294,6 +295,74 @@ class ZbsPrimaryStorageCase extends SubCase {
assert Q.New(ExternalPrimaryStorageSpaceVO.class)
.eq(ExternalPrimaryStorageSpaceVO_.primaryStorageUuid, ps.uuid)
.count() == 1

String nowConfig = Q.New(ExternalPrimaryStorageVO.class)
.select(ExternalPrimaryStorageVO_.config)
.eq(ExternalPrimaryStorageVO_.uuid, ps.uuid)
.findValue()
updateExternalPrimaryStorage {
uuid = ps.uuid
config ="{\"mdsUrls\":[\"root:password@127.0.1.4\",\"root:password@127.0.1.2\",\"root:password@127.0.1.3\"],\"logicalPoolName\":\"lpool1\"}"
oldConfig = nowConfig
}
String newConfig= Q.New(ExternalPrimaryStorageVO.class)
.select(ExternalPrimaryStorageVO_.config)
.eq(ExternalPrimaryStorageVO_.uuid, ps.uuid)
.findValue()
assert newConfig.contains("127.0.1.4")
expect(AssertionError.class) {
updateExternalPrimaryStorage {
uuid = ps.uuid
config ="{\"mdsUrls\":[\"root:password@127.0.1.5\",\"root:password@127.0.1.2\",\"root:password@127.0.1.3\"],\"logicalPoolName\":\"lpool1\"}"
oldConfig = nowConfig
}
}
String newConfig2= Q.New(ExternalPrimaryStorageVO.class)
.select(ExternalPrimaryStorageVO_.config)
.eq(ExternalPrimaryStorageVO_.uuid, ps.uuid)
.findValue()
assert !newConfig2.contains("127.0.1.5")
assert newConfig2.contains("127.0.1.4")
def exceptionCount = new AtomicInteger(0)
def successCount = new AtomicInteger(0)
def barrier = new CyclicBarrier(2)
def thread1 = Thread.start{
try {
barrier.await()
updateExternalPrimaryStorage {
uuid = ps.uuid
config ="{\"mdsUrls\":[\"root:password@127.0.1.6\",\"root:password@127.0.1.2\",\"root:password@127.0.1.3\"],\"logicalPoolName\":\"lpool1\"}"
oldConfig = newConfig2
}
successCount.incrementAndGet()
} catch (Throwable e) {
exceptionCount.incrementAndGet()
}
}
def thread2 = Thread.start{
try {
barrier.await()
updateExternalPrimaryStorage {
uuid = ps.uuid
config ="{\"mdsUrls\":[\"root:password@127.0.1.4\",\"root:password@127.0.1.7\",\"root:password@127.0.1.3\"],\"logicalPoolName\":\"lpool1\"}"
oldConfig = newConfig2
}
successCount.incrementAndGet()
} catch (Throwable e) {
exceptionCount.incrementAndGet()
}
}
thread1.join()
thread2.join()
retryInSecs {
String newConfig3= Q.New(ExternalPrimaryStorageVO.class)
.select(ExternalPrimaryStorageVO_.config)
.eq(ExternalPrimaryStorageVO_.uuid, ps.uuid)
.findValue()
assert ([ "127.0.1.6", "127.0.1.7" ].count { newConfig3.contains(it) } == 1)
assert exceptionCount.get() == 1
assert successCount.get() == 1
}
Copy link
Collaborator Author

@zstack-robot-2 zstack-robot-2 Jan 22, 2026

Choose a reason for hiding this comment

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

Comment from yu.qiu:

resolved

// update multi pools
// Config.Pool
updateExternalPrimaryStorage {
Expand Down
2 changes: 2 additions & 0 deletions utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6494,6 +6494,8 @@ public class CloudOperationsErrorCode {

public static final String ORG_ZSTACK_STORAGE_ADDON_PRIMARY_10014 = "ORG_ZSTACK_STORAGE_ADDON_PRIMARY_10014";

public static final String ORG_ZSTACK_STORAGE_ADDON_PRIMARY_10040 = "ORG_ZSTACK_STORAGE_ADDON_PRIMARY_10040";

public static final String ORG_ZSTACK_NETWORK_HOSTNETWORKINTERFACE_LLDP_10000 = "ORG_ZSTACK_NETWORK_HOSTNETWORKINTERFACE_LLDP_10000";

public static final String ORG_ZSTACK_NETWORK_HOSTNETWORKINTERFACE_LLDP_10001 = "ORG_ZSTACK_NETWORK_HOSTNETWORKINTERFACE_LLDP_10001";
Expand Down