Re: Review Request 45721: thermos local-time
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45721/#review130023 --- Se Choi, I wanted to wake this thread and see if you intended to proceed with this review or not. - John Sirois On April 5, 2016, 1:58 p.m., se choi wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45721/ > --- > > (Updated April 5, 2016, 1:58 p.m.) > > > Review request for Aurora, Joshua Cohen, John Sirois, and Stephan Erb. > > > Repository: aurora > > > Description > --- > > thermos local-time > > 1. Scheduler display +09:00 (Locale timezone) > 2. Thermos Observer display on UTC (+00:00) > > > Diffs > - > > src/main/python/apache/thermos/observer/http/templates/main.tpl > b905699897254b4a3ff6a3d03a072ac24d56e929 > src/main/python/apache/thermos/observer/http/templates/process.tpl > 4ca52bac41e638bb26c17bddee0d8946df895522 > src/main/python/apache/thermos/observer/http/templates/task.tpl > f3e06985eb3c05572aa4389d97da575b1179f616 > > Diff: https://reviews.apache.org/r/45721/diff/ > > > Testing > --- > > > Thanks, > > se choi > >
Re: Review Request 46459: Schema changes for resource management refactoring
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46459/ --- (Updated April 21, 2016, 11:03 p.m.) Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji. Changes --- Fixing unit test. Repository: aurora Description --- This is the first patch towards the generic resource management approach described [here](https://docs.google.com/document/d/1J9SIswRMpVKQpnlvJAMAJtKfPP7ZARFknuyXl-2aZ-M/edit#). 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 analysis - The requested ports have been flatmapped together with other resource types to simplify handling - 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` disabled) - `LogStorage` backfilling covers transaction replays - `ConfigurationManager` backfilling covers external RPC handling between v-1/v+1 client/server combinations Also, made some test changes to ensure our tests pass with or without `DBTaskStore` enabled. Diffs (updated) - api/src/main/thrift/org/apache/aurora/gen/api.thrift 0cf4d6e428806c2b17bd8a824a0bd4fc83c80766 src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java d91c742c7783905214de563321771ce33dba74a3 src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 041cec38f1f96985bbf85468ece0ebbcef2f2486 src/main/java/org/apache/aurora/scheduler/http/api/GsonMessageBodyHandler.java 44295f80ba8b464d502e72c11c517fac28716c59 src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java b6fc949d31371304c2e71363dd61fbf40cfd0ac8 src/main/java/org/apache/aurora/scheduler/resources/ResourceTypeConverter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 360914ede6f1978a071a9f7a94d1f328bc252e60 src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 25160df1be9539c1ecd4613db5deb99a9606a512 src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java e778a39ef61e456ad9d2d2aa6f3e5d69e44bf02c src/main/java/org/apache/aurora/scheduler/storage/db/migration/V003_CreateResourceTypesTable.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/migration/V004_CreateTaskResourceTable.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/ResourceTypeHandler.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java ed561c65947d41861d80229ad459392a4bceadf1 src/main/java/org/apache/aurora/scheduler/storage/db/views/DBResource.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java cdd1060401a38e26cd726ec95e2bb617ccf4708c src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java f586186b61a6025ffccef8cafe480aa6dbf9681c src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java b6922e1d5bdd188304359b5646be437ed09ec8d1 src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java PRE-CREATION src/main/python/apache/aurora/config/thrift.py be0cd68674a71bd4baadf276f40a4bc0223ce4be src/main/python/apache/aurora/tools/java/thrift_wrapper_codegen.py 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/SchedulerIT.java c31069047a9d59d6d635a86f673dd23c2b6868f1
Re: Review Request 46459: Schema changes for resource management refactoring
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46459/#review129971
---
Master (95dcca5) is red with this patch.
./build-support/jenkins/build.sh
src/test/python/apache/thermos/common/test_pathspec.py::test_exception_on_none_keys
[32mPASSED[0m
src/test/python/apache/thermos/common/test_planner.py::test_planner_empty
[32mPASSED[0m
src/test/python/apache/thermos/common/test_planner.py::test_planner_unordered
[32mPASSED[0m
src/test/python/apache/thermos/common/test_planner.py::test_planner_ordered
[32mPASSED[0m
src/test/python/apache/thermos/common/test_planner.py::test_planner_mixed
[32mPASSED[0m
src/test/python/apache/thermos/common/test_planner.py::test_planner_unsatisfiables
[32mPASSED[0m
FAILURES
TestInspectCommand.test_inspect_job_raw _
self =
[1mdef test_inspect_job_raw(self):[0m
[1m mock_stdout = [][0m
[1m def mock_print_out(msg, indent=0):[0m
[1mindent_str = " " * indent[0m
[1mmock_stdout.append("%s%s" % (indent_str,
msg))[0m
[1m job_config = self.get_job_config()[0m
[1m with contextlib.nested([0m
[1m
patch('apache.aurora.client.cli.context.AuroraCommandContext.print_out',[0m
[1m side_effect=mock_print_out),[0m
[1m
patch('apache.aurora.client.cli.context.AuroraCommandContext.get_job_config',[0m
[1m return_value=job_config)):[0m
[1mcmd = AuroraCommandLine()[0m
[1massert cmd.execute(['job', 'inspect', '--raw',
'west/bozo/test/hello', 'config.aurora']) == 0[0m
[1moutput = '\n'.join(mock_stdout)[0m
[1m> assert output == str(job_config.job())[0m
[1m[31mE AssertionError: assert
"JobConfigura...r='jenkins'))" == "JobConfigurat...r='jenkins'))"[0m
[1m[31mE Detailed information truncated, use
"-vv" to show[0m
src/test/python/apache/aurora/client/cli/test_inspect.py:92: AssertionError
generated xml file:
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
[1m[31m 1 failed, 655 passed, 6 skipped, 1 warnings in
197.57 seconds [0m
FAILURE
[32m
Waiting for background workers to finish.[0m
22:17:21 04:03 [complete][31m
FAILURE[0m
I will refresh this build result if you post a review containing "@ReviewBot
retry"
- Aurora ReviewBot
On April 21, 2016, 9:57 p.m., Maxim Khutornenko wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46459/
> ---
>
> (Updated April 21, 2016, 9:57 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](https://docs.google.com/document/d/1J9SIswRMpVKQpnlvJAMAJtKfPP7ZARFknuyXl-2aZ-M/edit#).
>
> 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 analysis
> - The requested ports have been flatmapped together with other resource types
> to simplify handling
> - 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;
>
Re: Review Request 46506: Fix bug when fetching a task with multiple assigned ports.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46506/#review129940 --- Ship it! Master (15eaa5d) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing "@ReviewBot retry" - Aurora ReviewBot On April 21, 2016, 7:02 p.m., Joshua Cohen wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46506/ > --- > > (Updated April 21, 2016, 7:02 p.m.) > > > Review request for Aurora and Maxim Khutornenko. > > > Bugs: AURORA-1672 > https://issues.apache.org/jira/browse/AURORA-1672 > > > Repository: aurora > > > Description > --- > > Because we did not include the ids for task ports and task events in the > query when mybatis tried > to map the results, there was no way to identify the task events returned in > each additional row > for ports as distinct. This resulted in each subsequent task fetch tacking on > extra task events. > > > Diffs > - > > src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml > fb78a3952c99de828945e84fc218873ea21dbbc5 > > src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java > 310c4d8e34ad75c080a22e30343c7da6d4a01e3c > > Diff: https://reviews.apache.org/r/46506/diff/ > > > Testing > --- > > Distilled down to the added test case which verifies the fix. > > Also verified the fix in the UI. > > > Thanks, > > Joshua Cohen > >
Re: Review Request 46506: Fix bug when fetching a task with multiple assigned ports.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46506/#review129939 --- Ship it! Ship It! - Maxim Khutornenko On April 21, 2016, 7:02 p.m., Joshua Cohen wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46506/ > --- > > (Updated April 21, 2016, 7:02 p.m.) > > > Review request for Aurora and Maxim Khutornenko. > > > Bugs: AURORA-1672 > https://issues.apache.org/jira/browse/AURORA-1672 > > > Repository: aurora > > > Description > --- > > Because we did not include the ids for task ports and task events in the > query when mybatis tried > to map the results, there was no way to identify the task events returned in > each additional row > for ports as distinct. This resulted in each subsequent task fetch tacking on > extra task events. > > > Diffs > - > > src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml > fb78a3952c99de828945e84fc218873ea21dbbc5 > > src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java > 310c4d8e34ad75c080a22e30343c7da6d4a01e3c > > Diff: https://reviews.apache.org/r/46506/diff/ > > > Testing > --- > > Distilled down to the added test case which verifies the fix. > > Also verified the fix in the UI. > > > Thanks, > > Joshua Cohen > >
Review Request 46506: Fix bug when fetching a task with multiple assigned ports.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46506/ --- Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1672 https://issues.apache.org/jira/browse/AURORA-1672 Repository: aurora Description --- Because we did not include the ids for task ports and task events in the query when mybatis tried to map the results, there was no way to identify the task events returned in each additional row for ports as distinct. This resulted in each subsequent task fetch tacking on extra task events. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml fb78a3952c99de828945e84fc218873ea21dbbc5 src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 310c4d8e34ad75c080a22e30343c7da6d4a01e3c Diff: https://reviews.apache.org/r/46506/diff/ Testing --- Distilled down to the added test case which verifies the fix. Also verified the fix in the UI. Thanks, Joshua Cohen
Re: Review Request 46499: Revert "Moving db migration into LogStorage"
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46499/#review129925 --- Ship it! Master (15eaa5d) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing "@ReviewBot retry" - Aurora ReviewBot On April 21, 2016, 5:08 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46499/ > --- > > (Updated April 21, 2016, 5:08 p.m.) > > > Review request for Aurora and Joshua Cohen. > > > Repository: aurora > > > Description > --- > > This reverts commit ae051f3b92797d5c9f328c6c6d42d03ee4077938. > > Moving the migrator call back to snapshot store. > > I realized we can't rely on post-recover data migration alone to satisfy > backfilling needs. During recovery, we perform various insertion calls (mem > store or log storage catch up) that rely on thrift schema matching mybatis > schema. This may not always be true (as we learned in AURORA-1603) and may > result in incorrect or duplicate data being inserted. > > The correct data migration sequence for all configuration cases should be: > 1. migrate schema > 2. migrate any data loaded from dbsnapshot (if applicable) > 3. apply snapshot with backfilling > 4. replay log store transactions with backfilling > > > Diffs > - > > > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java > 5c7d92f00ddda0a1f366ba1ca33b61829fa16ad9 > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java > 74c468860b42a19c29b624f6f0978e6a1ef895d3 > > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java > 97b9e261be69cd77149aca4ba20d5c628f857aef > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > a2b1d2216c69a0d983434c8c600d461d538badf7 > > src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java > d5918b92a6461003772ab3d7d4440a92ba6cdd80 > > Diff: https://reviews.apache.org/r/46499/diff/ > > > Testing > --- > > > Thanks, > > Maxim Khutornenko > >
Review Request 46499: Revert "Moving db migration into LogStorage"
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46499/ --- Review request for Aurora and Joshua Cohen. Repository: aurora Description --- This reverts commit ae051f3b92797d5c9f328c6c6d42d03ee4077938. Moving the migrator call back to snapshot store. I realized we can't rely on post-recover data migration alone to satisfy backfilling needs. During recovery, we perform various insertion calls (mem store or log storage catch up) that rely on thrift schema matching mybatis schema. This may not always be true (as we learned in AURORA-1603) and may result in incorrect or duplicate data being inserted. The correct data migration sequence for all configuration cases should be: 1. migrate schema 2. migrate any data loaded from dbsnapshot (if applicable) 3. apply snapshot with backfilling 4. replay log store transactions with backfilling Diffs - src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 5c7d92f00ddda0a1f366ba1ca33b61829fa16ad9 src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 74c468860b42a19c29b624f6f0978e6a1ef895d3 src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 97b9e261be69cd77149aca4ba20d5c628f857aef src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java a2b1d2216c69a0d983434c8c600d461d538badf7 src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java d5918b92a6461003772ab3d7d4440a92ba6cdd80 Diff: https://reviews.apache.org/r/46499/diff/ Testing --- Thanks, Maxim Khutornenko
