Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linear response time on services query in elasticsearch #1526

Closed
codefromthecrypt opened this issue Feb 24, 2017 · 27 comments
Closed

Linear response time on services query in elasticsearch #1526

codefromthecrypt opened this issue Feb 24, 2017 · 27 comments

Comments

@codefromthecrypt
Copy link
Member

Getting some reports of eventual timeouts on service and span name queries on elasticsearch. Ex at 80GB/day you eventually will time out (reported by @pauldraper)

Ultimately one big issue is the shape of the json, which requires nested queries to access the service name of a span. The new reporting format (#1499) will allow us to eventually fix this part, but we can't pin to a span format-based implementation to fix a performance problem!

In cassandra3, we started with a materialized view, but had to switch to manual indexing for similar performance reasons (#1374). The downside is that it amplifies writes and also is less straightforward for those who might be simply writing json to ES (as opposed to via a collector).

Regardless of what we do, it would be nice to have a perf test which can showcase evidence of linear latency patterns on service or span name queries. This would catch any system having indexes which aren't ideal.

We have some next steps to consider...

  • firstly, properly diagnose the issue. As ES is primarily a cache, maybe there's some tuning of the index template to help with nested queries on service and span names
  • next, see what needs to be done in the collector directly vs somewhere else. For example, if we have to do manual indexing we could consider an ES pipeline stage to compose that.

cc @openzipkin/elasticsearch

@codefromthecrypt
Copy link
Member Author

another thing I thought of is that we should collect notes about what sort of data they are putting into zipkin. For example, historically spans had very few tags and people used sampling. Particularly OpenTracing tend to put a lot of tags (each of which ends up in the subquery for service names). It will be interesting to know how many spans and how big they tend to be when users experience problems.

@semyonslepov
Copy link

There are some statistics about our "bad" experience:

  • ~6 Gb of traces in each of daily indices (keeping 5 last indices)
  • ~1 Kb per span
  • 3-4 tags per span

( - running on m3.medium.elasticsearch in AWS, if it matters)

The main workload is writing and some users accidentally come to a web-interface for looking their traces.

@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Apr 11, 2017 via email

@codefromthecrypt
Copy link
Member Author

I have been thinking about this and believe adding a new index for service and span name is the best option. This would be just like cassandra where we keep a separate document for service/span lookup:

CREATE TABLE IF NOT EXISTS zipkin3.span_name_by_service (
    service_name text,
    span_name    text,
    PRIMARY KEY (service_name, span_name)
)

Backfilling this index would be relatively simple and could be done a myriad of ways. Unlike adding new data to the span index, this approach doesn't risk people accidentally clobbering their spans to support it. It also doesn't depend on any specific span representation (current or future).

In implementation, easiest would be a daily index like we use for dependency links. We could set the document ID to service and span name so that the data doesn't grow unbounded. If daily is too course granularity, we could make the ID naming convention include the hour of the day or minute of the hour and still significantly bound the data size.

We could make the read side look at both, and only execute the slow nested query when the ideal index contains no data. This means users don't have to do anything to start using it.

Operationally, I like this approach as being similar to cassandra means less cognitive overhead for cross-storage maintainers.

@openzipkin/elasticsearch @semyonslepov wdyt?

@mansu
Copy link

mansu commented Apr 11, 2017

This sounds great! A couple of questions:

  • When is this new index populated? Is it when a span is written to storage?
  • Having the ability to set the granularity of the index name would be awesome. So, it would be nice to set the granularity to weekly or monthly also. That way, we may not need to clean up this index if we so desire.

@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Apr 11, 2017 via email

@mansu
Copy link

mansu commented Apr 11, 2017

Thanks for the explanation. +1 in this case.

I was asking a longer term index since it would mean cleaning up one more index when we purge the data. But, if this is added an additional index type on the daily zipkin index, daily granularity is an awesome solution.

@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Apr 11, 2017 via email

@semyonslepov
Copy link

Is it safe to do
math to extrapolate span count from this?

Yes, it's fair enough. Actual counts may differ, but it's correct on average.

About indexes: sounds good and transparent for users. Am I right that we will use the same index for spans' names lookup and it's going to be faster too?

@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Apr 11, 2017 via email

@semyonslepov
Copy link

So, we still don't have huge enough data, but of course, I will run some measurements on current and new versions and post the results.

@tramchamploo
Copy link

tramchamploo commented Apr 11, 2017

+1 for a new index, but is there any payload that is from existence check when persisting service/span names?
Also would change shape of spans be a long-term consideration that will be helpful for overall performance?

@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Apr 11, 2017 via email

@tramchamploo
Copy link

Great, this improvement on service/span name query will greatly improve user experience.

@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Apr 12, 2017 via email

@codefromthecrypt
Copy link
Member Author

will take a bit to refactor properly, but the more important code change at query time will look something like this...

service name query

-    SearchRequest.Filters filters = new SearchRequest.Filters();
-    filters.addRange("timestamp_millis", beginMillis, endMillis);
-    SearchRequest request = SearchRequest.forIndicesAndType(indices, SPAN)
-        .filters(filters)
-        .addAggregation(Aggregation.nestedTerms("annotations.endpoint.serviceName"))
-        .addAggregation(Aggregation.nestedTerms("binaryAnnotations.endpoint.serviceName"));
+    SearchRequest request = SearchRequest.forIndicesAndType(indices, SERVICE_SPAN)
+        .addAggregation(Aggregation.terms("serviceName", Integer.MAX_VALUE));

span name query

-    SearchRequest.Filters filters = new SearchRequest.Filters();
-    filters.addRange("timestamp_millis", beginMillis, endMillis);
-    filters.addNestedTerms(asList(
-        "annotations.endpoint.serviceName",
-        "binaryAnnotations.endpoint.serviceName"
-    ), serviceName.toLowerCase(Locale.ROOT));
-    SearchRequest request = SearchRequest.forIndicesAndType(indices, SPAN)
-        .filters(filters)
-        .addAggregation(Aggregation.terms("name", Integer.MAX_VALUE));
+    SearchRequest request = SearchRequest.forIndicesAndType(indices, SERVICE_SPAN)
+        .term("serviceName", serviceName.toLowerCase(Locale.ROOT))
+        .addAggregation(Aggregation.terms("spanName", Integer.MAX_VALUE));

@codefromthecrypt
Copy link
Member Author

#1560 is ready, just looking to make a backport script or at least pseudocode for adding "servicespan" into old indexes

@codefromthecrypt
Copy link
Member Author

If I can get a shipit or change requests on #1562 and #1560, I will release the newly optimized stuff tomorrow.

@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Apr 14, 2017

@semyonslepov @tramchamploo @mansu @devinsba @jcarres-mdsol can any of you try zipkin 1.23? I'd like to get someone to use it in real life before propagating to other projects.

@codefromthecrypt
Copy link
Member Author

if any of you do get a chance, along with response time on /api/v1/services and /api/v1/spans please also report back any differences in storage usage, too. A lot of mapping options are now turned off, so it should be less though I don't know what percentage real life usage will end up as.

@devinsba
Copy link
Member

devinsba commented Apr 14, 2017

I'll need to update the zipkin-aws image to be based off this in order for me to try it, gonna do a local build some time today

@codefromthecrypt
Copy link
Member Author

@devinsba try 0.2.2 :)

@semyonslepov
Copy link

semyonslepov commented Apr 18, 2017

There are some result from my tests:

/api/v1/services (1 RPS):

Test 1, ~1000000 documents in index, ~250 service names with 1 span name for each:
zipkin 1.21.0:
98% - 200 ms
50% - 150 ms

zipkin 1.23.0:
98% - 50 ms
50% - 20 ms

Test 2, ~1500000 in index, ~450 service names with 1 span name for each:

zipkin 1.21.0:
98% - 250 ms
50% - 200 ms

zipkin 1.23.0: (same as Test 1)
98% - 50 ms
50% - 20 ms

When data amount increases, this difference becomes more.

/api/v1/spans (10 RPS):

(one test, something about ~5000000 documents in index, ~250 service names, 1-2 span names for each service)

zipkin 1.21.0:
98% - 150 ms
50% - 50 ms

zipkin 1.23.0:
98% - 50 ms
50% - 20 ms

I saw no significant difference in storage usage for these types of requests with my test conditions.
But there is significant change of CPU usage on Zipkin-hosts: I made two tests with writing 300 spans per second on free-tier AWS t2.micro instance and got ~25% CPU load with 1.21.0, with 1.23.0 at some point (after 3-4 minutes from test beginning) it was 100% with request dropping (ok, it's not correct enough - I set 10s timeout for requests, so my test-tool didn't get any answers after 10s).

@codefromthecrypt
Copy link
Member Author

@semyonslepov thanks for the feedback! fair to say overall better? :)

