search

uber-node / ringpop-node

Scalable, fault-tolerant application-layer sharding for Node.js applications
http://uber.github.io/ringpop/
MIT License
1.18k stars 146 forks source link

Emit stats from ringpop-test program #271

Closed mennopruijssers closed 8 years ago

mennopruijssers commented 8 years ago

In preparation for automated performance and failure testing we want to be able to emit stats to a file or a network address from testpop. This commit adds a new flag to main.js to control if and where stats should be emitted; if the flag is omitted, no stats are emitted.

motiejus commented 8 years ago

Same question as in Go version. How are we going to test this?

mennopruijssers commented 8 years ago

@Motiejus currently main.js is currently not tested in an automated way: it's just a simple "example"-app. If we want to test this, I would vote for moving it to it's own module or repo, let it depend on uber/ringpop-node#master and add tests there for behaviour like this (and do the same for go's counterpart). I don't think that it should be part of this PR but could be a "pre-requirement" if we decide to move forward with this approach.

motiejus commented 8 years ago

I think we can make at least a unit test for the matching part; shouldn't be too hard?

I still think overloading a single configuration key for two different values doesn't seem right; do you think otherwise (we can have a talk, maybe I`m misunderstanding something)

motiejus commented 8 years ago

LGTM!

btromanova commented 8 years ago

After small fix in option description and comment about using _ephemeralSocket --- lgtm.