benchto icon indicating copy to clipboard operation
benchto copied to clipboard

Make benchmarks hardware agnostic

Open sopel39 opened this issue 7 years ago • 8 comments

Currently benchmarks contains attributes which are specific to the hardware and cluster:

  • data size (10GB, 100GB, 1TB)
  • number of runs, number of prewarms
  • file format

This makes reusing benchmarks on different clusters problematic since those elements need to be fine tuned to a given execution environment.

Major requirements of benchmarks are:

  • results should have low variability
  • execution time of benchmark should be reasonable (e.g: between 30s and 1min) in order to reduce static query overhead (e.g: client-server latency)

Having those requirements we propose to refactor benchto-driver and benchmarks in the following way:

  • add runs_scale_factor driver parameter. Increasing this parameter would reduce variability of benchmark runtimes (this addresses your issue @idemura )
  • use catalog (e.g: tpch) and data_size parameters in benchmarks instead of schema. data_size would be categorical variable (e.g: tiny, small, medium, large, huge). The purpose of the variable is to describe how "difficult" benchmark is to execute without specifying exact data size. This allows to use different mappings in various benchmark environments (e.g: tpch_medium=tpch_10gb_text). The most appropriate data size should be used for environment so that benchmark runtime is reasonable. Also, one definition of benchmark can be used to test different file formats.

FYI: @findepi @idemura

sopel39 avatar Mar 21 '17 09:03 sopel39

Thank you for quick response. Let's move all discussion here, in one place.

I can't agree that inheritance itself is bad as and idea. It works good in environments like Google borg, Google config language to name few and this is very convenient. What is problem there actually is that every parameter can be overriden. Before I explain approach to fix this, let's see what we try to solve here:

  • Add ability to override parameters,
  • Keep this under control (restrict) what can be overriden

Approach you suggest looks not generic enough. Every time something rises a new tiny-little-thingy that needs to be tuned we have to introduce more and more code in driver to support and so code of benchmark is coupled with Java code. I'd like to have this possibility in configs themselves.

I propose to fix inheritance. I totally agree that uncontrolled overriding can be a problem (and I observed such abuse at Google!). Let's only apply inheritance to designated section, say params or overrides. Other sections remain untouched by overrides (you can only replace them, as it was in a new file, or only add - we can figure out what behavior we want).

Example:

overrides:
  cluster: prn1
  catalog: movies

library:
  name: ${movies}
  url: master.${cluster}.facebook.com
  capacity: 100GB

If I need to tune this, I do in another file:

base: ../template/base.yaml

overrides:
  cluster: prn2

I see this to be:

  • Generic enough to not require custom Java code plumbing for new params
  • Overrides allow us to have generic templates decoupled with environment defines
  • Overrides are under control and you always know what you give to users to play with.

Thoughts?

idemura avatar Mar 21 '17 18:03 idemura

I think we are converging. I think we can mix approaches and decide on some convention (including inheritance).

Let's have example benchmark definition (presto/tpch.yaml):

datasource: presto
query-names: presto/tpch/${query}.sql
runs: 6
prewarm-runs: 2
before-execution: sleep-4s, presto/session_set_reorder_joins.sql, presto/session_set_join_distribution_type.sql
frequency: 7
database: hive

# default schema and data size mappings
tpch_tiny: 1gb
tpch_medium: 10gb

variables:
  1:
    query: q2, q8, q9
    schema: ${tpch_medium}
    reorder_joins: true
    join_distribution_type: automatic

Then we could have a "profile" yaml, for instance:

runs: 6
prewarm-runs: 2
tpch_tiny: 10gb

# NEW: it should be possible to override properties of individual benchmarks
presto.tpch.runs: 8
presto.tpch.tpch_tiny: 100gb

We could also have conventional (not enforced) overrides as you mentioned, e.g: benchmark:

...
overrides:
  tpch_tiny: 1gb

variables:
  1:
    query: q2, q8, q9
    schema: ${overrides.tpch_medium}

profile yaml:

overrides.tpch_tiny: 10gb
presto.tpch.overrides.tpch_tiny: 10gb

However, I think that inheritance should be explicit parameter to benchto driver instead of base path value, e.g:

--configuration overrides1.yml overrides2.yml

This has following advantages:

  • possibility to provide multiple override files (e.g: different overrides for tpch and tpchds)
  • you won't have to write override file for each benchmark file (base only points to one file)
  • Bechto won't be additionally responsible for file resolution.
  • we would avoid static path binding between overrides and benchmark files.

