Add synchronized around getCount dist usage

[spkrka]

Since TDigest objects are not thread safe, we need to protect reads from
getting corrupted data or potentially throwing exceptions.

[storozhukBM]

LGTM

[ao2017]

Sorry, I may have lost track of thing. Why do we need synchronized around distRef ? It is an atomic object .

[spkrka]

Without this change, it is possible for one thread to read the size of the TDigest at the same time as it's being updated by the record method. Since TDigest is not a thread safe object, this could result in either incorrect output from size, or possibly even unexpected exceptions

[ao2017]

Without this change, it is possible for one thread to read the size of the TDigest at the same time as it's being updated by the record method. Since TDigest is not a thread safe object, this could result in either incorrect output from size, or possibly even unexpected exceptions

I am not sure this is necessary. Tdigest is accessed through an atomicReference. Every operation is atomic so we will never missed any count.

If locking is not an issue, we can synchonized every method and remove the atomicReference. I don't think we need sychronized and the atomic reference.

[spkrka]

One thread does this:
distRef.get().add(..)

Another thread does this:
distRef.get().size()

Having an AtomicReference does not prevent add(..) from being called at the same time as size()

[ao2017]

One thread does this:
distRef.get().add(..)

Another thread does this:
distRef.get().size()

Having an AtomicReference does not prevent add(..) from being called at the same time as size()

One thread does this:
distRef.get().add(..)

Another thread does this:
distRef.get().size()

Having an AtomicReference does not prevent add(..) from being called at the same time as size()

That's correct. my bad. It has been a while :). I was more concern about thread safety around add / read operation. I was ok with size being off. I don't see a problem making getCount precise but that will increase context switching. What kind of stat are you getting on locking with this implementation ?

[spkrka]

I did not measure, and I am not sure how getCount is used - is it used for anything important?

[ao2017]

I did not measure, and I am not sure how getCount is used - is it used for anything important?

Almost every 30 seconds, whenever ffwdReporter reports metrics.