basho / basho_bench

A load-generation and testing tool for basically whatever you can write a returning Erlang function for.
Apache License 2.0
311 stars 197 forks source link

Fixes bug that prevents the use of custom key and value generators #221

Closed jbernardo95 closed 6 years ago

jbernardo95 commented 6 years ago

The function new/2 in both basho_bench_keygen and basho_bench_valgen is expected to return a function.

But if the generator provided is of type {function, Module, Function, Args} the new/2 function returns, instead of a function, the key or the value itself.

This PR fixes this problem by wrapping the erlang:apply call in a function so that the new/2 function returns a function instead of a value.

shino commented 6 years ago

@jbernardo95 Do you have some example that cause the bug?

I'm not sure (because of my poor memory), but the function {function, Mod, Fun, A} may return function that generates values? For example,

bar(UpperBound) ->
   fun() -> rand:uniform(UpperBound) end.

for the {function, foo, bar, 1000000}.

jbernardo95 commented 6 years ago

@shino Yes, you are right, I misread the docs.

From the docs:

{function, Module, Function, Args} — specifies an external function that should return a key generator function. The worker Id will be prepended to Args when the function is called.
jbernardo95 commented 6 years ago

@shino I guess I did this error because the way you set custom generators is not intuitive and not consistent with the default generators.

If I setup basho_bench to use the default uniform_int generator, in my head I think of uniform_int as a function that returns an integer (and this applies to all other default generators).

Why should the custom generator be different ? Why should it be a function that returns a function and not an integer ? Does not make sense to me.

On the other hand, your documentation is pretty good, so no excuses :)

shino commented 6 years ago

If I setup basho_bench to use the default uniform_int generator, in my head I think of uniform_int as a function that returns an integer (and this applies to all other default generators).

The source of uniform_int is a function that returns a function:

new({uniform_int, StartKey, NumKeys}, _Id)
  when is_integer(StartKey), is_integer(NumKeys), NumKeys > 0 ->
    fun() -> random:uniform(NumKeys) + StartKey - 1 end;

Why should the custom generator be different ? Why should it be a function that returns a function and not an integer ? Does not make sense to me.

Both can be acceptable ;) It's a matter of design, I think. And, basho_bench chose the current design and expects custom functions return functions (this is not correct, see below). Then, it is not very nice to change the design to not break compatibility.


Precisely speaking, custom function can return anything. One example is the driver for Riak CS (an object storage).

https://github.com/basho/basho_bench/blob/master/src/basho_bench_driver_cs.erl#L254

It returns a tuple of function and a kind of "state". This shows the flexibility of current design.

jbernardo95 commented 6 years ago

@shino Fair enough !

Thanks for the quick response 🙂