search

NationalSecurityAgency / timely

Accumulo backed time series database
https://code.nsa.gov/timely/
Apache License 2.0
378 stars 108 forks source link

Closes issue #203 - avoid calling sizeInBytes if it's not expected to exist #204

Closed matthpeterson closed 4 years ago

matthpeterson commented 4 years ago

@billoley are you ok with this MR? Jstack showed a lot of thrown exceptions on the now removed code and so I think it is best to remove it.

dlmarion commented 4 years ago

Another approach would be to fix the code such that it does not throw the NoSuchMethodException. You could check if the Method exists in the class' declared methods, then invoke it if it exists. As an optimization, you could keep a list of classes that had previous successful checks.

billoley commented 4 years ago

I don't see anything that implements the ObjectSizeOf method. I think this code was copied either from datawave or accumulo and used to estimate when to return from the DownsampleIterator to avoid blowing up the tserver. It would be a good idea as a followon to implement that interface on Downsample and Tag as a start. Might lead to better performance in all queries. Good catch!

I agree with your logic (class should implement the interface if it wants that call) and the change.

billoley commented 4 years ago

Correction - DownsampleIteratorTest tests this approach (implementing ObjectSizeOf) , but nothing else yet.

matthpeterson commented 4 years ago

Thanks for the reviews. I don't have merge privileges.