You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2014/09/24 02:44:03 UTC

Review Request 25969: When creating an update, store only the delta between the initial and desired states.

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

Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


Bugs: AURORA-718
    https://issues.apache.org/jira/browse/AURORA-718


Repository: aurora


Description
-------

When creating an update, store only the delta between the initial and desired states.


Diffs
-----

  src/main/java/org/apache/aurora/scheduler/base/Numbers.java 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
  src/main/java/org/apache/aurora/scheduler/base/Tasks.java 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
  src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java b7143941feb4824aee609741e8e2c0e015eecac3 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 15a9e2b9acd65f6268e70fb402414f5c01c42409 
  src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
  src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ae0320b44b55a3630e255484ea7a881daab6a7bc 
  src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
  src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306 
  src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 1d0bbfef583708bfdab30802724273e4b21ecf8f 
  src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 842256a16fe2775af865ed2ee91fd719b3476577 
  src/main/python/apache/aurora/client/api/__init__.py bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
  src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml e1179b449c05adc56206bfef271a9d440a77e042 
  src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518 
  src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
  src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
  src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java f7c7205d80edac714a12c2aff0cc7db27b1909b4 
  src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java ad4040c4a58d3e6f075a60712144aa6968270a55 
  src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 548d31580b5f2bae92900fddb80005f7d274a155 

Diff: https://reviews.apache.org/r/25969/diff/


Testing
-------

./gradlew build -Pq


Thanks,

Bill Farner


Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

Posted by Bill Farner <wf...@apache.org>.

> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java, line 195
> > <https://reviews.apache.org/r/25969/diff/1/?file=703687#file703687line195>
> >
> >     s/_get_/_fetch_ to match method name.

Fixed.


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, lines 1405-1410
> > <https://reviews.apache.org/r/25969/diff/1/?file=703689#file703689line1405>
> >
> >     How about moving this into a static helper in JobDiff for better readability?

Sure.


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 1415
> > <https://reviews.apache.org/r/25969/diff/1/?file=703689#file703689line1415>
> >
> >     Suggest a more descriptive message here mentioning updateOnlyTheseInstances property name.

Done.


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 148
> > <https://reviews.apache.org/r/25969/diff/1/?file=703692#file703692line148>
> >
> >     Does this translate into INVALID_REQUEST in thrift? Not sure it's a good idea unless we expose a jobDiff RPC to reliably compare job configs before the update. Otherwise, it's client diff vs. server diff with a chance of mismatch and subpar client experience.
> >     
> >     Should it rather be special-cased to return OK with a detail message stating a noop update?

Sounds good, changed to no-op/OK.


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 468
> > <https://reviews.apache.org/r/25969/diff/1/?file=703692#file703692line468>
> >
> >     Unless I am reading it wrong, I am not sure how this could even be possible now that noop instances are rejected by the Diff. Is there still a case for it?

Changing from rolling forward to rolling back will be the most common.  The updater will flip the instances, and skip untouched instances while walking backwards.
It can also happen if the scheduler fails over - in-flight instances will be re-evaluated, and may no-op at that point.


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 694
> > <https://reviews.apache.org/r/25969/diff/1/?file=703697#file703697line694>
> >
> >     s/../.

Fixed.


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml, line 421
> > <https://reviews.apache.org/r/25969/diff/1/?file=703696#file703696line421>
> >
> >     Why left join here? Events without update ID would not make any sense.

Thanks!  I clearly can't be trusted when it comes to SQL.


- Bill


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


On Sept. 24, 2014, 12:44 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25969/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2014, 12:44 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-718
>     https://issues.apache.org/jira/browse/AURORA-718
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> When creating an update, store only the delta between the initial and desired states.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
>   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
>   src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java b7143941feb4824aee609741e8e2c0e015eecac3 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 15a9e2b9acd65f6268e70fb402414f5c01c42409 
>   src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ae0320b44b55a3630e255484ea7a881daab6a7bc 
>   src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 1d0bbfef583708bfdab30802724273e4b21ecf8f 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 842256a16fe2775af865ed2ee91fd719b3476577 
>   src/main/python/apache/aurora/client/api/__init__.py bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml e1179b449c05adc56206bfef271a9d440a77e042 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java f7c7205d80edac714a12c2aff0cc7db27b1909b4 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java ad4040c4a58d3e6f075a60712144aa6968270a55 
>   src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 548d31580b5f2bae92900fddb80005f7d274a155 
> 
> Diff: https://reviews.apache.org/r/25969/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25969/#review54419
-----------------------------------------------------------



