diff --git a/application/src/main/java/org/thingsboard/server/service/script/AbstractJsInvokeService.java b/application/src/main/java/org/thingsboard/server/service/script/AbstractJsInvokeService.java index 31827f542e..daa74d013f 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/AbstractJsInvokeService.java +++ b/application/src/main/java/org/thingsboard/server/service/script/AbstractJsInvokeService.java @@ -31,7 +31,7 @@ import java.util.concurrent.atomic.AtomicInteger; public abstract class AbstractJsInvokeService implements JsInvokeService { protected Map scriptIdToNameMap = new ConcurrentHashMap<>(); - protected Map blackListedFunctions = new ConcurrentHashMap<>(); + protected Map blackListedFunctions = new ConcurrentHashMap<>(); @Override public ListenableFuture eval(JsScriptType scriptType, String scriptBody, String... argNames) { @@ -78,25 +78,53 @@ public abstract class AbstractJsInvokeService implements JsInvokeService { protected abstract int getMaxErrors(); + protected abstract long getMaxBlacklistDuration(); + protected void onScriptExecutionError(UUID scriptId) { - blackListedFunctions.computeIfAbsent(scriptId, key -> new AtomicInteger(0)).incrementAndGet(); + blackListedFunctions.computeIfAbsent(scriptId, key -> new BlackListInfo()).incrementAndGet(); } private String generateJsScript(JsScriptType scriptType, String functionName, String scriptBody, String... argNames) { - switch (scriptType) { - case RULE_NODE_SCRIPT: - return RuleNodeScriptFactory.generateRuleNodeScript(functionName, scriptBody, argNames); - default: - throw new RuntimeException("No script factory implemented for scriptType: " + scriptType); + if (scriptType == JsScriptType.RULE_NODE_SCRIPT) { + return RuleNodeScriptFactory.generateRuleNodeScript(functionName, scriptBody, argNames); } + throw new RuntimeException("No script factory implemented for scriptType: " + scriptType); } private boolean isBlackListed(UUID scriptId) { - if (blackListedFunctions.containsKey(scriptId)) { - AtomicInteger errorCount = blackListedFunctions.get(scriptId); - return errorCount.get() >= getMaxErrors(); + BlackListInfo errorCount = blackListedFunctions.get(scriptId); + if (errorCount != null) { + if (errorCount.getExpirationTime() <= System.currentTimeMillis()) { + blackListedFunctions.remove(scriptId); + return false; + } else { + return errorCount.get() >= getMaxErrors(); + } } else { return false; } } + + private class BlackListInfo { + private final AtomicInteger counter; + private long expirationTime; + + private BlackListInfo() { + this.counter = new AtomicInteger(0); + } + + public int get() { + return counter.get(); + } + + public int incrementAndGet() { + int result = counter.incrementAndGet(); + expirationTime = System.currentTimeMillis() + getMaxBlacklistDuration(); + return result; + } + + public long getExpirationTime() { + return expirationTime; + } + } } diff --git a/application/src/main/java/org/thingsboard/server/service/script/AbstractNashornJsInvokeService.java b/application/src/main/java/org/thingsboard/server/service/script/AbstractNashornJsInvokeService.java index 49a6304ebb..a9acfa4f42 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/AbstractNashornJsInvokeService.java +++ b/application/src/main/java/org/thingsboard/server/service/script/AbstractNashornJsInvokeService.java @@ -55,8 +55,8 @@ public abstract class AbstractNashornJsInvokeService extends AbstractJsInvokeSer private final AtomicInteger jsEvalMsgs = new AtomicInteger(0); private final AtomicInteger jsFailedMsgs = new AtomicInteger(0); private final AtomicInteger jsTimeoutMsgs = new AtomicInteger(0); - private final FutureCallback evalCallback = new JsStatCallback(jsEvalMsgs, jsTimeoutMsgs, jsFailedMsgs); - private final FutureCallback invokeCallback = new JsStatCallback(jsInvokeMsgs, jsTimeoutMsgs, jsFailedMsgs); + private final FutureCallback evalCallback = new JsStatCallback<>(jsEvalMsgs, jsTimeoutMsgs, jsFailedMsgs); + private final FutureCallback invokeCallback = new JsStatCallback<>(jsInvokeMsgs, jsTimeoutMsgs, jsFailedMsgs); @Autowired @Getter diff --git a/application/src/main/java/org/thingsboard/server/service/script/NashornJsInvokeService.java b/application/src/main/java/org/thingsboard/server/service/script/NashornJsInvokeService.java index 909565609c..66a14cc827 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/NashornJsInvokeService.java +++ b/application/src/main/java/org/thingsboard/server/service/script/NashornJsInvokeService.java @@ -20,6 +20,8 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.stereotype.Service; +import java.util.concurrent.TimeUnit; + @Slf4j @ConditionalOnProperty(prefix = "js", value = "evaluator", havingValue = "local", matchIfMissing = true) @Service @@ -37,6 +39,9 @@ public class NashornJsInvokeService extends AbstractNashornJsInvokeService { @Value("${js.local.max_errors}") private int maxErrors; + @Value("${js.local.max_black_list_duration_sec:60}") + private int maxBlackListDurationSec; + @Override protected boolean useJsSandbox() { return useJsSandbox; @@ -56,4 +61,9 @@ public class NashornJsInvokeService extends AbstractNashornJsInvokeService { protected int getMaxErrors() { return maxErrors; } + + @Override + protected long getMaxBlacklistDuration() { + return TimeUnit.SECONDS.toMillis(maxBlackListDurationSec); + } } diff --git a/application/src/main/java/org/thingsboard/server/service/script/RemoteJsInvokeService.java b/application/src/main/java/org/thingsboard/server/service/script/RemoteJsInvokeService.java index 1b5332bcfc..b0554decea 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/RemoteJsInvokeService.java +++ b/application/src/main/java/org/thingsboard/server/service/script/RemoteJsInvokeService.java @@ -36,6 +36,7 @@ import javax.annotation.PreDestroy; import java.util.Map; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; @@ -66,6 +67,9 @@ public class RemoteJsInvokeService extends AbstractJsInvokeService { @Value("${js.remote.max_errors}") private int maxErrors; + @Value("${js.remote.max_black_list_duration_sec:60}") + private int maxBlackListDurationSec; + @Value("${js.remote.stats.enabled:false}") private boolean statsEnabled; @@ -205,6 +209,7 @@ public class RemoteJsInvokeService extends AbstractJsInvokeService { @Override public void onFailure(Throwable t) { + onScriptExecutionError(scriptId); if (t instanceof TimeoutException || (t.getCause() != null && t.getCause() instanceof TimeoutException)) { kafkaTimeoutMsgs.incrementAndGet(); } @@ -216,6 +221,7 @@ public class RemoteJsInvokeService extends AbstractJsInvokeService { if (invokeResult.getSuccess()) { return invokeResult.getResult(); } else { + onScriptExecutionError(scriptId); log.debug("[{}] Failed to compile script due to [{}]: {}", scriptId, invokeResult.getErrorCode().name(), invokeResult.getErrorDetails()); throw new RuntimeException(invokeResult.getErrorDetails()); } @@ -245,4 +251,9 @@ public class RemoteJsInvokeService extends AbstractJsInvokeService { } } + @Override + protected long getMaxBlacklistDuration() { + return TimeUnit.SECONDS.toMillis(maxBlackListDurationSec); + } + } diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index e4a234fdb9..f08d2baf46 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/application/src/main/resources/thingsboard.yml @@ -425,11 +425,13 @@ js: # Specify thread pool size for JavaScript sandbox resource monitor monitor_thread_pool_size: "${LOCAL_JS_SANDBOX_MONITOR_THREAD_POOL_SIZE:4}" # Maximum CPU time in milliseconds allowed for script execution - max_cpu_time: "${LOCAL_JS_SANDBOX_MAX_CPU_TIME:3000}" + max_cpu_time: "${LOCAL_JS_SANDBOX_MAX_CPU_TIME:10000}" # Maximum allowed JavaScript execution errors before JavaScript will be blacklisted max_errors: "${LOCAL_JS_SANDBOX_MAX_ERRORS:3}" # JS Eval max request timeout. 0 - no timeout max_requests_timeout: "${LOCAL_JS_MAX_REQUEST_TIMEOUT:0}" + # Maximum time in seconds for black listed function to stay in the list. + max_black_list_duration_sec: "${LOCAL_JS_SANDBOX_MAX_BLACKLIST_DURATION_SEC:60}" stats: enabled: "${TB_JS_LOCAL_STATS_ENABLED:false}" print_interval_ms: "${TB_JS_LOCAL_STATS_PRINT_INTERVAL_MS:10000}" @@ -449,6 +451,8 @@ js: response_auto_commit_interval: "${REMOTE_JS_RESPONSE_AUTO_COMMIT_INTERVAL_MS:100}" # Maximum allowed JavaScript execution errors before JavaScript will be blacklisted max_errors: "${REMOTE_JS_SANDBOX_MAX_ERRORS:3}" + # Maximum time in seconds for black listed function to stay in the list. + max_black_list_duration_sec: "${REMOTE_JS_SANDBOX_MAX_BLACKLIST_DURATION_SEC:60}" stats: enabled: "${TB_JS_REMOTE_STATS_ENABLED:false}" print_interval_ms: "${TB_JS_REMOTE_STATS_PRINT_INTERVAL_MS:10000}" @@ -573,8 +577,7 @@ queue: enabled: "${TB_QUEUE_RULE_ENGINE_STATS_ENABLED:true}" print-interval-ms: "${TB_QUEUE_RULE_ENGINE_STATS_PRINT_INTERVAL_MS:10000}" queues: # TODO 2.5: specify correct ENV variable names. - - - name: "Main" + - name: "Main" topic: "${TB_QUEUE_RULE_ENGINE_TOPIC:tb.rule-engine.main}" poll-interval: "${TB_QUEUE_RULE_ENGINE_POLL_INTERVAL_MS:25}" partitions: "${TB_QUEUE_RULE_ENGINE_PARTITIONS:10}" @@ -585,8 +588,7 @@ queue: retries: "${TB_QUEUE_RULE_ENGINE_STRATEGY_RETRIES:3}" # Number of retries, 0 is unlimited failure-percentage: "${TB_QUEUE_RULE_ENGINE_STRATEGY_FAILURE_PERCENTAGE:0}" # Skip retry if failures or timeouts are less then X percentage of messages; pause-between-retries: "${TB_QUEUE_RULE_ENGINE_STRATEGY_RETRY_PAUSE:3}"# Time in seconds to wait in consumer thread before retries; - - - name: "${TB_QUEUE_RULE_ENGINE_HP_QUEUE_NAME:HighPriority}" + - name: "${TB_QUEUE_RULE_ENGINE_HP_QUEUE_NAME:HighPriority}" topic: "${TB_QUEUE_RULE_ENGINE_TOPIC:tb.rule-engine.hp}" poll-interval: "${TB_QUEUE_RULE_ENGINE_POLL_INTERVAL_MS:25}" partitions: "${TB_QUEUE_RULE_ENGINE_PARTITIONS:3}" diff --git a/application/src/test/java/org/thingsboard/server/service/script/TestNashornJsInvokeService.java b/application/src/test/java/org/thingsboard/server/service/script/TestNashornJsInvokeService.java index 1065fbf6d3..8a89de5dff 100644 --- a/application/src/test/java/org/thingsboard/server/service/script/TestNashornJsInvokeService.java +++ b/application/src/test/java/org/thingsboard/server/service/script/TestNashornJsInvokeService.java @@ -49,4 +49,9 @@ public class TestNashornJsInvokeService extends AbstractNashornJsInvokeService { protected int getMaxErrors() { return maxErrors; } + + @Override + protected long getMaxBlacklistDuration() { + return 100000; + } }