Skip to content

Commit

Permalink
share routing rules among all requests
Browse files Browse the repository at this point in the history
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 lyft#166
  • Loading branch information
deigote committed Sep 26, 2023
1 parent 7269bcf commit dc3a9fe
Showing 1 changed file with 9 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> result = new HashMap<String, String>();
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();
}
}

/**
Expand Down

0 comments on commit dc3a9fe

Please sign in to comment.