src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java
<https://reviews.apache.org/r/25969/#comment94545>

    s/_get_/_fetch_ to match method name.



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/25969/#comment94549>

    How about moving this into a static helper in JobDiff for better readability?



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/25969/#comment94550>

    Suggest a more descriptive message here mentioning updateOnlyTheseInstances property name.



src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
<https://reviews.apache.org/r/25969/#comment94558>

    Does this translate into INVALID_REQUEST in thrift? Not sure it's a good idea unless we expose a jobDiff RPC to reliably compare job configs before the update. Otherwise, it's client diff vs. server diff with a chance of mismatch and subpar client experience.
    
    Should it rather be special-cased to return OK with a detail message stating a noop update?



src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
<https://reviews.apache.org/r/25969/#comment94561>

    Unless I am reading it wrong, I am not sure how this could even be possible now that noop instances are rejected by the Diff. Is there still a case for it?



src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
<https://reviews.apache.org/r/25969/#comment94546>

    Why left join here? Events without update ID would not make any sense.



src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/25969/#comment94547>

    s/../.


- Maxim Khutornenko


On Sept. 24, 2014, 12:44 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25969/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2014, 12:44 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-718
>     https://issues.apache.org/jira/browse/AURORA-718
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> When creating an update, store only the delta between the initial and desired states.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
>   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
>   src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java b7143941feb4824aee609741e8e2c0e015eecac3 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 15a9e2b9acd65f6268e70fb402414f5c01c42409 
>   src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ae0320b44b55a3630e255484ea7a881daab6a7bc 
>   src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 1d0bbfef583708bfdab30802724273e4b21ecf8f 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 842256a16fe2775af865ed2ee91fd719b3476577 
>   src/main/python/apache/aurora/client/api/__init__.py bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml e1179b449c05adc56206bfef271a9d440a77e042 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java f7c7205d80edac714a12c2aff0cc7db27b1909b4 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java ad4040c4a58d3e6f075a60712144aa6968270a55 
>   src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 548d31580b5f2bae92900fddb80005f7d274a155 
> 
> Diff: https://reviews.apache.org/r/25969/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25969/
-----------------------------------------------------------

(Updated Sept. 25, 2014, 5:03 a.m.)


Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


Changes
-------

Rebase on master.


Bugs: AURORA-718
    https://issues.apache.org/jira/browse/AURORA-718


Repository: aurora


Description
-------

When creating an update, store only the delta between the initial and desired states.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/base/Numbers.java 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
  src/main/java/org/apache/aurora/scheduler/base/Tasks.java 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
  src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java b7143941feb4824aee609741e8e2c0e015eecac3 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 15a9e2b9acd65f6268e70fb402414f5c01c42409 
  src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
  src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4602dbb69a675097430de1345bf6bfea0d15ceca 
  src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 3bf81e6d05d6dc8240737fad0ad48e194ba3166c 
  src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 7efcaa4a80b25db69f4bbaa79435e58997ea393e 
  src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 1d0bbfef583708bfdab30802724273e4b21ecf8f 
  src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 842256a16fe2775af865ed2ee91fd719b3476577 
  src/main/python/apache/aurora/client/api/__init__.py bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
  src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml e1179b449c05adc56206bfef271a9d440a77e042 
  src/main/thrift/org/apache/aurora/gen/api.thrift 7436a5ac61013f576649665d5f585fa5e1bb2a56 
  src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
  src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 10fa31db10a80cbc05a4c79728d687766135b5e2 
  src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java f7c7205d80edac714a12c2aff0cc7db27b1909b4 
  src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java ad4040c4a58d3e6f075a60712144aa6968270a55 
  src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 548d31580b5f2bae92900fddb80005f7d274a155 

Diff: https://reviews.apache.org/r/25969/diff/


Testing
-------

./gradlew build -Pq


Thanks,

Bill Farner


Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25969/#review54442
-----------------------------------------------------------

Ship it!


Ship It!

- Maxim Khutornenko


On Sept. 24, 2014, 6:02 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25969/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2014, 6:02 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-718
>     https://issues.apache.org/jira/browse/AURORA-718
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> When creating an update, store only the delta between the initial and desired states.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
>   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
>   src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java b7143941feb4824aee609741e8e2c0e015eecac3 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 15a9e2b9acd65f6268e70fb402414f5c01c42409 
>   src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ae0320b44b55a3630e255484ea7a881daab6a7bc 
>   src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 1d0bbfef583708bfdab30802724273e4b21ecf8f 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 842256a16fe2775af865ed2ee91fd719b3476577 
>   src/main/python/apache/aurora/client/api/__init__.py bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml e1179b449c05adc56206bfef271a9d440a77e042 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java f7c7205d80edac714a12c2aff0cc7db27b1909b4 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java ad4040c4a58d3e6f075a60712144aa6968270a55 
>   src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 548d31580b5f2bae92900fddb80005f7d274a155 
> 
> Diff: https://reviews.apache.org/r/25969/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

