Performance testing of an osm-carto SQL patch

openstreetmap / chef

Chef configuration management repo for configuring & maintaining the OpenStreetMap servers.

Apache License 2.0

97 stars 66 forks source link

Performance testing of an osm-carto SQL patch #129

Closed kocio-pl closed 7 years ago

kocio-pl commented 7 years ago

We have a PR for osm-carto (https://github.com/gravitystorm/openstreetmap-carto/pull/2874) which might improve rendering performance, but it needs testing. My question is if you'd like to evaluate it on production and what prerequisites would be needed?

tomhughes commented 7 years ago

I don't really understand what you're asking for, and I don't have any way of evaluating anything other than rolling it out and seeing what happens.

Are you saying that you'd like me to stage the next rollout and do one server at a time so you can see how they compare?

I'm not sure it would tell you much as the servers are all rather different so any change from this would likely be tiny compared to changes caused by different load patterns or hardward capability.

kocio-pl commented 7 years ago

Are you saying that you'd like me to stage the next rollout and do one server at a time so you can see how they compare?

Yes, that was my intention.

I'm not sure it would tell you much as the servers are all rather different so any change from this would likely be tiny compared to changes caused by different load patterns or hardward capability.

I thought of before/after comparison - that should tell us something.

pnorman commented 7 years ago

I thought of before/after comparison - that should tell us something.

Only if we've horribly broken something. Before/after changes since all the tiles are dirty and need re-rendering.

kocio-pl commented 7 years ago

I have not thought about making all the tiles dirty. On the other hand visual changes should be subtle and a change on 1 server is still easier than announcing new release and rolling it on all the servers.

I don't expect this patch to break anything. While re-rendering dirty tiles changes overall usage pattern for about a week, you can still observe the worst/best/average performance per (meta)tile. You can also compare it with previous re-rendering pattern on this server.

But if you think this is not worth the effort, I can go with a typical release cycle.

pnorman commented 7 years ago

I don't expect this patch to break anything. While re-rendering dirty tiles changes overall usage pattern for about a week, you can still observe the worst/best/average performance per (meta)tile. You can also compare it with previous re-rendering pattern on this server.

No you can't! At least, not if you want useful results. The load varies too much. Sure, you might catch if the rendering times have doubled, but if this happens unexpectedly then we've put out a horribly broken release.

But if you think this is not worth the effort, I can go with a typical release cycle.

It's our job to release stylesheets which we're reasonably confident work, including performance. We're not perfect about this - see our patch releases for a history - but pretty good. When we get it wrong, anyone who's put our release into production will need to roll back, or if we've managed to catch and fix it before they do, upgrade to a patch release.

kocio-pl commented 7 years ago

I think we have a "chicken and egg" problem here, and that's exactly why I opened this ticket:

You just can't reasonably test such general performance change on your home machine, but it gives you some hint (and I already have it).
Testing it on a less used server (like @rrzefox) is a bit closer to reality, but not conclusive (and I also asked him to test it).
Testing on 1 out of 4 "real" servers is still not the same as rolling the release, and still not conclusive - but this is the closest to reality and still a test.

So: when would you be "reasonably confident" with testing? For me that would be comparing real OSMF server as much as it's possible, keeping in mind that it's still not the same as full production deployment. If that's not enough for you, it would mean a "release as a test" kind of situation, which is what I'd like to avoid.

kocio-pl commented 7 years ago

This patch is most probably harmless - just increasing performance/decreasing memory usage, so even if we're not sure how much we would gain this way, separate testing is not critical (problems are related to possible loosing of details on rendering).

But it's a good moment to review our testing strategy in case there is a risk of loosing performance (see: https://github.com/gravitystorm/openstreetmap-carto/issues/1287). I guess it will require some kind of cooperation with OSMF server admins anyway.