What do you think?

FYI: @kokosing

sopel39 avatar Mar 22 '17 16:03 sopel39

I'm not following everything, but seems to be quite complex because allows lots of ways to do same. In particular, I can't reason about profile yaml. Let's take example from above:

runs: 6
prewarm-runs: 2
tpch_tiny: 10gb

# NEW: it should be possible to override properties of individual benchmarks
presto.tpch.runs: 8
presto.tpch.tpch_tiny: 100gb

# Is this the same as:
presto:
  tpch:
    runs: 8
    tpch_tiny: 100gb

Q1: Is this file named like teradata.profile.yaml? Q2: Is first section contains defaults, and second for particular benchmark and those from particular benchmark inherit from default one? If so, presto.tpch.runs should follow file name presto/tpch.yaml? Q3: Do you restrict somehow what can be overriden?

As I understand, this is about moving all inheritance into one profile file, but inheritance is strictly from default profile?

Let me understand this proposal. I think though we can have two separate for now and figure out what we want from inheritance, what exactly is duplicated and then return back to this.

idemura avatar Mar 22 '17 19:03 idemura

I will try to answer your questions (@sopel39 please correct me if am wrong) and add my two cents.

A1. It could. A2. Yes. Yes. A3. No. The idea is leave this up to the user. We think that user may impose a convention that only some variables should be updatable, like in your case only these from overrides section.

Additionally I think that driver should be able to print out the all benchmarks configuration where all the inheritance (profile files) are applied (like docker-compose) with additional information what file produced this line. That way it will be really easy to track down any kind of issue with benchmark configuration.

I think the current problem (feature) with driver is that it loads all the benchmark files at once without any kind of order. We take advantage of this as we are running all the benchmarks against the cluster within some time limit. So this is why we would like to have one profile file to configure them all, or to have couple profile files to configure couple groups of benchmarks.

As I understand you would prefer to run benchmark one by one, is that correct? In that case you would like to say to benchto-driver to run this benchmark file, and then run this one.

kokosing avatar Mar 23 '17 06:03 kokosing

Let me understand this proposal. I think though we can have two separate for now and figure out what we want from inheritance, what exactly is duplicated and then return back to this.

We could have a copy for now in order not to block benchmarks. Concurrently we could improve benchto and refactor benchmarks so that they follow our proposal, so that copy is no longer needed.

sopel39 avatar Mar 23 '17 07:03 sopel39

@sopel39 I have concern this this approach.

If you guys say confirm that

If so, presto.tpch.runs should follow file name presto/tpch.yaml?

but above you complain about static binding with paths:

We would avoid static path binding between overrides and benchmark files.

Looks like contradiction to me. Profile should not reference file.

Consider use case. You have tpch, tpcds, facebook benchmarks (for our typical worloads), for your workload. Now I want say:

  • Make tpch daily, with one size (smaller),
  • Run tpch and tpcds weekly, tpch with different size,
  • Run fb weekly,
  • Do not run teradata benchmarks.

So, I want run only some benchmarks (should be supported), ability to have tpch with 2 profiles, maybe different profiles for facebook benchmark.

Params to benchmarks, as we concluded above, should be explicit in template (benchmark file with description of files).

# tpch.yaml:
profile: # overrides before
  a = ...
  b = ...

<description>

and profiles, that you can reuse (in command line: --benchmark=tpch.yaml --profiles=facebook_daily.yaml):

# facebook_daily
a = ...
b = ...

Seems like very clear design that supports all that we agreed to be values of this feature.

idemura avatar Mar 24 '17 00:03 idemura

but above you complain about static binding with paths:

It's not strictly a path, but rather benchmark name. The difference is that profile yaml can be located anywhere and override specific benchmark params by name, which base was explicitly pointing to a benchmark file with relative path.

So, I want run only some benchmarks (should be supported), ability to have tpch with 2 profiles, maybe different profiles for facebook benchmark.

Yes, we have a mechanism for that which is called activeBenchmarks. We are working to improve it, see @findepi PR: https://github.com/prestodb/benchto/pull/38

Other than that I think your use case/proposal is in line with our idea. Therefore I think we could follow this direction.

sopel39 avatar Mar 24 '17 09:03 sopel39

Great. Now there is not path in environment overrides section so profile (in env) can be applied to any benchmark description. It is not coupled with file name as I can see from PR. Good! Thanks.

idemura avatar Mar 24 '17 22:03 idemura