[Fix](ai) Fix some AI Resours Issues

be/src/exprs/aggregate/aggregate_function_ai_agg.h

@@ -170,7 +170,7 @@ class AggregateFunctionAIAggData {
 #endif
 
         return HttpClient::execute_with_retry(
-                _ai_config.max_retries, _ai_config.retry_delay_second,
+                ai_http_request_attempts(_ai_config.max_retries), _ai_config.retry_delay_second,
                 [this, &request_body, &response](HttpClient* client) -> Status {
                     return this->do_send_request(client, request_body, response);
                 });

be/src/exprs/function/ai/ai_adapter.h

@@ -22,6 +22,7 @@
 
 #include <algorithm>
 #include <cctype>
+#include <limits>
 #include <memory>
 #include <string>
 #include <string_view>
@@ -39,6 +40,14 @@
 
 namespace doris {
 
+// AI max_retries excludes the initial request, while HttpClient expects total attempts.
+inline int ai_http_request_attempts(int32_t max_retries) {
+    if (max_retries <= 0) {
+        return 1;
+    }
+    return max_retries == std::numeric_limits<int32_t>::max() ? max_retries : max_retries + 1;
+}
+
 struct AIResource {
     AIResource() = default;
     AIResource(const TAIResource& tai)

be/src/exprs/function/ai/ai_functions.h

@@ -209,7 +209,8 @@ class AIFunction : public IFunction {
     Status send_request_to_llm(const std::string& request_body, std::string& response,
                                const TAIResource& config, std::shared_ptr<AIAdapter>& adapter,
                                FunctionContext* context) const {
-        return HttpClient::execute_with_retry(config.max_retries, config.retry_delay_second,
+        return HttpClient::execute_with_retry(ai_http_request_attempts(config.max_retries),
+                                              config.retry_delay_second,
                                               [this, &request_body, &response, &config, &adapter,
                                                context](HttpClient* client) -> Status {
                                                   return this->do_send_request(client, request_body,

be/test/ai/ai_adapter_test.cpp

@@ -41,6 +41,22 @@ class MockHttpClient : public HttpClient {
     std::string _content_type;
 };
 
+TEST(AI_ADAPTER_TEST, max_retries_is_in_addition_to_initial_request) {
+    auto count_attempts = [](int32_t max_retries) {
+        int attempts = 0;
+        Status status = HttpClient::execute_with_retry(
+                ai_http_request_attempts(max_retries), 0, [&attempts](HttpClient*) {
+                    ++attempts;
+                    return Status::InternalError("force retry");
+                });
+        EXPECT_FALSE(status.ok());
+        return attempts;
+    };
+
+    EXPECT_EQ(count_attempts(0), 1);
+    EXPECT_EQ(count_attempts(2), 3);
+}
+
 TEST(AI_ADAPTER_TEST, local_adapter_request_chat_endpoint) {
     LocalAdapter adapter;
     TAIResource config;

fe/fe-core/src/main/java/org/apache/doris/catalog/AIResource.java

@@ -27,9 +27,8 @@
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.gson.annotations.SerializedName;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
 
+import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -54,9 +53,10 @@
  */
 
 public class AIResource extends Resource {
-    private static final Logger LOG = LogManager.getLogger(AIResource.class);
+    private static final String LEGACY_VALIDITY_CHECK = "ai.validity_check";
+
     @SerializedName(value = "properties")
-    private Map<String, String> properties;
+    private volatile Map<String, String> properties;
     @SerializedName(value = "createdByRoot")
     private boolean createdByRoot;
 
@@ -66,7 +66,7 @@ public AIResource() {
 
     public AIResource(String name) {
         super(name, ResourceType.AI);
-        properties = Maps.newHashMap();
+        properties = ImmutableMap.of();
     }
 
     public boolean isCreatedByRoot() {
@@ -80,84 +80,85 @@ void setCreatedByRoot(boolean createdByRoot) {
     @Override
     protected void setProperties(ImmutableMap<String, String> newProperties) throws DdlException {
         Preconditions.checkState(newProperties != null);
-        this.properties = Maps.newHashMap(newProperties);
-
-        AIProperties.requiredAIProperties(properties);
-
-        boolean needCheck = isNeedCheck(properties);
-        if (LOG.isDebugEnabled()) {
-            LOG.debug("AI resource need check validity: {}", needCheck);
-        }
+        Map<String, String> changedProperties = Maps.newHashMap(newProperties);
+        changedProperties.remove(LEGACY_VALIDITY_CHECK);
 
-        AIProperties.optionalAIProperties(this.properties);
+        AIProperties.requiredAIProperties(changedProperties);
+        AIProperties.optionalAIProperties(changedProperties);
+        this.properties = ImmutableMap.copyOf(changedProperties);
     }
 
     public String getProperty(String propertyKey) {
-        return properties.get(propertyKey);
-    }
-
-    private boolean isNeedCheck(Map<String, String> newProperties) {
-        boolean needCheck = !this.properties.containsKey(AIProperties.VALIDITY_CHECK)
-                || Boolean.parseBoolean(this.properties.get(AIProperties.VALIDITY_CHECK));
-
-        if (newProperties != null && newProperties.containsKey(AIProperties.VALIDITY_CHECK)) {
-            needCheck = Boolean.parseBoolean(newProperties.get(AIProperties.VALIDITY_CHECK));
-        }
-
-        if ("LOCAL".equalsIgnoreCase(this.properties.getOrDefault(AIProperties.PROVIDER_TYPE, ""))) {
-            needCheck = false;
+        readLock();
+        try {
+            return properties.get(propertyKey);
+        } finally {
+            readUnlock();
         }
-        return needCheck;
     }
 
     @Override
-    public void modifyProperties(Map<String, String> properties) throws DdlException {
-        boolean needCheck = isNeedCheck(properties);
-        if (LOG.isDebugEnabled()) {
-            LOG.debug("AI resource need check validity: {}", needCheck);
-        }
-
-        if (needCheck) {
+    public void modifyProperties(Map<String, String> newProperties) throws DdlException {
+        writeLock();
+        try {
             Map<String, String> changedProperties = new HashMap<>(this.properties);
-            changedProperties.putAll(properties);
+            changedProperties.remove(LEGACY_VALIDITY_CHECK);
+            for (Map.Entry<String, String> kv : newProperties.entrySet()) {
+                if (LEGACY_VALIDITY_CHECK.equals(kv.getKey())) {
+                    continue;
+                }
+                replaceIfEffectiveValue(changedProperties, kv.getKey(), kv.getValue());
+                if (AIProperties.API_KEY.equals(kv.getKey())) {
+                    changedProperties.put(kv.getKey(), kv.getValue());
+                }
+            }
             AIProperties.requiredAIProperties(changedProperties);
+            AIProperties.optionalAIProperties(changedProperties);
+
+            this.properties = ImmutableMap.copyOf(changedProperties);
+            ++version;
+        } finally {
+            writeUnlock();
         }
+        super.modifyProperties(newProperties);
+    }
 
-        // modify properties
-        writeLock();
-        for (Map.Entry<String, String> kv : properties.entrySet()) {
-            replaceIfEffectiveValue(this.properties, kv.getKey(), kv.getValue());
-            if (kv.getKey().equals(AIProperties.API_KEY)) {
-                this.properties.put(kv.getKey(), kv.getValue());
-            }
+    @Override
+    public void gsonPostProcess() throws IOException {
+        super.gsonPostProcess();
+        if (properties != null) {
+            Map<String, String> loadedProperties = Maps.newHashMap(properties);
+            loadedProperties.remove(LEGACY_VALIDITY_CHECK);
+            properties = ImmutableMap.copyOf(loadedProperties);
         }
-        ++version;
-        writeUnlock();
-        super.modifyProperties(properties);
     }
 
     @Override
     public Map<String, String> getCopiedProperties() {
-        return Maps.newHashMap(properties);
+        return getPropertiesSnapshot();
     }
 
     @Override
     protected void getProcNodeData(BaseProcResult result) {
         String lowerCaseType = type.name().toLowerCase();
         result.addRow(Lists.newArrayList(name, lowerCaseType, "id", String.valueOf(id)));
         readLock();
-        result.addRow(Lists.newArrayList(name, lowerCaseType, "version", String.valueOf(version)));
-        for (Map.Entry<String, String> entry : properties.entrySet()) {
-            if (entry.getKey().equals(AIProperties.API_KEY)) {
-                result.addRow(Lists.newArrayList(name, lowerCaseType, entry.getKey(), "******"));
-            } else {
-                result.addRow(Lists.newArrayList(name, lowerCaseType, entry.getKey(), entry.getValue()));
+        try {
+            result.addRow(Lists.newArrayList(name, lowerCaseType, "version", String.valueOf(version)));
+            for (Map.Entry<String, String> entry : properties.entrySet()) {
+                if (entry.getKey().equals(AIProperties.API_KEY)) {
+                    result.addRow(Lists.newArrayList(name, lowerCaseType, entry.getKey(), "******"));
+                } else {
+                    result.addRow(Lists.newArrayList(name, lowerCaseType, entry.getKey(), entry.getValue()));
+                }
             }
+        } finally {
+            readUnlock();
         }
-        readUnlock();
     }
 
     public TAIResource toThrift() throws NumberFormatException {
+        Map<String, String> properties = getPropertiesSnapshot();
         TAIResource tAIResource = new TAIResource();
         tAIResource.setProviderType(properties.get(AIProperties.PROVIDER_TYPE));
         tAIResource.setEndpoint(properties.get(AIProperties.ENDPOINT));
@@ -198,4 +199,13 @@ public TAIResource toThrift() throws NumberFormatException {
 
         return tAIResource;
     }
+
+    private Map<String, String> getPropertiesSnapshot() {
+        readLock();
+        try {
+            return Maps.newHashMap(properties);
+        } finally {
+            readUnlock();
+        }
+    }
 }

fe/fe-core/src/main/java/org/apache/doris/catalog/Resource.java

@@ -367,10 +367,10 @@ private void notifyUpdate(Map<String, String> properties) {
 
     public void applyDefaultProperties() {}
 
-    public static void registerUsedAIResourceName(String resourceName) {
+    public static void registerUsedAIResource(AIResource resource) {
         ConnectContext ctx = ConnectContext.get();
         if (ctx != null && ctx.getStatementContext() != null) {
-            ctx.getStatementContext().registerUsedAIResourceName(resourceName);
+            ctx.getStatementContext().registerUsedAIResource(resource.getName(), resource.toThrift());
         }
     }
 }

fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/AIProperties.java

@@ -23,6 +23,7 @@
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 
 public class AIProperties extends BaseProperties {
@@ -50,8 +51,6 @@ public class AIProperties extends BaseProperties {
     public static final String DEFAULT_ANTHROPIC_VERSION = "2023-06-01";
     public static final String DEFAULT_DIMENSIONS = "-1";
 
-    public static final String VALIDITY_CHECK = "ai.validity_check";
-
     public static final List<String> REQUIRED_FIELDS = Arrays.asList(ENDPOINT, PROVIDER_TYPE, MODEL_NAME);
     public static final List<String> PROVIDERS
             = Arrays.asList("OPENAI", "LOCAL", "GEMINI", "DEEPSEEK", "ANTHROPIC",
@@ -66,8 +65,9 @@ public static void requiredAIProperties(Map<String, String> properties) throws D
         }
 
         // Check the provider is valid
-        properties.put(PROVIDER_TYPE, properties.get(PROVIDER_TYPE).toUpperCase());
-        if (PROVIDERS.stream().noneMatch(s -> s.equals(properties.get(PROVIDER_TYPE).toUpperCase()))) {
+        String provider = properties.get(PROVIDER_TYPE).toUpperCase(Locale.ROOT);
+        properties.put(PROVIDER_TYPE, provider);
+        if (!PROVIDERS.contains(provider)) {
             throw new DdlException("Provider must be one of " + PROVIDERS);
         }
 
@@ -78,22 +78,81 @@ public static void requiredAIProperties(Map<String, String> properties) throws D
                     + properties.get(AIProperties.PROVIDER_TYPE));
         }
 
-        // Check weather the 'temperature' is valid
+        // Check whether the numeric properties are valid
         String temp = properties.get(AIProperties.TEMPERATURE);
-        if (!Strings.isNullOrEmpty(temp) && !temp.equals("-1")) {
-            double tempVal = Double.parseDouble(temp);
-            if (!(tempVal >= 0 && tempVal <= 1)) {
-                throw new DdlException("Temperature must be a double between 0 and 1");
+        if (properties.containsKey(AIProperties.TEMPERATURE)) {
+            double tempVal;
+            try {
+                tempVal = Double.parseDouble(temp);
+            } catch (NullPointerException | NumberFormatException e) {
+                throw new DdlException("Temperature must be -1 or a double between 0 and 1", e);
+            }
+            if (!Double.isFinite(tempVal) || (tempVal != -1 && !(tempVal >= 0 && tempVal <= 1))) {
+                throw new DdlException("Temperature must be -1 or a double between 0 and 1");
             }
         }
 
-        // Check 'dimensions'
-        temp = properties.get(AIProperties.DIMENSIONS);
-        if (!Strings.isNullOrEmpty(temp) && temp.equals("-1")) {
-            int tempVal = Integer.parseInt(temp);
-            if (tempVal <= 0) {
-                throw new DdlException("Dimensions must be a positive integer");
-            }
+        if (properties.containsKey(AIProperties.MAX_TOKEN)) {
+            checkPositiveLongOrDefault(properties.get(AIProperties.MAX_TOKEN), "Max token");
+        }
+        if (properties.containsKey(AIProperties.MAX_RETRIES)) {
+            checkMaxRetries(properties.get(AIProperties.MAX_RETRIES));
+        }
+        if (properties.containsKey(AIProperties.RETRY_DELAY_SECOND)) {
+            checkNonNegativeInteger(properties.get(AIProperties.RETRY_DELAY_SECOND), "Retry delay second");
+        }
+        if (properties.containsKey(AIProperties.DIMENSIONS)) {
+            checkPositiveIntegerOrDefault(properties.get(AIProperties.DIMENSIONS), "Dimensions");
+        }
+    }
+
+    private static void checkPositiveLongOrDefault(String value, String propertyName) throws DdlException {
+        long parsedValue;
+        try {
+            parsedValue = Long.parseLong(value);
+        } catch (NumberFormatException e) {
+            throw new DdlException(propertyName + " must be a positive integer or -1", e);
+        }
+        if (parsedValue != -1 && parsedValue <= 0) {
+            throw new DdlException(propertyName + " must be a positive integer or -1");
+        }
+    }
+
+    private static void checkNonNegativeInteger(String value, String propertyName) throws DdlException {
+        int parsedValue;
+        try {
+            parsedValue = Integer.parseInt(value);
+        } catch (NumberFormatException e) {
+            throw new DdlException(propertyName + " must be a non-negative integer", e);
+        }
+        if (parsedValue < 0) {
+            throw new DdlException(propertyName + " must be a non-negative integer");
+        }
+    }
+
+    private static void checkMaxRetries(String value) throws DdlException {
+        int parsedValue;
+        try {
+            parsedValue = Integer.parseInt(value);
+        } catch (NumberFormatException e) {
+            throw new DdlException("Max retries must be a non-negative integer less than "
+                    + Integer.MAX_VALUE, e);
+        }
+        if (parsedValue < 0 || parsedValue == Integer.MAX_VALUE) {
+            throw new DdlException("Max retries must be a non-negative integer less than "
+                    + Integer.MAX_VALUE);
+        }
+    }
+
+    private static void checkPositiveIntegerOrDefault(String value, String propertyName) throws DdlException {
+        int parsedValue;
+        try {
+            parsedValue = Integer.parseInt(value);
+        } catch (NumberFormatException e) {
+            throw new DdlException(propertyName + " must be a positive integer or -1", e);
+        }
+        if (parsedValue != -1 && parsedValue <= 0) {
+            throw new DdlException(propertyName + " must be a positive integer or -1");
         }
     }