Reducing the unused calls to elasticsearch_client #303
Conversation
| @@ -868,7 +876,7 @@ def run_rule(self, rule, endtime, starttime=None): | |||
| :return: The number of matches that the rule produced. | |||
| """ | |||
| run_start = time.time() | |||
| self.thread_data.current_es = self.es_clients.setdefault(rule['name'], elasticsearch_client(rule)) | |||
There was a problem hiding this comment.
setdefault results in the correct behaviour, but because it takes a concrete default value, elasticsearch_client is getting called every run and the result is disregarded. From what I see dictionary does not have a lazy option.
There was a problem hiding this comment.
Yes, I see where this will help improve efficiency. I think a unit test that proves the same elasticsearch_client instance is returned from the new getter function, when called twice with the same rule, would be good unit coverage.
There was a problem hiding this comment.
This is especially bad for us running in AWS because inside Auth we call get_credentials which is an HTTP call to the Ec2 metdata service:
elastalert2/elastalert/auth.py
Line 59 in d455e55
| es_client = self.es_clients.get(key) | ||
| if es_client is None: | ||
| es_client = elasticsearch_client(rule) | ||
| self.es_clients[key] = es_client |
There was a problem hiding this comment.
Could be condensed into a single line,
self.es_clients[key] = es_client = elasticsearch_client(rule)
Not sure what your stylistic preference is.
There was a problem hiding this comment.
I prefer readability over condensed code, so what you have looks fine.
|
Looking good with this fix:
Running 210 rules every 2.5 minutes ^ |
Adding helper method to reduce the number of unnecessary calls to
elasticsearch_clienthelper method.