yeah I wouldn't expect zipkin CPU to go down based on this change, though I would expect elasticsearch's CPU to go down. zipkin is actually doing slightly more, but if its CPU load when consuming becomes an issue we can probably profile a bit.

@semyonslepov
Copy link

Made some more test today to prove earlier theories.
Yes, we can say that service names query is much better (span names query too, but not so significant). And it's awesome!
(ES CPU goes to top on more than 3-4 /api/v1/services queries per second on our setup. Taking into account the fact of existing browser cache for service names, we don't have so many requests from our users to this API handler. But anyway we have to keep it in mind)

Regarding ES CPU, I don't see that it's going down with 1.23.0 release. Even more, in my tests it's going up on the same workload (sending ~1500 spans per second) (I'm not sure about the cleanness of my tests, maybe there is a noise, but I have done them twice for each release. Will be fine to hear something from another users). On the same ES configuration I saw something about 50-55% load of ES CPU with 1.21.0 release and 70% on 1.23.0 (70% vs 95% on another test).

Actually, Zipkin CPU doesn't increase so fast on more powerful configurations (m3.large instance with 2 CPU cores and 7.5GB RAM) and as it was in my yesterday tests (t2.micro instance with 1 CPU core and 1GB RAM, today CPUs are slighlty more powerful). So, this difference is not so critical.

@devinsba
Copy link
Member

devinsba commented Apr 19, 2017

For contrast, at ~800 RPS peak for our ES cluster I am seeing no discernible change in the CPU usage (maybe 1% increase, likely just increased load), our max CPU is only like 15% though because we have more instances than we need. And none on the app either.

We are ES 2.3 with 5 m3.large data nodes

@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Apr 20, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants