-
Notifications
You must be signed in to change notification settings - Fork 314
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
Allow to control recency of ids in conflicts #529
Allow to control recency of ids in conflicts #529
Conversation
With this commit we add a new parameter to the `bulk` parameter source called `recency` that allows to control the bias towards more recent ids when generating id conflicts.
esrally/track/params.py
Outdated
# A recency > 0 biases id selection towards more recent ids. The recency parameter decides | ||
# by how much we bias. See docs for the resulting curve. | ||
# | ||
# idx_range is in the interval [0;1]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess there's a typo here, [0;1]
-> [0,1]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was actually intentional. The German decimal separator is a comma so it could be misinterpreted by a German as 1/10 instead of the interval from zero to 1 (inclusive). However, we can change it as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it anyway now as I've seen that we use the comma notation in all other places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very elegant way of handling recency!
Thanks for adding thorough documentation and the example plot.
LGTM.
esrally/track/params.py
Outdated
idx_range = min(self.randexp(GenerateActionMetaData.RECENCY_SLOPE * self.recency), 1) | ||
# the resulting index is in the range [0, self.id_up_to). Note that a smaller idx_range | ||
# biases towards more recently used ids (higher indexes). | ||
idx = int(round((self.id_up_to - 1) * (1 - idx_range))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does round()
anyway return an int
, if the optional ndigits parameter is not passed?
No harm being explicit here, but wondering about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, you are right. I was probably assuming that it would behave similarly to Math#round
in Java, hence the explicit conversion.
With this commit we add a new parameter to the
bulk
parameter sourcecalled
recency
that allows to control the bias towards more recentids when generating id conflicts.