-
Notifications
You must be signed in to change notification settings - Fork 7
Conversation
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.
Could you maybe also fix the README.markdown
in the counter
scenario and remove (or adapt) the comment about the actual and expected counter value in section "Scripted chaos tests".
##### Docker toolbox | ||
|
||
Although Mac recently got a *native* [Docker for Mac](https://docs.docker.com/docker-for-mac/) implementation you still | ||
have to use the [Docker Toolbox][toolbox] (which interfaces VirtualBox) to use Blockade and therefore *eventuate-chaos* |
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.
spelling: Blockade -> blockade (here and in the next line)
resolvers += "OJO Snapshots" at "https://oss.jfrog.org/oss-snapshot-local" | ||
resolvers += "Eventuate Releases" at "https://dl.bintray.com/rbmhtechnology/maven" | ||
|
||
val eventuateVersion = "0.8-M1" |
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.
Maybe M2 now :-)
"com.typesafe.akka" %% "akka-remote" % "2.4-M2" % "test", | ||
"com.rbmhtechnology" %% "eventuate" % "0.5-SNAPSHOT" % "test", | ||
"org.slf4j" % "slf4j-log4j12" % "1.7.9" % "test" | ||
"com.typesafe.akka" %% "akka-actor" % "2.4.4", |
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.
The akka dependencies come transitively with eventuate. I would propose to leave them out here so they do not have to be maintained in multiple places.
for node, port in nodes.items(): | ||
interact.request(HOST, port, 'dump') | ||
|
||
return '' |
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.
What is this good for?
for node, port in nodes.items(): | ||
interact.request(HOST, port, 'dump') | ||
|
||
return '' | ||
|
||
class CounterOperation(interact.Operation): | ||
|
||
def __init__(self): | ||
self.state = {'counter': 0} |
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.
This looks crude. This is a Map instead of a plain int so it can be passed around and modified?!?
PARSER = argparse.ArgumentParser(description='start CRDT-counter chaos test') | ||
PARSER.add_argument('-i', '--iterations', type=int, default=30, help='number of failure/partition iterations') | ||
PARSER.add_argument('--interval', type=float, default=0.1, help='delay between requests') | ||
PARSER.add_argument('-l', '--locations', type=int, default=3, help='number of locations') | ||
PARSER.add_argument('-d', '--delay', type=int, default=10, help='delay between each random partition') | ||
PARSER.add_argument('--settle', type=int, default=60, help='number of seconds to wait for settling') | ||
PARSER.add_argument('-r', '--runners', type=int, default=1, help='number of request runners') |
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.
Would it make sense to make the default 3 (like the number of locations?)
|
||
if COUNTER_VALUE is None: | ||
sys.exit(1) | ||
else: | ||
print('All %d nodes converged to the counter value: %d' % (len(NODES), COUNTER_VALUE)) | ||
|
||
EXPECTED_VALUE = INITIAL_COUNTER + DIFF | ||
if COUNTER_VALUE != EXPECTED_VALUE: |
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.
Doee this make sense at all, given that requests that fail are also considered in the DIFF
?
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.
There are test scenarios that may succeed without failures (i.e. LevelDB based tests).
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.
Only LevelDB without restarts, right?
Hey @kongo2002, I hope you're doing well! Are you going to work in this / the feedback, or do you think someone else should take over? |
Hi @magro! As soon as there is some spare time I will look into this PR again - probably a matter of days I assume. But no promises for sure ;-) Cheers, |
Ok, great! :-) |
@kongo2002 Thanks for fixing the issues. |
This PR serves as a RC for the first release including:
state-cassandra
scenario at least) (Chaos tests should include event-sourced actor crashes #6)