spotify / ffwd

a flexible metric forwarding agent
https://spotify.github.io/ffwd/
Apache License 2.0
79 stars 33 forks source link

Address the issue in 0.6.5 release #209

Closed malish8632 closed 4 years ago

malish8632 commented 4 years ago

This is fixing the issue encountered in production where services with high frequency metrics (more than one) stopped sending metrics.

Exception example: Error in channel, closing java.lang.ClassCastException: class com.spotify.ffwd.model.Metric cannot be cast to class java.lang.Comparable (com.spotify.ffwd.model.Metric is in unnamed module of loader 'app'; java.lang.Comparable is in module java.base of loader 'bootstrap')

@ao2017

sming commented 4 years ago

Do we require strict/stable ordering? I.e. where 2 (or more) identical elements are guaranteed to be put in the same order every time? If so, this test doesn't verify that, unless I'm mistaken.

On Wed, Aug 5, 2020 at 3:04 PM Sergey Rustamov notifications@github.com wrote:

@malish8632 commented on this pull request.

In api/src/test/java/com/spotify/ffwd/util/HighFrequencyDetectorTest.java https://github.com/spotify/ffwd/pull/209#discussion_r465940738:

  • for (int i = 0; i < 30; i++) {
  • list.add(
  • new Metric(
  • "KEY" + y,
  • 42.0 + i,
  • new Date(),
  • ImmutableSet.of(),
  • Map.of("tag1", "value1", "what", "fun"),
  • ImmutableMap.of(),
  • null));
  • }
  • finalList.addAll(detector.detect(list));
  • }
  • }
  • assertEquals(90, finalList.size());

I don't care about sorting itself as long as it is consistent.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/spotify/ffwd/pull/209#discussion_r465940738, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKWXWR2DYGULP3DO7U43E3R7GUKXANCNFSM4PVYZJZQ .

--

--

Peter Kingswell

Backend Engineer, Prism Squad

peterk@spotify.com

Brooklyn, NY

+1 646 675 6541

innovative | collaborative | sincere | passionate | playful

malish8632 commented 4 years ago

@sming the test is not testing what you describe. Also the logic of sorting is in private method, we can't test.

the logic requires the elements to be sorted all the time in the same order. ex:

  1. key1,key3 <--- this is first case we got only 2 elements (every time it is executed the logic returns them in the same order)
  2. key1,key2,key3 <--- in this case new element was added but every time it is sorted it will produce this result