aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joshua Cohen <>
Subject Re: Review Request 46459: Schema changes for resource management refactoring
Date Fri, 22 Apr 2016 15:15:31 GMT

This is an automatically generated e-mail. To reply, visit:

Overall looks good to me. One question though: what happens in the scenario where we receive
a TaskConfig that has both the individual task-level resources populated as well as the new
resource collection. Further, what happens in that scenario if the values don't match? I don't
think we can rely on the client to send us valid data because there are lots of users who
generate thrift requests directly, rather than using the client.

src/main/java/org/apache/aurora/scheduler/configuration/ (line 322)

    Do you think it would make sense to find all instances of resources with multiple values
rather than stopping at the first? That way if someone specified multiple invalid resources
they can avoid multiple round trips to correct each instance?
    (I suppose it does potentially open up the ability for a malicious user to craft a config
that could cause the scheduler to perform extra work, but I'm not sure we're really protected
against that scenario in general)

src/main/java/org/apache/aurora/scheduler/resources/ (lines 49 -

    Would it make sense to make this a default method on the interface?

src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml (line 161)

    Should this be included as part of the initial query rather than requring a sub-select?

- Joshua Cohen

On April 21, 2016, 11:03 p.m., Maxim Khutornenko wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> -----------------------------------------------------------
> (Updated April 21, 2016, 11:03 p.m.)
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
> Repository: aurora
> Description
> -------
> This is the first patch towards the generic resource management approach described [here](
> Apologies for the large diff but this is the minimal logically correct set of changes
to ensure proper schema migration/backfilling. 
> A few design/implementation notes:
> - All resources are stored in a single table with values converted to String. This way
we can handle additional resource types by just adding a `ResourceTypeConverter` for the new
type. Among other benefits the increased human readability of SQL data simplifying ad-hoc
> - The requested ports have been flatmapped together with other resource types to simplify
> - Resource details are described in `ResourceType` that provides translation bridge between
generic representation and specific resource types
> - `IResource` immutable wrapper got couple of new methods:
>   - `public static Resource newBuilder(int id, Object value)`: for creating mutable resource
values in a generic way when reloading resources from DB;
>   - `public Object getRawValue()`: for accessing resource value in a generic way when
storing it in DB;
> Notes on data migration/backfilling:
> - The `resource_types` table needs to be populated before any migration happens as it
provides foreign keys for the `task_resource`
> - Next, `task_resource` data migration covering task configs already present in the DB
(e.g. those inserted by job updates or in case of `DBTaskStore` enabled)
> - `SnapshotStoreImpl` backfilling covers data not present in the dbsnapshot (e.g. `DBTaskStore`
> - `LogStorage` backfilling covers transaction replays
> - `ConfigurationManager` backfilling covers external RPC handling between v-1/v+1 client/server
> Also, made some test changes to ensure our tests pass with or without `DBTaskStore` enabled.
> Diffs
> -----
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 0cf4d6e428806c2b17bd8a824a0bd4fc83c80766

>   src/main/java/org/apache/aurora/scheduler/base/ d91c742c7783905214de563321771ce33dba74a3

>   src/main/java/org/apache/aurora/scheduler/configuration/ 041cec38f1f96985bbf85468ece0ebbcef2f2486

>   src/main/java/org/apache/aurora/scheduler/http/api/ 44295f80ba8b464d502e72c11c517fac28716c59

>   src/main/java/org/apache/aurora/scheduler/resources/ b6fc949d31371304c2e71363dd61fbf40cfd0ac8

>   src/main/java/org/apache/aurora/scheduler/resources/ PRE-CREATION

>   src/main/java/org/apache/aurora/scheduler/storage/db/ 360914ede6f1978a071a9f7a94d1f328bc252e60

>   src/main/java/org/apache/aurora/scheduler/storage/db/ 25160df1be9539c1ecd4613db5deb99a9606a512

>   src/main/java/org/apache/aurora/scheduler/storage/db/ e778a39ef61e456ad9d2d2aa6f3e5d69e44bf02c

>   src/main/java/org/apache/aurora/scheduler/storage/db/migration/
>   src/main/java/org/apache/aurora/scheduler/storage/db/migration/
>   src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/
>   src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/ PRE-CREATION

>   src/main/java/org/apache/aurora/scheduler/storage/db/views/ cdd1060401a38e26cd726ec95e2bb617ccf4708c

>   src/main/java/org/apache/aurora/scheduler/storage/log/ f586186b61a6025ffccef8cafe480aa6dbf9681c

>   src/main/java/org/apache/aurora/scheduler/storage/log/ b6922e1d5bdd188304359b5646be437ed09ec8d1

>   src/main/java/org/apache/aurora/scheduler/storage/log/ PRE-CREATION

>   src/main/python/apache/aurora/config/ be0cd68674a71bd4baadf276f40a4bc0223ce4be

>   src/main/python/apache/aurora/tools/java/ 3465fe9ab4a4403270854d20dbc1d20dfc40a80d

>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 36df7ed5e23e360f23e6130dca7ae1ce234ee2ce

>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 92a0798d4d97920b786c4713f8779a7a32767652

>   src/test/java/org/apache/aurora/scheduler/app/ c31069047a9d59d6d635a86f673dd23c2b6868f1

>   src/test/java/org/apache/aurora/scheduler/configuration/
>   src/test/java/org/apache/aurora/scheduler/resources/
>   src/test/java/org/apache/aurora/scheduler/resources/ PRE-CREATION

>   src/test/java/org/apache/aurora/scheduler/storage/backup/ a33f6f7cbfc1e8262585fdf6c59e14c2afc661e0

>   src/test/java/org/apache/aurora/scheduler/storage/log/ bf9479d60531324578354c4a15fcfdac040e7ffc

>   src/test/java/org/apache/aurora/scheduler/storage/log/ ff9c1d08a9a99a69e94d634c895d20fa6c8c2f88

>   src/test/java/org/apache/aurora/scheduler/storage/log/ PRE-CREATION

>   src/test/java/org/apache/aurora/scheduler/thrift/ fd6a40c8a6f0a16f0914b8025fd6207770bd740e

>   src/test/java/org/apache/aurora/scheduler/thrift/ 50a1fdbb3dad501bf12fc245e32123a8630dbba5

>   src/test/java/org/apache/aurora/scheduler/thrift/
>   src/test/python/apache/aurora/client/cli/ 452699c08d642e7166903d7797c1650c735266db

>   src/test/python/apache/aurora/config/ 88292d3c4423c0555088a0adaee3c0e62ed0567e

> Diff:
> Testing
> -------
> Manual scheduler upgrades/rollbacks in Vagrant.
> Thanks,
> Maxim Khutornenko

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