gavinchou commented on code in PR #65670:
URL: https://github.com/apache/doris/pull/65670#discussion_r3587750299
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/ResourceMgr.java:
##########
@@ -145,16 +147,22 @@ public void replayDropResource(DropResourceOperationLog
operationLog) {
}
public void alterResource(String resourceName, Map<String, String>
properties) throws DdlException {
- if (!nameToResource.containsKey(resourceName)) {
+ Resource resource = nameToResource.get(resourceName);
+ if (resource == null) {
throw new DdlException("Resource(" + resourceName + ") dose not
exist.");
}
- Resource resource = nameToResource.get(resourceName);
- resource.modifyProperties(properties);
+ Resource logResource;
+ EditLog.EditLogItem logItem;
+ synchronized (resource.getAlterLock()) {
Review Comment:
Addressed in 22305cf59f6. Generic Resource.toString()/write() and
ResourceMgr.write() no longer acquire the ALTER/state locks, and the checkpoint
snapshot expansion was removed. The new blocked-S3-validation test verifies
that a state reader can still acquire the resource read lock while external
validation is waiting.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/ResourceMgr.java:
##########
@@ -145,16 +147,22 @@ public void replayDropResource(DropResourceOperationLog
operationLog) {
}
public void alterResource(String resourceName, Map<String, String>
properties) throws DdlException {
- if (!nameToResource.containsKey(resourceName)) {
+ Resource resource = nameToResource.get(resourceName);
+ if (resource == null) {
throw new DdlException("Resource(" + resourceName + ") dose not
exist.");
}
- Resource resource = nameToResource.get(resourceName);
- resource.modifyProperties(properties);
+ Resource logResource;
+ EditLog.EditLogItem logItem;
+ synchronized (resource.getAlterLock()) {
+ resource.modifyProperties(properties);
+ // Snapshot and enqueue under the same lock to preserve concurrent
ALTER order.
+ logResource = getAlterLogResource(resource);
Review Comment:
The live-object risk is worth considering for CREATE as a separate
follow-up, but the reported failure and required fix are specifically on ALTER
after the resource is already published. This PR now limits the behavioral
change to that reproduced path; changing CREATE journal semantics without a
matching reproduction/test would broaden the scope beyond this fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
