New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Timer sum double #442
Timer sum double #442
Conversation
…the sample bundle
1. serializeV1 is only for testing and writes sum as long 2. Tests prove that written in any format, the serializer is able to deserialize them.
methods to take timerVersion parameter to reduce code duplication
@@ -535,7 +556,7 @@ private static AbstractRollupStat getStatFromRollup(byte statType, BasicRollup b | |||
public ByteBuffer toByteBuffer(Object o) { | |||
try { | |||
byte type = typeOf(o); | |||
byte[] buf = new byte[sizeOf(o, type)]; | |||
byte[] buf = new byte[sizeOf(o, type, VERSION_2_TIMER)]; |
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.
It seems a bit strange to pass in the timer
version while serializing non-timer rollups like gauges, counters etc. What do you think about having two overloaded methods named sizeOf
with one accepting just two arguments i.e (Object, Type)
which will be used by all non-timer rollup types and then the other one with timerVersion
as the third argument, which gets called by the timer serializer?
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 is much cleaner. I originally thought there was a problem doing it this way, but it seems much better. I'll push some changes shortly.
+1 |
This is the continuation of:
#440
Chinmay asked for some changes, and Tilo asked me to do them while she is at Rax.io for the week.
The only part of this PR that still needs to be reviewed is this:
24ecd6e