search

scylladb / scylla-bench

43 stars 36 forks source link

main.go: improve -select-order-by #79

Closed dkropachev closed 2 years ago

dkropachev commented 2 years ago

Rename it to -select-order-by Make it possible to run ascending order also Make it possible to run quries with different orders one by one

mmatczuk commented 2 years ago

I fail to understand the rationale behind running with random order. Usually you want runs to be repeatable, for instance you can pre generate a random sequence of order by. So if you really want this feature it shall be very explicit in the CLI.

dkropachev commented 2 years ago

I fail to understand the rationale behind running with random order. Usually you want runs to be repeatable, for instance you can pre generate a random sequence of order by. So if you really want this feature it shall be very explicit in the CLI.

Thanks for catching it, you are totally right, I've redone that part and also make it be not random, but rather pick these orders one by one, which makes it possiblle to use it for performance too.

mmatczuk commented 2 years ago

To be clear the rand part can be easily put to bash with $RANDOM it does not belong here.

The other issue is that this program does not initialize seed, only in test. This means that all runs select the same element from the input array.

dkropachev commented 2 years ago

To be clear the rand part can be easily put to bash with $RANDOM it does not belong here.

The other issue is that this program does not initialize seed, only in test. This means that all runs select the same element from the input array.

Sorry, did not push the change. Please take a one more look at the code, it is not using random anymore.

I've also see that seed is set here - https://github.com/scylladb/scylla-bench/blob/master/random/random.go#L34-L38 I will take a look at the seed subject and create another issue+PR if it is necessary.

Thanks.