Skip to content

Commit

Permalink
Fix bug allowing zero conflict probability
Browse files Browse the repository at this point in the history
PR elastic#510 doesn't allow setting conflict probability to 0 as the
parameter check is failing, assuming only open intervals are allowed.

Fix bug and also add a number of unit tests.
  • Loading branch information
dliappis committed May 25, 2018
1 parent f00b32e commit 9e0f02f
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 4 deletions.
10 changes: 6 additions & 4 deletions esrally/track/params.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import math
import types
import inspect
import operator
from enum import Enum

from esrally import exceptions
Expand Down Expand Up @@ -443,7 +444,7 @@ def __init__(self, track, params, **kwargs):
raise exceptions.InvalidSyntax("Unknown 'conflicts' setting [%s]" % id_conflicts)

if self.id_conflicts != IndexIdConflict.NoConflicts:
self.conflict_probability = self.float_param(params, name="conflict-probability", default_value=25, min_value=0, max_value=100)
self.conflict_probability = self.float_param(params, name="conflict-probability", default_value=25, min_value=0, max_value=100, min_operator=operator.lt)
self.on_conflict = params.get("on-conflict", "index")
if self.on_conflict not in ["index", "update"]:
raise exceptions.InvalidSyntax("Unknown 'on-conflict' setting [{}]".format(self.on_conflict))
Expand Down Expand Up @@ -484,12 +485,13 @@ def __init__(self, track, params, **kwargs):

self.ingest_percentage = self.float_param(params, name="ingest-percentage", default_value=100, min_value=0, max_value=100)

def float_param(self, params, name, default_value, min_value, max_value):
def float_param(self, params, name, default_value, min_value, max_value, min_operator=operator.le):
try:
value = float(params.get(name, default_value))
if value <= min_value or value > max_value:
if min_operator(value, min_value) or value > max_value:
interval_min = "(" if min_operator is operator.le else "["
raise exceptions.InvalidSyntax(
"'{}' must be in the range ({:.1f}, {:.1f}] but was {:.1f}".format(name, min_value, max_value, value))
"'{}' must be in the range {}{:.1f}, {:.1f}] but was {:.1f}".format(name, interval_min, min_value, max_value, value))
return value
except ValueError:
raise exceptions.InvalidSyntax("'{}' must be numeric".format(name))
Expand Down
37 changes: 37 additions & 0 deletions tests/track/params_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,43 @@ def test_restricts_number_of_bulks_if_required(self):
# should issue three bulks of size 10.000
self.assertEqual(3, partition.size())

def test_create_with_conflict_probability_zero(self):
params.BulkIndexParamSource(track=track.Track(name="unit-test"), params={
"bulk-size": 5000,
"conflicts": "sequential",
"conflict-probability": 0
})

def test_create_with_conflict_probability_too_low(self):
with self.assertRaises(exceptions.InvalidSyntax) as ctx:
params.BulkIndexParamSource(track=track.Track(name="unit-test"), params={
"bulk-size": 5000,
"conflicts": "sequential",
"conflict-probability": -0.1
})

self.assertEqual("'conflict-probability' must be in the range [0.0, 100.0] but was -0.1", ctx.exception.args[0])

def test_create_with_conflict_probability_too_high(self):
with self.assertRaises(exceptions.InvalidSyntax) as ctx:
params.BulkIndexParamSource(track=track.Track(name="unit-test"), params={
"bulk-size": 5000,
"conflicts": "sequential",
"conflict-probability": 100.1
})

self.assertEqual("'conflict-probability' must be in the range [0.0, 100.0] but was 100.1", ctx.exception.args[0])

def test_create_with_conflict_probability_not_numeric(self):
with self.assertRaises(exceptions.InvalidSyntax) as ctx:
params.BulkIndexParamSource(track=track.Track(name="unit-test"), params={
"bulk-size": 5000,
"conflicts": "sequential",
"conflict-probability": "100 percent"
})

self.assertEqual("'conflict-probability' must be numeric", ctx.exception.args[0])


class BulkDataGeneratorTests(TestCase):
class TestBulkReader:
Expand Down

0 comments on commit 9e0f02f

Please sign in to comment.