aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joshua Cohen <jco...@apache.org>
Subject Re: Review Request 47261: Generalizing AcceptedOffer resource management.
Date Mon, 16 May 2016 14:40:18 GMT


> On May 13, 2016, 7:17 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/resources/MesosResourceConverter.java,
line 52
> > <https://reviews.apache.org/r/47261/diff/3/?file=1380194#file1380194line52>
> >
> >     Instead of having this be `Supplier<?>` can we change the MesosResourceConverter
interface to take a type parameter?
> >     
> >     So..
> >     
> >         public interface MesosResourceConverter<T> {
> >           ...
> >            
> >           Iterable<Resource> toMesosResource(
> >               ...
> >               Supplier<T> resourceRequest,
> >               ...);
> >            
> >           ...
> >            
> >           class ScalarConverter implements MesosResourceConverter<Double>
{
> >             ...
> >           }
> >             
> >           class RangeConverter implements MesosResourceConverter<Set<Integer>>
{
> >             ...
> >           }
> >         }
> >     
> >     That should allow us to do away with the casts and the `@SuppressWarnings`?
> 
> Maxim Khutornenko wrote:
>     Unfortunately it won't. This is exactly the path I started with but had to backpedal
due to having even worse cast/warning cases. The problem is that `ResourceType` has to expose
`MesosResourceConverter<?>` and that results in an ugly case when converter typeparam
had no bearing on port mapper typeparam, which was essentially a `? != ?` case. This is one
of the best examples where lack of Java generics reification bites back hard. The current
approach minimizes the amount of damage in that respect.

Yeah, apologies for not assuming you had already tried this, but thanks for explaining the
issue!


- Joshua


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47261/#review133009
-----------------------------------------------------------


On May 13, 2016, 10:58 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47261/
> -----------------------------------------------------------
> 
> (Updated May 13, 2016, 10:58 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This patch moves `AcceptedOffer` onto a new resource model. 
> 
> Not particularly happy with the unchecked suppression but all alternatives required handling
different mesos resource types outside of the `MesosResourceConverter`, which was less than
ideal. Happy to consider any suggestions here.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 703ca3b707320bbda48fa89c45404093b241e785

>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java ef1b5bc2990035ba359509c861fb44ff27b5b8fe

>   src/main/java/org/apache/aurora/scheduler/resources/AcceptedOffer.java fce6621bc3fd88a9b891fb464e28bf5da0c3867b

>   src/main/java/org/apache/aurora/scheduler/resources/MesosResourceConverter.java f3fe05cadd93e99e2a6f2f644b7f850ce9647d03

>   src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java 3b38469b4819f9ae232c468df8ba652f25001ad3

>   src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java c8e11a406f8a0d19d4e3abf08ab46c27a4003e38

>   src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java dea7943d40eef1c31663c506a06b8e933f8e6dd2

>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 276320ae03f93a5351c9d3648dd35f578f6e4cfe

>   src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java cf4d350c35e8f539457850a6b3dca45369c3849e

>   src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java 36c5c115e9e7ca0bc0e3b34e4148edf349d1e874

>   src/test/java/org/apache/aurora/scheduler/resources/MesosResourceConverterTest.java
d4bb5aa1dcc846f8f4d4d153dd733a9c45537976 
>   src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java ba597b866275e438a113c58206abbb430cc2a535

> 
> Diff: https://reviews.apache.org/r/47261/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message