> On Jan. 23, 2018, 6:11 p.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java > > Lines 319 (patched) > > <https://reviews.apache.org/r/65281/diff/1/?file=1944352#file1944352line319> > > > > Shouldn't `RUNNING` -> `PARTITIONED` be `DOWN`? > > Bill Farner wrote: > +1 > > Stephan Erb wrote: > I considered this as at first as well. However, it is the users choice to > be in `PARTITIONED` state, so this should not be counted as "cluster-induced > downtime". In addition, `PARTITIONED` does not indicate that the task is > down. It merely means that we don't know. > > David McLaughlin wrote: > That was my thought too. They are optimistically still running. > > Santhosh Kumar Shanmugham wrote: > That makes sense. > > Shouldn't we then include the time spent in `PARTITIONED` as `UP` in that > case? If I am reading it correctly, we will exclude time spent in > `PARTITIONED`, even if the task came an `UP` state. > > David McLaughlin wrote: > You're right, this should carry the same logic as RUNNING.
I've made this change. Santhosh I'll wait for your ship it before commit. Question: I put PARTITIONED here blindly because I saw STARTING. STARTING is now a legitimate running state... should STARTING also be considered UP? I'm not sure if it actually impacts the SLA % reported, but do I know a whole bunch of crons can stay in STARTING for their whole duration. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65281/#review196047 ----------------------------------------------------------- On Jan. 25, 2018, 2:04 a.m., David McLaughlin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65281/ > ----------------------------------------------------------- > > (Updated Jan. 25, 2018, 2:04 a.m.) > > > Review request for Aurora and Jordan Ly. > > > Repository: aurora > > > Description > ------- > > Support PARTITIONED state in SLA calculations. Also added a test to protect > against this test failing in the future. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java > 5d8d5bd8f705770979f284d26d2e932aabe707e5 > src/test/java/org/apache/aurora/scheduler/sla/SlaAlgorithmTest.java > 2e719ac6b7aea86faa22deff2cc6b5f73135761c > > > Diff: https://reviews.apache.org/r/65281/diff/2/ > > > Testing > ------- > > ./gradlew test > > > Thanks, > > David McLaughlin > >
