Allow measuring load with a gauge instead of a meter

[jdanbrown]

I've spec'ed a relatively simple extension to allow smart listeners to measure their load using an arbitrary Gauge[Double] instead of a Meter. Meter-based load then becomes a special case where gauge.value = meter.oneMinuteRate.

Additions to the API:

• ClusterConfig.useSmartGaugeBalancing
• SmartGaugedListener
• GaugedBalancingPolicy (superclass of MeteredBalancingPolicy, which is now almost trivial)

A smart gauged listener looks like this:

abstract class SmartGaugedListener extends Listener {
  def startWork(workUnit: String)
  def workload(workUnit: String): Double
}

Note that the user doesn't actually expose a Gauge[Double] anywhere—they instead provide def workload(workUnit: String): Double which Cluster uses to make gauges for GaugedBalancingPolicy. Alternatively, the user could expose something like a Map[String, Gauge[Double]], but it seems unnecessarily complicated—would it offer any advantage that I'm not thinking of?

Everything is backward compatible except for MeteredBalancingPolicy.meters: AtomicMap.atomicNBHM[String, Meter], which is now MeteredBalancingPolicy.gauges: AtomicMap.atomicNBHM[String, Gauge[Double]]—is that an important compatibility to maintain?

Note that GaugedBalancingPolicy.scala is mostly the same as the previous version of MeteredBalancingPolicy.scala, but the github diff doesn't indicate that. Do a manual diff to see.

Let me know what you think.

[eribeiro]

This commit is very cool. Really. But you forgot to justify what strong benefits the change from Gauge to Meter will bring on, and include additional unit tests that show that this extensive change doesn't break anything. I mean, your changes are pretty cool, but they add extra complexity (I am looking at the pattern matching stuff) so you should have strong use cases and requirements that justify that, and Ordasity is already used outside Boundary so it would be nice to listen what other users have to say and carefully evaluate its impact before commit this change, if ever. Scott has the final word, nonetheless. :)

OTOH the PR 12 is very nice! Congrats! :-)

protip: never forget the tests! Any line of code you add, add two more lines of tests!

Cheers,
Edward

[jdanbrown]

This isn't a change from meters to gauges, it's just adding gauge-based balancing in addition to the existing support for meter-based balancing. I'm using gauge balancing because I have a memory-bound cluster whose load can't be properly measured with a meter. As noted above, existing meter functionality is almost completely unchanged. In particular, the existing test suite passes with only one minor name change.

I disagree that the pattern matching adds extra complexity—on the contrary, I think it removes complexity by expressing control flow in a more uniform way that's easier to read.

[eribeiro]

I know you are adding support to gauges and I take your point, but it is exactly this name changing one of the things that worries me with this commit. Anything that changes 'public' methods/interfaces/fields should be taken with extra care not to break working code in production. First possibility : this test suit change is irrelevant an it is OK to commit. Second possibility: it can break working code and should not be done ( the name change or any public method / interface change for that matter ).

It is a very nice commit, but I suggest to keep it on the backburner until more Ordasity users (or the project leader, who has the final word) manifest their interested on having it commited. I understand that you need this change, but you is not the only user of this project, so let's hear other people, even if it takes times. I am in favour of your change, at first, but let's hear others.

PS: pattern matching is cool and I use it a lot, but lines 70 - 75 freaked me out, just that. :-) I can obviousy understand them, but a truth table? Really ?! :-)

[cscotta]

Hey Dan,

I dig the concept of the ability to balance by gauge – we've got a(t least one) memory-bound app as well, so that approach totally makes sense.

Give me another day or so to look over the code. I dig the implementation but have a couple thoughts on how a bit of it might be able to be cleaned up a bit (this might not be true and the impl might be the best way as-is, which'd be fine -- but need to square that with my mental model first).

But yeah – gauge-based balancing would be excellent and is something that I've been wanting to have for awhile. Really excited that you put this together; it'll be great to have in Ordasity.

– Scott

[jdanbrown]

Hey Scott, sounds great, no rush.

I agree that there's some potential for clean up—here are two things off the top of my head, and maybe you have some other stuff in mind too:

1. I dislike that Cluster is responsible for knowing that a MeteredBalancingPolicy is a special case of a GaugedBalancingPolicy. I wanted this detail to be encapsulated in MeteredBalancingPolicy somewhere, but I decided instead to stick closer to the original structure of the code and this was the most natural solution. Maybe there's a clever, small reorg to get this encapsulation.
2. The names (CountListener, SmartListener, SmartGaugedListener) and (useSmartBalancing, useSmartGaugedBalancing) should probably be brought more inline with the names (CountBalancingPolicy, GaugedBalancingPolicy, MeteredBalancingPolicy). I think we could do this and provide back compat with deprecation.