Re: Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.
> On Aug. 11, 2016, 6:16 p.m., Michael Park wrote: > > Hi Klaus, could you explain what the motivation is for this patch? > > Currently, your analysis seems correct that reserved resources are always > > non-revocable. > > However, the current code seems that it'll be more future-proof. > > That is, even after reserved resources becomes revocable it would remain > > correct. > > > > Anyway, I'm curiuos as to why this patch is being suggested. Thanks! > > Klaus Ma wrote: > Try to improve the performance by avoid unnecessary operation :). > > Michael Park wrote: > That would've been my guess. Are there any numbers to support the patch? > > Klaus Ma wrote: > The number dependent on cases; anyway, I'll append some number for it. > > Guangya Liu wrote: > I think that this will not impact performance much as we always need two > resources operations here: `nonRevocable()` and `+` , the time consumed in > those two calls should be same even with this fix. > > Michael Park wrote: > Hey Klaus, Just curious if you've determined whether or not this actually > has notable improvements? > If so, could you post some numbers? If not, could you discard the review? @Michael, let's discard this for now. I'm working on other tasks, wil re-open it when the numbers is ready. - Klaus --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45081/#review145489 --- On April 19, 2016, 12:01 p.m., Klaus Ma wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45081/ > --- > > (Updated April 19, 2016, 12:01 p.m.) > > > Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and > Michael Park. > > > Bugs: MESOS-4988 > https://issues.apache.org/jira/browse/MESOS-4988 > > > Repository: mesos > > > Description > --- > > Allocator will only allocate non-revocable resources to satify quota. As the > reserved resources can not be revocable, it's not necessary to call > `nonRevocable()` for reserved resources. > > > Diffs > - > > src/master/allocator/mesos/hierarchical.cpp > 70291075c00a9a557529c2562dedcfc6c6c3ec32 > > Diff: https://reviews.apache.org/r/45081/diff/ > > > Testing > --- > > make > make check > > > Thanks, > > Klaus Ma > >
Re: Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.
> On Aug. 11, 2016, 10:16 a.m., Michael Park wrote: > > Hi Klaus, could you explain what the motivation is for this patch? > > Currently, your analysis seems correct that reserved resources are always > > non-revocable. > > However, the current code seems that it'll be more future-proof. > > That is, even after reserved resources becomes revocable it would remain > > correct. > > > > Anyway, I'm curiuos as to why this patch is being suggested. Thanks! > > Klaus Ma wrote: > Try to improve the performance by avoid unnecessary operation :). > > Michael Park wrote: > That would've been my guess. Are there any numbers to support the patch? > > Klaus Ma wrote: > The number dependent on cases; anyway, I'll append some number for it. > > Guangya Liu wrote: > I think that this will not impact performance much as we always need two > resources operations here: `nonRevocable()` and `+` , the time consumed in > those two calls should be same even with this fix. Hey Klaus, Just curious if you've determined whether or not this actually has notable improvements? If so, could you post some numbers? If not, could you discard the review? - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45081/#review145489 --- On April 19, 2016, 4:01 a.m., Klaus Ma wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45081/ > --- > > (Updated April 19, 2016, 4:01 a.m.) > > > Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and > Michael Park. > > > Bugs: MESOS-4988 > https://issues.apache.org/jira/browse/MESOS-4988 > > > Repository: mesos > > > Description > --- > > Allocator will only allocate non-revocable resources to satify quota. As the > reserved resources can not be revocable, it's not necessary to call > `nonRevocable()` for reserved resources. > > > Diffs > - > > src/master/allocator/mesos/hierarchical.cpp > 70291075c00a9a557529c2562dedcfc6c6c3ec32 > > Diff: https://reviews.apache.org/r/45081/diff/ > > > Testing > --- > > make > make check > > > Thanks, > > Klaus Ma > >
Re: Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.
> On 八月 11, 2016, 10:16 a.m., Michael Park wrote: > > Hi Klaus, could you explain what the motivation is for this patch? > > Currently, your analysis seems correct that reserved resources are always > > non-revocable. > > However, the current code seems that it'll be more future-proof. > > That is, even after reserved resources becomes revocable it would remain > > correct. > > > > Anyway, I'm curiuos as to why this patch is being suggested. Thanks! > > Klaus Ma wrote: > Try to improve the performance by avoid unnecessary operation :). > > Michael Park wrote: > That would've been my guess. Are there any numbers to support the patch? > > Klaus Ma wrote: > The number dependent on cases; anyway, I'll append some number for it. I think that this will not impact performance much as we always need two resources operations here: `nonRevocable()` and `+` , the time consumed in those two calls should be same even with this fix. - Guangya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45081/#review145489 --- On 四月 19, 2016, 4:01 a.m., Klaus Ma wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45081/ > --- > > (Updated 四月 19, 2016, 4:01 a.m.) > > > Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and > Michael Park. > > > Bugs: MESOS-4988 > https://issues.apache.org/jira/browse/MESOS-4988 > > > Repository: mesos > > > Description > --- > > Allocator will only allocate non-revocable resources to satify quota. As the > reserved resources can not be revocable, it's not necessary to call > `nonRevocable()` for reserved resources. > > > Diffs > - > > src/master/allocator/mesos/hierarchical.cpp > 70291075c00a9a557529c2562dedcfc6c6c3ec32 > > Diff: https://reviews.apache.org/r/45081/diff/ > > > Testing > --- > > make > make check > > > Thanks, > > Klaus Ma > >
Re: Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.
> On Aug. 11, 2016, 6:16 p.m., Michael Park wrote: > > Hi Klaus, could you explain what the motivation is for this patch? > > Currently, your analysis seems correct that reserved resources are always > > non-revocable. > > However, the current code seems that it'll be more future-proof. > > That is, even after reserved resources becomes revocable it would remain > > correct. > > > > Anyway, I'm curiuos as to why this patch is being suggested. Thanks! > > Klaus Ma wrote: > Try to improve the performance by avoid unnecessary operation :). > > Michael Park wrote: > That would've been my guess. Are there any numbers to support the patch? The number dependent on cases; anyway, I'll append some number for it. - Klaus --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45081/#review145489 --- On April 19, 2016, 12:01 p.m., Klaus Ma wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45081/ > --- > > (Updated April 19, 2016, 12:01 p.m.) > > > Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and > Michael Park. > > > Bugs: MESOS-4988 > https://issues.apache.org/jira/browse/MESOS-4988 > > > Repository: mesos > > > Description > --- > > Allocator will only allocate non-revocable resources to satify quota. As the > reserved resources can not be revocable, it's not necessary to call > `nonRevocable()` for reserved resources. > > > Diffs > - > > src/master/allocator/mesos/hierarchical.cpp > 70291075c00a9a557529c2562dedcfc6c6c3ec32 > > Diff: https://reviews.apache.org/r/45081/diff/ > > > Testing > --- > > make > make check > > > Thanks, > > Klaus Ma > >
Re: Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.
> On Aug. 11, 2016, 10:16 a.m., Michael Park wrote: > > Hi Klaus, could you explain what the motivation is for this patch? > > Currently, your analysis seems correct that reserved resources are always > > non-revocable. > > However, the current code seems that it'll be more future-proof. > > That is, even after reserved resources becomes revocable it would remain > > correct. > > > > Anyway, I'm curiuos as to why this patch is being suggested. Thanks! > > Klaus Ma wrote: > Try to improve the performance by avoid unnecessary operation :). That would've been my guess. Are there any numbers to support the patch? - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45081/#review145489 --- On April 19, 2016, 4:01 a.m., Klaus Ma wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45081/ > --- > > (Updated April 19, 2016, 4:01 a.m.) > > > Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and > Michael Park. > > > Bugs: MESOS-4988 > https://issues.apache.org/jira/browse/MESOS-4988 > > > Repository: mesos > > > Description > --- > > Allocator will only allocate non-revocable resources to satify quota. As the > reserved resources can not be revocable, it's not necessary to call > `nonRevocable()` for reserved resources. > > > Diffs > - > > src/master/allocator/mesos/hierarchical.cpp > 70291075c00a9a557529c2562dedcfc6c6c3ec32 > > Diff: https://reviews.apache.org/r/45081/diff/ > > > Testing > --- > > make > make check > > > Thanks, > > Klaus Ma > >
Re: Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.
> On Aug. 11, 2016, 6:16 p.m., Michael Park wrote: > > Hi Klaus, could you explain what the motivation is for this patch? > > Currently, your analysis seems correct that reserved resources are always > > non-revocable. > > However, the current code seems that it'll be more future-proof. > > That is, even after reserved resources becomes revocable it would remain > > correct. > > > > Anyway, I'm curiuos as to why this patch is being suggested. Thanks! Try to improve the performance by avoid unnecessary operation :). - Klaus --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45081/#review145489 --- On April 19, 2016, 12:01 p.m., Klaus Ma wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45081/ > --- > > (Updated April 19, 2016, 12:01 p.m.) > > > Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and > Michael Park. > > > Bugs: MESOS-4988 > https://issues.apache.org/jira/browse/MESOS-4988 > > > Repository: mesos > > > Description > --- > > Allocator will only allocate non-revocable resources to satify quota. As the > reserved resources can not be revocable, it's not necessary to call > `nonRevocable()` for reserved resources. > > > Diffs > - > > src/master/allocator/mesos/hierarchical.cpp > 70291075c00a9a557529c2562dedcfc6c6c3ec32 > > Diff: https://reviews.apache.org/r/45081/diff/ > > > Testing > --- > > make > make check > > > Thanks, > > Klaus Ma > >
Re: Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45081/#review145489 --- Hi Klaus, could you explain what the motivation is for this patch? Currently, your analysis seems correct that reserved resources are always non-revocable. However, the current code seems that it'll be more future-proof. That is, even after reserved resources becomes revocable it would remain correct. Anyway, I'm curiuos as to why this patch is being suggested. Thanks! - Michael Park On April 19, 2016, 4:01 a.m., Klaus Ma wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45081/ > --- > > (Updated April 19, 2016, 4:01 a.m.) > > > Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and > Michael Park. > > > Bugs: MESOS-4988 > https://issues.apache.org/jira/browse/MESOS-4988 > > > Repository: mesos > > > Description > --- > > Allocator will only allocate non-revocable resources to satify quota. As the > reserved resources can not be revocable, it's not necessary to call > `nonRevocable()` for reserved resources. > > > Diffs > - > > src/master/allocator/mesos/hierarchical.cpp > 70291075c00a9a557529c2562dedcfc6c6c3ec32 > > Diff: https://reviews.apache.org/r/45081/diff/ > > > Testing > --- > > make > make check > > > Thanks, > > Klaus Ma > >
Re: Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45081/#review124449 --- Patch looks great! Reviews applied: [45081] Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh - Mesos ReviewBot On March 20, 2016, 2:06 p.m., Klaus Ma wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45081/ > --- > > (Updated March 20, 2016, 2:06 p.m.) > > > Review request for mesos and Alexander Rukletsov. > > > Bugs: MESOS-4988 > https://issues.apache.org/jira/browse/MESOS-4988 > > > Repository: mesos > > > Description > --- > > Allocator will only allocate non-revocable resources to satify quota. As the > reserved resources can not be revocable, it's not necessary to call > `nonRevocable()` for reserved resources. > > > Diffs > - > > src/master/allocator/mesos/hierarchical.cpp > 70291075c00a9a557529c2562dedcfc6c6c3ec32 > > Diff: https://reviews.apache.org/r/45081/diff/ > > > Testing > --- > > make > make check > > > Thanks, > > Klaus Ma > >
Re: Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45081/ --- (Updated March 20, 2016, 10:06 p.m.) Review request for mesos and Alexander Rukletsov. Changes --- Update the bugs Bugs: MESOS-4988 https://issues.apache.org/jira/browse/MESOS-4988 Repository: mesos Description --- Allocator will only allocate non-revocable resources to satify quota. As the reserved resources can not be revocable, it's not necessary to call `nonRevocable()` for reserved resources. Diffs - src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 Diff: https://reviews.apache.org/r/45081/diff/ Testing --- make make check Thanks, Klaus Ma
Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45081/ --- Review request for mesos and Alexander Rukletsov. Repository: mesos Description --- Allocator will only allocate non-revocable resources to satify quota. As the reserved resources can not be revocable, it's not necessary to call `nonRevocable()` for reserved resources. Diffs - src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 Diff: https://reviews.apache.org/r/45081/diff/ Testing --- make make check Thanks, Klaus Ma
