davsclaus commented on code in PR #13040:
URL: https://github.com/apache/camel/pull/13040#discussion_r1481643218
##########
catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/models.properties:
##########
@@ -204,6 +205,7 @@ to
toD
tokenize
topic
+totalRequestsConfig
Review Comment:
I assume these are left over from the old code
##########
catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/models.properties:
##########
@@ -18,6 +18,7 @@ circuitBreaker
claimCheck
combinedServiceDiscovery
combinedServiceFilter
+concurrentConfig
Review Comment:
I assume these are left over from the old code
##########
core/camel-core-model/src/main/java/org/apache/camel/model/ThrottleDefinition.java:
##########
@@ -247,6 +375,19 @@ public ThrottleDefinition executorService(String
executorService) {
return this;
}
+ /**
+ * Sets the throttling mode
Review Comment:
We may want a little bit more doc here. Maybe talk a bit what each mode does.
##########
core/camel-management-api/src/main/java/org/apache/camel/api/management/mbean/ManagedConcurrentRequestsThrottlerMBean.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.api.management.mbean;
+
+import org.apache.camel.api.management.ManagedAttribute;
+
+public interface ManagedConcurrentRequestsThrottlerMBean extends
ManagedProcessorMBean {
Review Comment:
Can we avoid having different mbeans - as there is only 1 EIP and ideally we
should match that with the mbean too.
Keep the old mbean name, and then add attribute for mode, so you can see
there which mode it uses.
##########
core/camel-core-reifier/src/main/java/org/apache/camel/reifier/ThrottleReifier.java:
##########
@@ -33,30 +35,45 @@ public ThrottleReifier(Route route, ProcessorDefinition<?>
definition) {
@Override
public Processor createProcessor() throws Exception {
+
boolean async = parseBoolean(definition.getAsyncDelayed(), false);
boolean shutdownThreadPool = willCreateNewThreadPool(definition, true);
ScheduledExecutorService threadPool =
getConfiguredScheduledExecutorService("Throttle", definition, true);
+ Expression correlation = null;
+ if (definition.getCorrelationExpression() != null) {
+ correlation =
createExpression(definition.getCorrelationExpression());
+ }
+
+ boolean reject = parseBoolean(definition.getRejectExecution(), false);
// max requests per period is mandatory
Expression maxRequestsExpression =
createMaxRequestsPerPeriodExpression();
if (maxRequestsExpression == null) {
throw new IllegalArgumentException("MaxRequestsPerPeriod
expression must be provided on " + this);
}
- Expression correlation = null;
- if (definition.getCorrelationExpression() != null) {
- correlation =
createExpression(definition.getCorrelationExpression());
- }
+ if (ThrottlingMode.toMode(definition.getMode()) ==
ThrottlingMode.ConcurrentRequests) {
Review Comment:
Add parseString(definition...) as that does property placeholders
##########
catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/models/totalRequestsConfig.json:
##########
@@ -0,0 +1,16 @@
+{
Review Comment:
I assume these are left over from the old code
##########
catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/models/concurrentConfig.json:
##########
@@ -0,0 +1,16 @@
+{
Review Comment:
I assume these are left over from the old code
##########
core/camel-core-model/src/main/java/org/apache/camel/model/ThrottleDefinition.java:
##########
@@ -89,35 +120,132 @@ public String getShortName() {
@Override
public String getLabel() {
- return "throttle[" + getExpression() + "]";
+ return "throttle[" + description() + "]";
}
// Fluent API
//
-------------------------------------------------------------------------
/**
- * Sets the maximum number of concurrent requests
+ * Sets the time period during which the maximum request count is valid for
+ *
+ * @param timePeriodMillis period in millis
+ * @return the builder
+ */
+ public ThrottleDefinition timePeriodMillis(long timePeriodMillis) {
+ return timePeriodMillis(Long.toString(timePeriodMillis));
+ }
+
+ /**
+ * Sets the time period during which the maximum request count is valid for
*
- * @param maximumConcurrentRequests the maximum number of concurrent
requests
+ * @param timePeriodMillis period in millis
+ * @return the builder
+ */
+ public ThrottleDefinition timePeriodMillis(String timePeriodMillis) {
+ setTimePeriodMillis(timePeriodMillis);
+ return this;
+ }
+
+ /**
+ * Sets the maximum number of requests
+ *
+ * @param maximumConcurrentRequests the maximum number of requests
(according to the mode in use - either
+ * concurrent or by time period)
* @return the builder
*/
- public ThrottleDefinition maximumConcurrentRequests(long
maximumConcurrentRequests) {
+ public ThrottleDefinition maximumRequests(long maximumConcurrentRequests) {
setExpression(
ExpressionNodeHelper.toExpressionDefinition(ExpressionBuilder.constantExpression(maximumConcurrentRequests)));
return this;
}
/**
- * Sets the number of concurrent requests
+ * Sets the maximum number of requests
*
- * @param maximumConcurrentRequests the maximum number of concurrent
requests
+ * @param maximumConcurrentRequests the maximum number of requests
(according to the mode in use - either
+ * concurrent or by time period)
* @return the builder
*/
- public ThrottleDefinition maximumConcurrentRequests(String
maximumConcurrentRequests) {
+ public ThrottleDefinition maximumRequests(String
maximumConcurrentRequests) {
setExpression(
-
ExpressionNodeHelper.toExpressionDefinition(ExpressionBuilder.simpleExpression(maximumConcurrentRequests)));
+
ExpressionNodeHelper.toExpressionDefinition(ExpressionBuilder.constantExpression(maximumConcurrentRequests)));
return this;
}
+ /**
+ * Sets the maximum number of concurrent requests
+ *
+ * @param maximumConcurrentRequests the maximum number of concurrent
requests
+ * @deprecated Use {@link #maximumRequests(long)}
+ * @return the builder
+ */
+ @Deprecated(since = "4.4.0")
Review Comment:
The new throttler code was added in 4.3 (non LTS) so IMHO its fine to
remove/change the code as upgrade from LTS to LTS is what matters.
##########
core/camel-core-model/src/main/java/org/apache/camel/model/ThrottleDefinition.java:
##########
@@ -89,35 +120,132 @@ public String getShortName() {
@Override
public String getLabel() {
- return "throttle[" + getExpression() + "]";
+ return "throttle[" + description() + "]";
}
// Fluent API
//
-------------------------------------------------------------------------
/**
- * Sets the maximum number of concurrent requests
+ * Sets the time period during which the maximum request count is valid for
+ *
+ * @param timePeriodMillis period in millis
+ * @return the builder
+ */
+ public ThrottleDefinition timePeriodMillis(long timePeriodMillis) {
+ return timePeriodMillis(Long.toString(timePeriodMillis));
+ }
+
+ /**
+ * Sets the time period during which the maximum request count is valid for
*
- * @param maximumConcurrentRequests the maximum number of concurrent
requests
+ * @param timePeriodMillis period in millis
+ * @return the builder
+ */
+ public ThrottleDefinition timePeriodMillis(String timePeriodMillis) {
+ setTimePeriodMillis(timePeriodMillis);
+ return this;
+ }
+
+ /**
+ * Sets the maximum number of requests
+ *
+ * @param maximumConcurrentRequests the maximum number of requests
(according to the mode in use - either
+ * concurrent or by time period)
* @return the builder
*/
- public ThrottleDefinition maximumConcurrentRequests(long
maximumConcurrentRequests) {
+ public ThrottleDefinition maximumRequests(long maximumConcurrentRequests) {
setExpression(
ExpressionNodeHelper.toExpressionDefinition(ExpressionBuilder.constantExpression(maximumConcurrentRequests)));
return this;
}
/**
- * Sets the number of concurrent requests
+ * Sets the maximum number of requests
*
- * @param maximumConcurrentRequests the maximum number of concurrent
requests
+ * @param maximumConcurrentRequests the maximum number of requests
(according to the mode in use - either
+ * concurrent or by time period)
* @return the builder
*/
- public ThrottleDefinition maximumConcurrentRequests(String
maximumConcurrentRequests) {
+ public ThrottleDefinition maximumRequests(String
maximumConcurrentRequests) {
setExpression(
-
ExpressionNodeHelper.toExpressionDefinition(ExpressionBuilder.simpleExpression(maximumConcurrentRequests)));
+
ExpressionNodeHelper.toExpressionDefinition(ExpressionBuilder.constantExpression(maximumConcurrentRequests)));
return this;
}
+ /**
+ * Sets the maximum number of concurrent requests
+ *
+ * @param maximumConcurrentRequests the maximum number of concurrent
requests
+ * @deprecated Use {@link #maximumRequests(long)}
+ * @return the builder
+ */
+ @Deprecated(since = "4.4.0")
+ public ThrottleDefinition maximumConcurrentRequests(long
maximumConcurrentRequests) {
+ if (ThrottlingMode.toMode(mode) == ThrottlingMode.ConcurrentRequests) {
+ setExpression(
+ ExpressionNodeHelper
+
.toExpressionDefinition(ExpressionBuilder.constantExpression(maximumConcurrentRequests)));
+ return this;
+ } else {
+ throw new IllegalArgumentException(
+ "Maximum concurrent requests can only be set when using
concurrent requests mode");
+ }
+ }
+
+ /**
+ * Sets the number of concurrent requests
+ *
+ * @param maximumConcurrentRequests the maximum number of concurrent
requests
+ * @deprecated Use {@link #maximumRequests(long)}
+ * @return the builder
+ */
+ @Deprecated(since = "4.4.0")
+ public ThrottleDefinition maximumConcurrentRequests(String
maximumConcurrentRequests) {
Review Comment:
The new throttler code was added in 4.3 (non LTS) so IMHO its fine to
remove/change the code as upgrade from LTS to LTS is what matters.
##########
core/camel-core-model/src/main/java/org/apache/camel/model/ThrottleDefinition.java:
##########
@@ -57,29 +57,60 @@ public class ThrottleDefinition extends ExpressionNode
implements ExecutorServic
@XmlAttribute
@Metadata(label = "advanced", javaType = "java.lang.Boolean")
private String rejectExecution;
+ @XmlAttribute
+ @Metadata(defaultValue = "1000", javaType = "java.time.Duration")
+ private String timePeriodMillis;
+
+ @XmlAttribute
+ @Metadata(label = "advanced", javaType =
"org.apache.camel.model.ThrottlingMode", defaultValue = "TotalRequests",
Review Comment:
I dont think it should be advanced, and I think this should be the first
option, eg move to top so user select mode first.
--
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: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