Posted by Bill Farner <wf...@apache.org>.

> On Sept. 25, 2014, 12:04 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java, line 49
> > <https://reviews.apache.org/r/25969/diff/2/?file=704586#file704586line49>
> >
> >     Should this be a RangeSet<Integer> instead?

I chose against this simply because it was non-trivial to translate Set<Integer> -> RangeSet<Integer>.  I believe it involves looping over the integers, and using something like:

    rangeBuilder.add(Range.closedOpen(i, i+1).canonicalize(DiscreteDomain.integers()));

but i found that pretty unreadable.  Since i already have a Set to begin with and the object is quickly discarded, i didn't see much value in converting it to a RangeSet.


- Bill


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


On Sept. 24, 2014, 6:02 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25969/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2014, 6:02 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-718
>     https://issues.apache.org/jira/browse/AURORA-718
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> When creating an update, store only the delta between the initial and desired states.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
>   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
>   src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java b7143941feb4824aee609741e8e2c0e015eecac3 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 15a9e2b9acd65f6268e70fb402414f5c01c42409 
>   src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ae0320b44b55a3630e255484ea7a881daab6a7bc 
>   src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 1d0bbfef583708bfdab30802724273e4b21ecf8f 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 842256a16fe2775af865ed2ee91fd719b3476577 
>   src/main/python/apache/aurora/client/api/__init__.py bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml e1179b449c05adc56206bfef271a9d440a77e042 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java f7c7205d80edac714a12c2aff0cc7db27b1909b4 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java ad4040c4a58d3e6f075a60712144aa6968270a55 
>   src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 548d31580b5f2bae92900fddb80005f7d274a155 
> 
> Diff: https://reviews.apache.org/r/25969/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25969/#review54483
-----------------------------------------------------------

Ship it!



src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java
<https://reviews.apache.org/r/25969/#comment94633>

    Should this be a RangeSet<Integer> instead?


- Kevin Sweeney


On Sept. 24, 2014, 11:02 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25969/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2014, 11:02 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-718
>     https://issues.apache.org/jira/browse/AURORA-718
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> When creating an update, store only the delta between the initial and desired states.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
>   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
>   src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java b7143941feb4824aee609741e8e2c0e015eecac3 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 15a9e2b9acd65f6268e70fb402414f5c01c42409 
>   src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ae0320b44b55a3630e255484ea7a881daab6a7bc 
>   src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 1d0bbfef583708bfdab30802724273e4b21ecf8f 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 842256a16fe2775af865ed2ee91fd719b3476577 
>   src/main/python/apache/aurora/client/api/__init__.py bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml e1179b449c05adc56206bfef271a9d440a77e042 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java f7c7205d80edac714a12c2aff0cc7db27b1909b4 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java ad4040c4a58d3e6f075a60712144aa6968270a55 
>   src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 548d31580b5f2bae92900fddb80005f7d274a155 
> 
> Diff: https://reviews.apache.org/r/25969/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25969/
-----------------------------------------------------------

(Updated Sept. 24, 2014, 6:02 p.m.)


Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


Bugs: AURORA-718
    https://issues.apache.org/jira/browse/AURORA-718


Repository: aurora


Description
-------

When creating an update, store only the delta between the initial and desired states.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/base/Numbers.java 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
  src/main/java/org/apache/aurora/scheduler/base/Tasks.java 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
  src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java b7143941feb4824aee609741e8e2c0e015eecac3 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 15a9e2b9acd65f6268e70fb402414f5c01c42409 
  src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
  src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ae0320b44b55a3630e255484ea7a881daab6a7bc 
  src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
  src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306 
  src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 1d0bbfef583708bfdab30802724273e4b21ecf8f 
  src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 842256a16fe2775af865ed2ee91fd719b3476577 
  src/main/python/apache/aurora/client/api/__init__.py bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
  src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml e1179b449c05adc56206bfef271a9d440a77e042 
  src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518 
  src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
  src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
  src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java f7c7205d80edac714a12c2aff0cc7db27b1909b4 
  src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java ad4040c4a58d3e6f075a60712144aa6968270a55 
  src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 548d31580b5f2bae92900fddb80005f7d274a155 

Diff: https://reviews.apache.org/r/25969/diff/


Testing
-------

./gradlew build -Pq


Thanks,

Bill Farner