I've implemented the above suggestion here: https://reviews.apache.org/r/61238/
On Fri, Jul 28, 2017 at 7:19 PM, Bill Farner <wfar...@apache.org> wrote: > 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 >> > >
