This is an automated email from the ASF dual-hosted git repository.
xuba pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/amoro.git
The following commit(s) were added to refs/heads/master by this push:
new 60d27cca0 [AMORO-3660] Refactor InternalContainers to Containers and
update related references (#3661)
60d27cca0 is described below
commit 60d27cca031b1fe36dc0f65503d550fe0e6fae00
Author: Xu Bai <[email protected]>
AuthorDate: Thu Aug 21 00:41:22 2025 +0800
[AMORO-3660] Refactor InternalContainers to Containers and update related
references (#3661)
* Refactor InternalContainers to Containers and update related references
* Refactor resource release logic to ensure proper handling of
non-optimizer resource containers
---------
Co-authored-by: nathan.ma <[email protected]>
---
.../apache/amoro/server/AmoroServiceContainer.java | 4 ++--
.../dashboard/controller/OptimizerController.java | 22 ++++++++++++++----
.../controller/OptimizerGroupController.java | 26 +++++++++++++++-------
.../dashboard/controller/SettingController.java | 4 ++--
.../{InternalContainers.java => Containers.java} | 24 +++++++-------------
.../org/apache/amoro/server/AmsEnvironment.java | 14 ++++++++++--
6 files changed, 60 insertions(+), 34 deletions(-)
diff --git
a/amoro-ams/src/main/java/org/apache/amoro/server/AmoroServiceContainer.java
b/amoro-ams/src/main/java/org/apache/amoro/server/AmoroServiceContainer.java
index 4d418d7f5..ddbf8a1c7 100644
--- a/amoro-ams/src/main/java/org/apache/amoro/server/AmoroServiceContainer.java
+++ b/amoro-ams/src/main/java/org/apache/amoro/server/AmoroServiceContainer.java
@@ -42,8 +42,8 @@ import org.apache.amoro.server.persistence.DataSourceFactory;
import org.apache.amoro.server.persistence.HttpSessionHandlerFactory;
import org.apache.amoro.server.persistence.SqlSessionFactoryProvider;
import org.apache.amoro.server.resource.ContainerMetadata;
+import org.apache.amoro.server.resource.Containers;
import org.apache.amoro.server.resource.DefaultOptimizerManager;
-import org.apache.amoro.server.resource.InternalContainers;
import org.apache.amoro.server.resource.OptimizerManager;
import org.apache.amoro.server.scheduler.inline.InlineTableExecutors;
import org.apache.amoro.server.table.DefaultTableManager;
@@ -533,7 +533,7 @@ public class AmoroServiceContainer {
containerList.add(container);
}
}
- InternalContainers.init(containerList);
+ Containers.init(containerList);
}
}
diff --git
a/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/OptimizerController.java
b/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/OptimizerController.java
index a65acfb9f..18e0fde2f 100644
---
a/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/OptimizerController.java
+++
b/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/OptimizerController.java
@@ -20,11 +20,13 @@ package org.apache.amoro.server.dashboard.controller;
import io.javalin.http.Context;
import org.apache.amoro.resource.Resource;
+import org.apache.amoro.resource.ResourceContainer;
import org.apache.amoro.resource.ResourceGroup;
import org.apache.amoro.resource.ResourceType;
import org.apache.amoro.server.dashboard.response.OkResponse;
+import org.apache.amoro.server.manager.AbstractOptimizerContainer;
import org.apache.amoro.server.resource.ContainerMetadata;
-import org.apache.amoro.server.resource.InternalContainers;
+import org.apache.amoro.server.resource.Containers;
import org.apache.amoro.server.resource.OptimizerInstance;
import org.apache.amoro.server.resource.OptimizerManager;
import org.apache.amoro.shade.guava32.com.google.common.base.Preconditions;
@@ -62,7 +64,13 @@ public class OptimizerController {
"The resource ID %s has not been indexed" + " to any optimizer.",
resourceId));
Resource resource = optimizerManager.getResource(resourceId);
resource.getProperties().putAll(optimizerInstances.get(0).getProperties());
-
InternalContainers.get(resource.getContainerName()).releaseResource(resource);
+ ResourceContainer rc = Containers.get(resource.getContainerName());
+ Preconditions.checkState(
+ rc instanceof AbstractOptimizerContainer,
+ "Cannot release optimizer on non-optimizer resource container %s.",
+ resource.getContainerName());
+ ((AbstractOptimizerContainer) rc).releaseResource(resource);
+
optimizerManager.deleteResource(resourceId);
optimizerManager.deleteOptimizer(resource.getGroupName(), resourceId);
ctx.json(OkResponse.of("Success to release optimizer"));
@@ -80,7 +88,13 @@ public class OptimizerController {
.setProperties(resourceGroup.getProperties())
.setThreadCount(parallelism)
.build();
-
InternalContainers.get(resource.getContainerName()).requestResource(resource);
+ ResourceContainer rc = Containers.get(resource.getContainerName());
+ Preconditions.checkState(
+ rc instanceof AbstractOptimizerContainer,
+ "Cannot create optimizer on non-optimizer resource container %s.",
+ resource.getContainerName());
+ ((AbstractOptimizerContainer) rc).requestResource(resource);
+
optimizerManager.createResource(resource);
ctx.json(OkResponse.of("success to create optimizer"));
}
@@ -89,7 +103,7 @@ public class OptimizerController {
public void getContainers(Context ctx) {
ctx.json(
OkResponse.of(
- InternalContainers.getMetadataList().stream()
+ Containers.getMetadataList().stream()
.map(ContainerMetadata::getName)
.collect(Collectors.toList())));
}
diff --git
a/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/OptimizerGroupController.java
b/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/OptimizerGroupController.java
index 147e9d691..8aba3989f 100644
---
a/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/OptimizerGroupController.java
+++
b/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/OptimizerGroupController.java
@@ -20,6 +20,7 @@ package org.apache.amoro.server.dashboard.controller;
import io.javalin.http.Context;
import org.apache.amoro.resource.Resource;
+import org.apache.amoro.resource.ResourceContainer;
import org.apache.amoro.resource.ResourceGroup;
import org.apache.amoro.resource.ResourceType;
import org.apache.amoro.server.dashboard.model.OptimizerInstanceInfo;
@@ -28,9 +29,10 @@ import
org.apache.amoro.server.dashboard.model.TableOptimizingInfo;
import org.apache.amoro.server.dashboard.response.OkResponse;
import org.apache.amoro.server.dashboard.response.PageResult;
import org.apache.amoro.server.dashboard.utils.PropertiesUtil;
+import org.apache.amoro.server.manager.AbstractOptimizerContainer;
import org.apache.amoro.server.optimizing.OptimizingStatus;
import org.apache.amoro.server.resource.ContainerMetadata;
-import org.apache.amoro.server.resource.InternalContainers;
+import org.apache.amoro.server.resource.Containers;
import org.apache.amoro.server.resource.OptimizerInstance;
import org.apache.amoro.server.resource.OptimizerManager;
import org.apache.amoro.server.table.TableManager;
@@ -154,10 +156,6 @@ public class OptimizerGroupController {
public void getOptimizerGroups(Context ctx) {
List<Map<String, String>> result =
optimizerManager.listResourceGroups().stream()
- .filter(
- resourceGroup ->
- !InternalContainers.UNMANAGED_CONTAINER_NAME.equals(
- resourceGroup.getContainer()))
.map(
e -> {
Map<String, String> mapObj = new HashMap<>();
@@ -206,7 +204,13 @@ public class OptimizerGroupController {
"The resource ID %s has not been indexed" + " to any optimizer.",
resourceId));
Resource resource = optimizerManager.getResource(resourceId);
resource.getProperties().putAll(optimizerInstances.get(0).getProperties());
-
InternalContainers.get(resource.getContainerName()).releaseResource(resource);
+ ResourceContainer rc = Containers.get(resource.getContainerName());
+ Preconditions.checkState(
+ rc instanceof AbstractOptimizerContainer,
+ "Cannot release optimizer on non-optimizer resource container %s.",
+ resource.getContainerName());
+ ((AbstractOptimizerContainer) rc).releaseResource(resource);
+
optimizerManager.deleteResource(resourceId);
optimizerManager.deleteOptimizer(resource.getGroupName(), resourceId);
ctx.json(OkResponse.of("Success to release optimizer"));
@@ -225,7 +229,13 @@ public class OptimizerGroupController {
.setProperties(resourceGroup.getProperties())
.setThreadCount(parallelism)
.build();
-
InternalContainers.get(resource.getContainerName()).requestResource(resource);
+ ResourceContainer rc = Containers.get(resource.getContainerName());
+ Preconditions.checkState(
+ rc instanceof AbstractOptimizerContainer,
+ "Cannot create optimizer on non-optimizer resource container %s.",
+ resource.getContainerName());
+ ((AbstractOptimizerContainer) rc).requestResource(resource);
+
optimizerManager.createResource(resource);
ctx.json(OkResponse.of("success to scaleOut optimizer"));
}
@@ -300,7 +310,7 @@ public class OptimizerGroupController {
public void getContainers(Context ctx) {
ctx.json(
OkResponse.of(
- InternalContainers.getMetadataList().stream()
+ Containers.getMetadataList().stream()
.map(ContainerMetadata::getName)
.collect(Collectors.toList())));
}
diff --git
a/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/SettingController.java
b/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/SettingController.java
index faf86b082..43b16ad3a 100644
---
a/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/SettingController.java
+++
b/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/SettingController.java
@@ -24,7 +24,7 @@ import org.apache.amoro.resource.ResourceGroup;
import org.apache.amoro.server.AmoroManagementConf;
import org.apache.amoro.server.dashboard.response.OkResponse;
import org.apache.amoro.server.resource.ContainerMetadata;
-import org.apache.amoro.server.resource.InternalContainers;
+import org.apache.amoro.server.resource.Containers;
import org.apache.amoro.server.resource.OptimizerManager;
import org.apache.amoro.shade.guava32.com.google.common.collect.Sets;
@@ -72,7 +72,7 @@ public class SettingController {
/** Get container settings. */
public void getContainerSetting(Context ctx) {
- List<ContainerMetadata> containerMetas =
InternalContainers.getMetadataList();
+ List<ContainerMetadata> containerMetas = Containers.getMetadataList();
List<Map<String, Object>> result = new ArrayList<>();
Objects.requireNonNull(containerMetas)
.forEach(
diff --git
a/amoro-ams/src/main/java/org/apache/amoro/server/resource/InternalContainers.java
b/amoro-ams/src/main/java/org/apache/amoro/server/resource/Containers.java
similarity index 78%
rename from
amoro-ams/src/main/java/org/apache/amoro/server/resource/InternalContainers.java
rename to
amoro-ams/src/main/java/org/apache/amoro/server/resource/Containers.java
index c5eecb33d..a97641160 100644
---
a/amoro-ams/src/main/java/org/apache/amoro/server/resource/InternalContainers.java
+++ b/amoro-ams/src/main/java/org/apache/amoro/server/resource/Containers.java
@@ -18,7 +18,7 @@
package org.apache.amoro.server.resource;
-import org.apache.amoro.resource.InternalResourceContainer;
+import org.apache.amoro.resource.ResourceContainer;
import org.apache.amoro.shade.guava32.com.google.common.base.Preconditions;
import org.apache.amoro.shade.guava32.com.google.common.collect.Maps;
@@ -27,17 +27,10 @@ import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
-public class InternalContainers {
- public static final String UNMANAGED_CONTAINER_NAME = "unmanaged";
+public class Containers {
private static final Map<String, ContainerWrapper> globalContainers =
Maps.newHashMap();
private static volatile boolean isInitialized = false;
- static {
- ContainerMetadata metadata = new
ContainerMetadata(UNMANAGED_CONTAINER_NAME, "");
- ContainerWrapper externalContainer = new ContainerWrapper(metadata, null);
- globalContainers.put(UNMANAGED_CONTAINER_NAME, externalContainer);
- }
-
public static void init(List<ContainerMetadata> containerList) {
Preconditions.checkState(!isInitialized, "OptimizerContainers has been
initialized");
Preconditions.checkNotNull(containerList, "containerList is null");
@@ -46,7 +39,7 @@ public class InternalContainers {
isInitialized = true;
}
- public static InternalResourceContainer get(String name) {
+ public static ResourceContainer get(String name) {
checkInitialized();
return Optional.ofNullable(globalContainers.get(name))
.map(ContainerWrapper::getContainer)
@@ -70,7 +63,7 @@ public class InternalContainers {
}
private static class ContainerWrapper {
- private final InternalResourceContainer container;
+ private final ResourceContainer container;
private final ContainerMetadata metadata;
public ContainerWrapper(ContainerMetadata metadata) {
@@ -78,12 +71,12 @@ public class InternalContainers {
this.container = loadResourceContainer(metadata.getImplClass());
}
- ContainerWrapper(ContainerMetadata metadata, InternalResourceContainer
container) {
+ ContainerWrapper(ContainerMetadata metadata, ResourceContainer container) {
this.metadata = metadata;
this.container = container;
}
- public InternalResourceContainer getContainer() {
+ public ResourceContainer getContainer() {
return container;
}
@@ -91,11 +84,10 @@ public class InternalContainers {
return metadata;
}
- private InternalResourceContainer loadResourceContainer(String implClass) {
+ private ResourceContainer loadResourceContainer(String implClass) {
try {
Class<?> clazz = Class.forName(implClass);
- InternalResourceContainer resourceContainer =
- (InternalResourceContainer) clazz.newInstance();
+ ResourceContainer resourceContainer = (ResourceContainer)
clazz.newInstance();
resourceContainer.init(metadata.getName(), metadata.getProperties());
return resourceContainer;
} catch (ClassNotFoundException | InstantiationException |
IllegalAccessException e) {
diff --git
a/amoro-ams/src/test/java/org/apache/amoro/server/AmsEnvironment.java
b/amoro-ams/src/test/java/org/apache/amoro/server/AmsEnvironment.java
index 827135da4..bf0464453 100644
--- a/amoro-ams/src/test/java/org/apache/amoro/server/AmsEnvironment.java
+++ b/amoro-ams/src/test/java/org/apache/amoro/server/AmsEnvironment.java
@@ -28,10 +28,12 @@ import org.apache.amoro.mixed.CatalogLoader;
import org.apache.amoro.mixed.MixedFormatCatalog;
import org.apache.amoro.optimizer.standalone.StandaloneOptimizer;
import org.apache.amoro.properties.CatalogMetaProperties;
+import org.apache.amoro.resource.ResourceContainer;
import org.apache.amoro.resource.ResourceGroup;
import org.apache.amoro.server.catalog.DefaultCatalogManager;
import org.apache.amoro.server.catalog.ServerCatalog;
-import org.apache.amoro.server.resource.InternalContainers;
+import org.apache.amoro.server.manager.AbstractOptimizerContainer;
+import org.apache.amoro.server.resource.Containers;
import org.apache.amoro.server.resource.OptimizerManager;
import org.apache.amoro.shade.guava32.com.google.common.collect.Maps;
import org.apache.amoro.shade.guava32.com.google.common.io.MoreFiles;
@@ -285,7 +287,15 @@ public class AmsEnvironment {
.listOptimizers()
.forEach(
resource -> {
-
InternalContainers.get(resource.getContainerName()).releaseResource(resource);
+ ResourceContainer rc =
Containers.get(resource.getContainerName());
+ if (!(rc instanceof AbstractOptimizerContainer)) {
+ LOG.warn(
+ "Cannot stop optimizer on non-optimizer resource container
{}.",
+ resource.getContainerName());
+ return;
+ }
+
+ ((AbstractOptimizerContainer) rc).releaseResource(resource);
});
}
