From dc3a9fef8b587de45cfea16e94c0c67c8cd41e4e Mon Sep 17 00:00:00 2001 From: Diego Toharia Date: Tue, 26 Sep 2023 12:16:37 +0200 Subject: [PATCH] share routing rules among all requests the previous implementation required to read the routing rules config file for each request, adding unnecessary latency. More importantly, it was done in a non-thread-safe way, as the `rulesEngine` and `ruleFactory` were shared among all requests, but the `Rules` creation was not. \ Since the class MVELRuleFactory is stateful (as it has `RuleDefinitionReader reader` and `ParserContext parserContext` as private members), it should not be shared between concurrent threads. `Rules` OTOH is a read-only class after it's created (it's just iterated over by the `DefaultRulesEngine`). Given that the rules are the same for all requests, and that the `Rules` usage is thread-safe, it makes sense to share it across all of them.\ We have deployed this code in production at Datadog and our APM profiling tool shows a reduction of around ~1-2%, plus we stopped seeing the error described in https://github.com/lyft/presto-gateway/issues/166 --- .../ha/router/RoutingGroupSelector.java | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/gateway-ha/src/main/java/com/lyft/data/gateway/ha/router/RoutingGroupSelector.java b/gateway-ha/src/main/java/com/lyft/data/gateway/ha/router/RoutingGroupSelector.java index e1aa06e5..e02c70ef 100644 --- a/gateway-ha/src/main/java/com/lyft/data/gateway/ha/router/RoutingGroupSelector.java +++ b/gateway-ha/src/main/java/com/lyft/data/gateway/ha/router/RoutingGroupSelector.java @@ -36,24 +36,21 @@ static RoutingGroupSelector byRoutingGroupHeader() { static RoutingGroupSelector byRoutingRulesEngine(String rulesConfigPath) { RulesEngine rulesEngine = new DefaultRulesEngine(); MVELRuleFactory ruleFactory = new MVELRuleFactory(new YamlRuleDefinitionReader()); - - return request -> { - try { - Rules rules = ruleFactory.createRules( - new FileReader(rulesConfigPath)); + try { + final Rules rules = ruleFactory.createRules(new FileReader(rulesConfigPath)); + return request -> { Facts facts = new Facts(); HashMap result = new HashMap(); facts.put("request", request); facts.put("result", result); rulesEngine.fire(rules, facts); return result.get("routingGroup"); - } catch (Exception e) { - Logger.log.error("Error opening rules configuration file," - + " using routing group header as default.", e); - return Optional.ofNullable(request.getHeader(ROUTING_GROUP_HEADER)) - .orElse(request.getHeader(ALTERNATE_ROUTING_GROUP_HEADER)); - } - }; + }; + } catch (Exception e) { + Logger.log.error("Error opening rules configuration file," + + " using routing group header as default.", e); + return byRoutingGroupHeader(); + } } /**