V313patch #38

2 commits merged into from Jan 18, 2011


None yet

2 participants


Implemented per http://code.google.com/p/project-voldemort/issues/detail?id=313

  • New JMX options:
    • percentGetReturningEmptyResponse
    • percentGetAllReturningEmptyResponse
    • max{Put,Get,GetAll,Delete}LatencyInMs
    • {average,max}{Put,Get,GetAll}SizeInBytes
  • New unit tests for all.

Had to specialize the type for StatsTrackingStore to in order to support the size metrics.

Since the contract for get and get_all is to return empty lists or no result, respectively, for empty results, this is how the "null response rate" has been descirbed. Before this patch the RequestCounter class (and accompanying Accumulator class) were very generic. This patch adds operation-specific logic into it. This bends the abstraction a reasonable amount, though much more would argue for subclassing these. It may also be reasonable to do so now; let me know what you think.

Still need to add support to StatusServlet for these new JMX stats, if needed. Wanted to get feedback at this point.

All unit tests pass, except for for voldemort.store.mysql.MysqlStorageEngineTest, which fails on master for me as well.


Looks great. Only comment I would have is whether it is possible to use Time interface to fake out the clock instead of Thread.sleep? This doesn't always work, but we have had a lot of trouble with sleeps in tests being a little slow and fragile.


Updated to use mockito to mock the Time interface to fake out the passage of time, so there's no need to sleep. Good call. This required adding a new constructor for the dependency injection.

Re: StatusServlet. This seems like an odd beast. After going to the effort of publishing via jmx, we then run our own servlet to display them? Does this need to be updated for the new stats? It seems like a burden to be in the business of displaying the jmx results, particularly since the servlet has a dependency on the jmx but is not closely tied to it.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment