Neat bug! The implementation gets into trouble when different resource types are in play, giving inconsistent results depending on which argument is 'left'. e.g.
host A: cpu=2, disk=1 host B: cpu=2, ram=1 In this example, compare(A, B) considers A > B, and compare(B, A) considers B > A. The fix would be to take the union of ResourceTypes between the two ResourceBags being compared, rather than only iterating over the ResourceTypes in one parameter. On Fri, Jul 28, 2017 at 4:07 PM, Mauricio Garavaglia < mauriciogaravag...@gmail.com> wrote: > Hi guys, > > There seems to be a bug in this comparator in PreemptionVictimFilter ( > https://github.com/apache/aurora/blob/master/src/main/ > java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java# > L142) > because the comparator doesn’t satisfy transitivity, which is required by > Array.sort's Tim Sort implementation. This fails with a > "java.lang.IllegalArgumentException: Comparison method violates its > general > contract!" from time to time. > > For example this input of A, B, and C doesn't work in the current > comparator. > > A: <1, 1, 1> > B: <2, 2, 2> > C: <0, 2, 1> > > Based on the comparator: > > B > A > B == C > C == A > > Constructs impossible situation where C == B > A == C. As a workaround we > patched to simply sort based on RAM, but anyone have any suggestions about > what a permanent fix for upstream should be? Thanks > > Mauricio >
