Re: Review Request 63176: Add Cache-Control header to static assets, to allow for cache expiration

[David McLaughlin]

> On Oct. 20, 2017, 5:01 a.m., Bill Farner wrote:
> > I don't feel strongly, but `DefaultServlet` can also send an etag header by 
> > instead setting `"etags", "true"`.
> 
> David McLaughlin wrote:
> Note that it would also need the change here otherwise the browser 
> doesn't send the "If-None-Match" header either. The theoritical advantage of 
> an E-Tag over Last-Modified is that the cache (should) survive a restart when 
> the content doesn't change. But I checked the implementation of the ETags 
> from DefaultServlet and it uses the Last-Modified value in there anyway. So I 
> think the Last-Modified value (which seems to be set to JAR creation time) is 
> what we want here outside of a custom content-addressable strong E-Tag 
> implementation.

Sorry that should be "survive a new build." 

This is the implementation of the E-Tag generator:


public String getWeakETag(String suffix)
{
try
{
StringBuilder b = new StringBuilder(32);
b.append("W/\"");

String name=getName();
int length=name.length();
long lhash=0;
for (int i=0; ihttps://reviews.apache.org/r/63176/#review188809
---


On Oct. 20, 2017, 4:46 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63176/
> ---
> 
> (Updated Oct. 20, 2017, 4:46 a.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The Scheduler currently returns last-modified headers for static assets, the 
> idea is that the client (the browser) will send HTTP GET requests with an 
> If-Modified-Since header and the Jetty Servlet will intercept them and return 
> 304s, saving everyone some download time. 
> 
> But without a max-age or Expires header set, the browsers are just caching 
> the assets indefinitely - causing real pains when you want to propagate UI 
> changes to users. This sets the max-age of the browser cache to an hour.
> 
> I'll have a follow-up review to disable caching of resources completely when 
> in Vagrant - because that gets annoying when developing the UI.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
> 
> 
> Diff: https://reviews.apache.org/r/63176/diff/1/
> 
> 
> Testing
> ---
> 
> I verified the caching behavior in Chrome with my local Vagrant.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63176: Add Cache-Control header to static assets, to allow for cache expiration

[David McLaughlin]

> On Oct. 20, 2017, 5:01 a.m., Bill Farner wrote:
> > I don't feel strongly, but `DefaultServlet` can also send an etag header by 
> > instead setting `"etags", "true"`.

Note that it would also need the change here otherwise the browser doesn't send 
the "If-None-Match" header either. The theoritical advantage of an E-Tag over 
Last-Modified is that the cache (should) survive a restart when the content 
doesn't change. But I checked the implementation of the ETags from 
DefaultServlet and it uses the Last-Modified value in there anyway. So I think 
the Last-Modified value (which seems to be set to JAR creation time) is what we 
want here outside of a custom content-addressable strong E-Tag implementation.


- David


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


On Oct. 20, 2017, 4:46 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63176/
> ---
> 
> (Updated Oct. 20, 2017, 4:46 a.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The Scheduler currently returns last-modified headers for static assets, the 
> idea is that the client (the browser) will send HTTP GET requests with an 
> If-Modified-Since header and the Jetty Servlet will intercept them and return 
> 304s, saving everyone some download time. 
> 
> But without a max-age or Expires header set, the browsers are just caching 
> the assets indefinitely - causing real pains when you want to propagate UI 
> changes to users. This sets the max-age of the browser cache to an hour.
> 
> I'll have a follow-up review to disable caching of resources completely when 
> in Vagrant - because that gets annoying when developing the UI.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
> 
> 
> Diff: https://reviews.apache.org/r/63176/diff/1/
> 
> 
> Testing
> ---
> 
> I verified the caching behavior in Chrome with my local Vagrant.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63176: Add Cache-Control header to static assets, to allow for cache expiration

[Bill Farner]

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



I don't feel strongly, but `DefaultServlet` can also send an etag header by 
instead setting `"etags", "true"`.

- Bill Farner


On Oct. 19, 2017, 9:46 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63176/
> ---
> 
> (Updated Oct. 19, 2017, 9:46 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The Scheduler currently returns last-modified headers for static assets, the 
> idea is that the client (the browser) will send HTTP GET requests with an 
> If-Modified-Since header and the Jetty Servlet will intercept them and return 
> 304s, saving everyone some download time. 
> 
> But without a max-age or Expires header set, the browsers are just caching 
> the assets indefinitely - causing real pains when you want to propagate UI 
> changes to users. This sets the max-age of the browser cache to an hour.
> 
> I'll have a follow-up review to disable caching of resources completely when 
> in Vagrant - because that gets annoying when developing the UI.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
> 
> 
> Diff: https://reviews.apache.org/r/63176/diff/1/
> 
> 
> Testing
> ---
> 
> I verified the caching behavior in Chrome with my local Vagrant.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63176: Add Cache-Control header to static assets, to allow for cache expiration

[Aurora ReviewBot]

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



Master (a935389) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 20, 2017, 4:46 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63176/
> ---
> 
> (Updated Oct. 20, 2017, 4:46 a.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The Scheduler currently returns last-modified headers for static assets, the 
> idea is that the client (the browser) will send HTTP GET requests with an 
> If-Modified-Since header and the Jetty Servlet will intercept them and return 
> 304s, saving everyone some download time. 
> 
> But without a max-age or Expires header set, the browsers are just caching 
> the assets indefinitely - causing real pains when you want to propagate UI 
> changes to users. This sets the max-age of the browser cache to an hour.
> 
> I'll have a follow-up review to disable caching of resources completely when 
> in Vagrant - because that gets annoying when developing the UI.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
> 
> 
> Diff: https://reviews.apache.org/r/63176/diff/1/
> 
> 
> Testing
> ---
> 
> I verified the caching behavior in Chrome with my local Vagrant.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Review Request 63176: Add Cache-Control header to static assets, to allow for cache expiration

[David McLaughlin]

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

Review request for Aurora, Jordan Ly and Santhosh Kumar Shanmugham.


Repository: aurora


Description
---

The Scheduler currently returns last-modified headers for static assets, the 
idea is that the client (the browser) will send HTTP GET requests with an 
If-Modified-Since header and the Jetty Servlet will intercept them and return 
304s, saving everyone some download time. 

But without a max-age or Expires header set, the browsers are just caching the 
assets indefinitely - causing real pains when you want to propagate UI changes 
to users. This sets the max-age of the browser cache to an hour.

I'll have a follow-up review to disable caching of resources completely when in 
Vagrant - because that gets annoying when developing the UI.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
93cd20adc28ed700719e472bb2331137a93d1d9d 


Diff: https://reviews.apache.org/r/63176/diff/1/


Testing
---

I verified the caching behavior in Chrome with my local Vagrant.


Thanks,

David McLaughlin

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Aurora ReviewBot]

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



Master (a935389) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 18, 2017, 11:41 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 11:41 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1ec6d740013575bb016645ea0863a7cd4c272838 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 5a9099bf9dd292249d72bc3a7604fbb3394f30ea 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 254c9b7a3fa0b8c3fb47a351748baf9b77f2f2e4 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> df37fb764b3efb7ef97c1a63bdef638c06aea61c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
> 

Re: Review Request 63157: Provide a formal way to disable offer declining

[Aurora ReviewBot]

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


Ship it!




Master (5397013) 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 Oct. 19, 2017, 3:48 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63157/
> ---
> 
> (Updated Oct. 19, 2017, 3:48 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Increasing the offer hold time to effectively disable offer declines is a 
> trap, as the queue of asynchronous declines will grow without bound.  This 
> introduces a command line argument to explicitly disable declining.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/main/java/org/apache/aurora/scheduler/offers/Deferment.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
> 4c6fd546a450917b7542329b020f42ef6379f3b7 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 8f4f63c73c2c2133c7beaf22e1abccfd966f542c 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 815a7e8200ad7b25080556bddd54d407a74678cc 
> 
> 
> Diff: https://reviews.apache.org/r/63157/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63062: Expose list of neighbors in the instance page

[Aurora ReviewBot]

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


Ship it!




Master (5397013) 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 Oct. 19, 2017, 10:52 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63062/
> ---
> 
> (Updated Oct. 19, 2017, 10:52 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kai Huang, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Expose list of neighbors in the instance page
> 
> Exposing the list of neighbor task on the host is useful when application 
> tail letencies degrade because of noisy neighbors. Currenlty our service 
> oweners need to jump to the thermos-UI to get this info. This feature will 
> definately help spotting misbehaving neighbors a lot easier.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskNeighbors.js PRE-CREATION 
>   ui/src/main/js/components/TaskStatus.js 
> b5149182c80b0edcdec88d5c6e24d3015409b0fc 
>   ui/src/main/js/components/__tests__/TaskNeighbors-test.js PRE-CREATION 
>   ui/src/main/js/pages/Instance.js c4d625c063b6037ae2ff93e638a3040436515cd5 
>   ui/src/main/js/utils/Common.js 8f2da7c8a5b44d2ded89a2070ea6ac4bf1fb6ed5 
>   ui/src/main/sass/components/_instance-page.scss 
> 99204fdfca4441d824c3dfff083f78e1d094b4c9 
> 
> 
> Diff: https://reviews.apache.org/r/63062/diff/4/
> 
> 
> Testing
> ---
> 
> ? ./gradlew ui:test
> ? ./gradlew ui:lint
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-16 at 6.27.44 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/7fa71abf-f83e-412f-b554-dc880ccdb25c__Screen_Shot_2017-10-16_at_6.27.44_PM.png
> Neighbor list spanning over the entire page.
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/19/38b2d466-991a-4b71-aed7-8b0bada9e678__Screen_Shot_2017-10-19_at_3.48.08_PM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63157: Provide a formal way to disable offer declining

[Jordan Ly]

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


Ship it!




Ship It!

- Jordan Ly


On Oct. 19, 2017, 10:48 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63157/
> ---
> 
> (Updated Oct. 19, 2017, 10:48 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Increasing the offer hold time to effectively disable offer declines is a 
> trap, as the queue of asynchronous declines will grow without bound.  This 
> introduces a command line argument to explicitly disable declining.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/main/java/org/apache/aurora/scheduler/offers/Deferment.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
> 4c6fd546a450917b7542329b020f42ef6379f3b7 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 8f4f63c73c2c2133c7beaf22e1abccfd966f542c 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 815a7e8200ad7b25080556bddd54d407a74678cc 
> 
> 
> Diff: https://reviews.apache.org/r/63157/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63062: Expose list of neighbors in the instance page

[Reza Motamedi]

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

(Updated Oct. 19, 2017, 10:52 p.m.)


Review request for Aurora, David McLaughlin, Kai Huang, and Santhosh Kumar 
Shanmugham.


Repository: aurora


Description
---

# Expose list of neighbors in the instance page

Exposing the list of neighbor task on the host is useful when application tail 
letencies degrade because of noisy neighbors. Currenlty our service oweners 
need to jump to the thermos-UI to get this info. This feature will definately 
help spotting misbehaving neighbors a lot easier.


Diffs
-

  ui/src/main/js/components/TaskNeighbors.js PRE-CREATION 
  ui/src/main/js/components/TaskStatus.js 
b5149182c80b0edcdec88d5c6e24d3015409b0fc 
  ui/src/main/js/components/__tests__/TaskNeighbors-test.js PRE-CREATION 
  ui/src/main/js/pages/Instance.js c4d625c063b6037ae2ff93e638a3040436515cd5 
  ui/src/main/js/utils/Common.js 8f2da7c8a5b44d2ded89a2070ea6ac4bf1fb6ed5 
  ui/src/main/sass/components/_instance-page.scss 
99204fdfca4441d824c3dfff083f78e1d094b4c9 


Diff: https://reviews.apache.org/r/63062/diff/4/


Testing
---

? ./gradlew ui:test
? ./gradlew ui:lint


File Attachments (updated)


Screen Shot 2017-10-16 at 6.27.44 PM.png
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/7fa71abf-f83e-412f-b554-dc880ccdb25c__Screen_Shot_2017-10-16_at_6.27.44_PM.png
Neighbor list spanning over the entire page.
  
https://reviews.apache.org/media/uploaded/files/2017/10/19/38b2d466-991a-4b71-aed7-8b0bada9e678__Screen_Shot_2017-10-19_at_3.48.08_PM.png


Thanks,

Reza Motamedi

Re: Review Request 63062: Expose list of neighbors in the instance page

[Reza Motamedi]

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

(Updated Oct. 19, 2017, 10:51 p.m.)


Review request for Aurora, David McLaughlin, Kai Huang, and Santhosh Kumar 
Shanmugham.


Changes
---

- Address review feedback.
- Make neighbor list span over the entire page to deal with wrapping neighbor 
task-keys.


Repository: aurora


Description
---

# Expose list of neighbors in the instance page

Exposing the list of neighbor task on the host is useful when application tail 
letencies degrade because of noisy neighbors. Currenlty our service oweners 
need to jump to the thermos-UI to get this info. This feature will definately 
help spotting misbehaving neighbors a lot easier.


Diffs
-

  ui/src/main/js/components/TaskNeighbors.js PRE-CREATION 
  ui/src/main/js/components/TaskStatus.js 
b5149182c80b0edcdec88d5c6e24d3015409b0fc 
  ui/src/main/js/components/__tests__/TaskNeighbors-test.js PRE-CREATION 
  ui/src/main/js/pages/Instance.js c4d625c063b6037ae2ff93e638a3040436515cd5 
  ui/src/main/js/utils/Common.js 8f2da7c8a5b44d2ded89a2070ea6ac4bf1fb6ed5 
  ui/src/main/sass/components/_instance-page.scss 
99204fdfca4441d824c3dfff083f78e1d094b4c9 


Diff: https://reviews.apache.org/r/63062/diff/4/


Testing
---

? ./gradlew ui:test
? ./gradlew ui:lint


File Attachments (updated)


Screen Shot 2017-10-16 at 6.27.44 PM.png
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/7fa71abf-f83e-412f-b554-dc880ccdb25c__Screen_Shot_2017-10-16_at_6.27.44_PM.png
Screen Shot 2017-10-19 at 3.48.08 PM.png
  
https://reviews.apache.org/media/uploaded/files/2017/10/19/3a0f43dd-de7d-4aff-8668-b6ef36f52009__Screen_Shot_2017-10-19_at_3.48.08_PM.png
Neighbor list spanning over the entire page.
  
https://reviews.apache.org/media/uploaded/files/2017/10/19/38b2d466-991a-4b71-aed7-8b0bada9e678__Screen_Shot_2017-10-19_at_3.48.08_PM.png


Thanks,

Reza Motamedi

Re: Review Request 63062: Expose list of neighbors in the instance page

[David McLaughlin]

> On Oct. 17, 2017, 7:53 p.m., David McLaughlin wrote:
> > File Attachment: Screen Shot 2017-10-16 at 6.27.44 PM.png - Screen Shot 
> > 2017-10-16 at 6.27.44 PM.png
> > 
> >
> > I'd also try and find a task with a longer job key and see what it does 
> > to the layout. It might make more sense to have this widget as a row 
> > spanning the whole width.
> 
> Reza Motamedi wrote:
> This happens when task-keys are super long. I made the `div` span the the 
> entire column. However what I think would be better is if we push the state 
> machine a few cells to the right. 
> I have seen examples where the TaskId is so long that that it wraps over 
> one line. My suggestion should help with that as well.

The state machine can have messages that also span multiple lines. A 50/50 
split is good, I think.


- David


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


On Oct. 19, 2017, 10:37 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63062/
> ---
> 
> (Updated Oct. 19, 2017, 10:37 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kai Huang, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Expose list of neighbors in the instance page
> 
> Exposing the list of neighbor task on the host is useful when application 
> tail letencies degrade because of noisy neighbors. Currenlty our service 
> oweners need to jump to the thermos-UI to get this info. This feature will 
> definately help spotting misbehaving neighbors a lot easier.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskNeighbors.js PRE-CREATION 
>   ui/src/main/js/components/TaskStatus.js 
> b5149182c80b0edcdec88d5c6e24d3015409b0fc 
>   ui/src/main/js/components/__tests__/TaskNeighbors-test.js PRE-CREATION 
>   ui/src/main/js/pages/Instance.js c4d625c063b6037ae2ff93e638a3040436515cd5 
>   ui/src/main/js/utils/Common.js 8f2da7c8a5b44d2ded89a2070ea6ac4bf1fb6ed5 
>   ui/src/main/sass/components/_instance-page.scss 
> 99204fdfca4441d824c3dfff083f78e1d094b4c9 
> 
> 
> Diff: https://reviews.apache.org/r/63062/diff/4/
> 
> 
> Testing
> ---
> 
> ? ./gradlew ui:test
> ? ./gradlew ui:lint
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-16 at 6.27.44 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/7fa71abf-f83e-412f-b554-dc880ccdb25c__Screen_Shot_2017-10-16_at_6.27.44_PM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63157: Provide a formal way to disable offer declining

[Bill Farner]

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

(Updated Oct. 19, 2017, 3:48 p.m.)


Review request for Aurora and Jordan Ly.


Repository: aurora


Description
---

Increasing the offer hold time to effectively disable offer declines is a trap, 
as the queue of asynchronous declines will grow without bound.  This introduces 
a command line argument to explicitly disable declining.


Diffs (updated)
-

  RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
7d3766829161457b1b3ba50bce128047d10b2c58 
  src/main/java/org/apache/aurora/scheduler/offers/Deferment.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
e8334310a2a46a0ccb09ee6e4122c515892d3996 
  src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
4c6fd546a450917b7542329b020f42ef6379f3b7 
  src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 
  src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
8f4f63c73c2c2133c7beaf22e1abccfd966f542c 
  src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
815a7e8200ad7b25080556bddd54d407a74678cc 


Diff: https://reviews.apache.org/r/63157/diff/3/

Changes: https://reviews.apache.org/r/63157/diff/2-3/


Testing
---


Thanks,

Bill Farner

Re: Review Request 63157: Provide a formal way to disable offer declining

[Bill Farner]

> On Oct. 19, 2017, 3:25 p.m., Jordan Ly wrote:
> > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
> > Line 329 (original), 319-324 (patched)
> > 
> >
> > Feels like a lot of stuff going on here.
> > 
> > I think it might be cleaner if the caller is forced to call `remove` on 
> > the offer from the same agent. Additionally, maybe name this 
> > `addIfNotPresent` and a small comment stating that this method will return 
> > a `HostOffer` if we already have one from the particular agent.

> Feels like a lot of stuff going on here.

For a function called `add()`, i agree.  A rename and comment sounds warranted.

> I think it might be cleaner if the caller is forced to call remove on the 
> offer from the same agent

I'm not a fan of this.  It's much easier to reason about consistency during 
concurrent access if this is all performed while holding a lock.


- Bill


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


On Oct. 19, 2017, 1:23 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63157/
> ---
> 
> (Updated Oct. 19, 2017, 1:23 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Increasing the offer hold time to effectively disable offer declines is a 
> trap, as the queue of asynchronous declines will grow without bound.  This 
> introduces a command line argument to explicitly disable declining.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/main/java/org/apache/aurora/scheduler/offers/Deferment.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
> 4c6fd546a450917b7542329b020f42ef6379f3b7 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 8f4f63c73c2c2133c7beaf22e1abccfd966f542c 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 815a7e8200ad7b25080556bddd54d407a74678cc 
> 
> 
> Diff: https://reviews.apache.org/r/63157/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63157: Provide a formal way to disable offer declining

[Aurora ReviewBot]

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


Ship it!




Master (5397013) 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 Oct. 19, 2017, 8:23 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63157/
> ---
> 
> (Updated Oct. 19, 2017, 8:23 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Increasing the offer hold time to effectively disable offer declines is a 
> trap, as the queue of asynchronous declines will grow without bound.  This 
> introduces a command line argument to explicitly disable declining.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/main/java/org/apache/aurora/scheduler/offers/Deferment.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
> 4c6fd546a450917b7542329b020f42ef6379f3b7 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 8f4f63c73c2c2133c7beaf22e1abccfd966f542c 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 815a7e8200ad7b25080556bddd54d407a74678cc 
> 
> 
> Diff: https://reviews.apache.org/r/63157/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63062: Expose list of neighbors in the instance page

[Reza Motamedi]

> On Oct. 17, 2017, 7:50 p.m., Santhosh Kumar Shanmugham wrote:
> > File Attachment: Screen Shot 2017-10-16 at 6.27.44 PM.png - Screen Shot 
> > 2017-10-16 at 6.27.44 PM.png
> > 
> >
> > Wonder if we need to paginate this? Think the most neighbors I have 
> > seen is ~20.

Mybe we can add that later if we start seeing that is distracting. I don't 
think in reality this is going to be very long.


> On Oct. 17, 2017, 7:50 p.m., Santhosh Kumar Shanmugham wrote:
> > ui/src/main/js/components/TaskNeighbors.js
> > Lines 21 (patched)
> > 
> >
> > `isNullyOrEmpty` already checks if the collection is empty? In which 
> > case `tasks.length === 0` is redudant.

Good catch! thanks :)


- Reza


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


On Oct. 19, 2017, 10:37 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63062/
> ---
> 
> (Updated Oct. 19, 2017, 10:37 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kai Huang, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Expose list of neighbors in the instance page
> 
> Exposing the list of neighbor task on the host is useful when application 
> tail letencies degrade because of noisy neighbors. Currenlty our service 
> oweners need to jump to the thermos-UI to get this info. This feature will 
> definately help spotting misbehaving neighbors a lot easier.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskNeighbors.js PRE-CREATION 
>   ui/src/main/js/components/TaskStatus.js 
> b5149182c80b0edcdec88d5c6e24d3015409b0fc 
>   ui/src/main/js/components/__tests__/TaskNeighbors-test.js PRE-CREATION 
>   ui/src/main/js/pages/Instance.js c4d625c063b6037ae2ff93e638a3040436515cd5 
>   ui/src/main/js/utils/Common.js 8f2da7c8a5b44d2ded89a2070ea6ac4bf1fb6ed5 
>   ui/src/main/sass/components/_instance-page.scss 
> 99204fdfca4441d824c3dfff083f78e1d094b4c9 
> 
> 
> Diff: https://reviews.apache.org/r/63062/diff/4/
> 
> 
> Testing
> ---
> 
> ? ./gradlew ui:test
> ? ./gradlew ui:lint
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-16 at 6.27.44 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/7fa71abf-f83e-412f-b554-dc880ccdb25c__Screen_Shot_2017-10-16_at_6.27.44_PM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63062: Expose list of neighbors in the instance page

[Reza Motamedi]

> On Oct. 17, 2017, 7:53 p.m., David McLaughlin wrote:
> > File Attachment: Screen Shot 2017-10-16 at 6.27.44 PM.png - Screen Shot 
> > 2017-10-16 at 6.27.44 PM.png
> > 
> >
> > I'd also try and find a task with a longer job key and see what it does 
> > to the layout. It might make more sense to have this widget as a row 
> > spanning the whole width.

This happens when task-keys are super long. I made the `div` span the the 
entire column. However what I think would be better is if we push the state 
machine a few cells to the right. 
I have seen examples where the TaskId is so long that that it wraps over one 
line. My suggestion should help with that as well.


- Reza


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


On Oct. 19, 2017, 10:37 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63062/
> ---
> 
> (Updated Oct. 19, 2017, 10:37 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kai Huang, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Expose list of neighbors in the instance page
> 
> Exposing the list of neighbor task on the host is useful when application 
> tail letencies degrade because of noisy neighbors. Currenlty our service 
> oweners need to jump to the thermos-UI to get this info. This feature will 
> definately help spotting misbehaving neighbors a lot easier.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskNeighbors.js PRE-CREATION 
>   ui/src/main/js/components/TaskStatus.js 
> b5149182c80b0edcdec88d5c6e24d3015409b0fc 
>   ui/src/main/js/components/__tests__/TaskNeighbors-test.js PRE-CREATION 
>   ui/src/main/js/pages/Instance.js c4d625c063b6037ae2ff93e638a3040436515cd5 
>   ui/src/main/js/utils/Common.js 8f2da7c8a5b44d2ded89a2070ea6ac4bf1fb6ed5 
>   ui/src/main/sass/components/_instance-page.scss 
> 99204fdfca4441d824c3dfff083f78e1d094b4c9 
> 
> 
> Diff: https://reviews.apache.org/r/63062/diff/4/
> 
> 
> Testing
> ---
> 
> ? ./gradlew ui:test
> ? ./gradlew ui:lint
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-16 at 6.27.44 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/7fa71abf-f83e-412f-b554-dc880ccdb25c__Screen_Shot_2017-10-16_at_6.27.44_PM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63062: Expose list of neighbors in the instance page

[Reza Motamedi]

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

(Updated Oct. 19, 2017, 10:37 p.m.)


Review request for Aurora, David McLaughlin, Kai Huang, and Santhosh Kumar 
Shanmugham.


Repository: aurora


Description
---

# Expose list of neighbors in the instance page

Exposing the list of neighbor task on the host is useful when application tail 
letencies degrade because of noisy neighbors. Currenlty our service oweners 
need to jump to the thermos-UI to get this info. This feature will definately 
help spotting misbehaving neighbors a lot easier.


Diffs (updated)
-

  ui/src/main/js/components/TaskNeighbors.js PRE-CREATION 
  ui/src/main/js/components/TaskStatus.js 
b5149182c80b0edcdec88d5c6e24d3015409b0fc 
  ui/src/main/js/components/__tests__/TaskNeighbors-test.js PRE-CREATION 
  ui/src/main/js/pages/Instance.js c4d625c063b6037ae2ff93e638a3040436515cd5 
  ui/src/main/js/utils/Common.js 8f2da7c8a5b44d2ded89a2070ea6ac4bf1fb6ed5 
  ui/src/main/sass/components/_instance-page.scss 
99204fdfca4441d824c3dfff083f78e1d094b4c9 


Diff: https://reviews.apache.org/r/63062/diff/4/

Changes: https://reviews.apache.org/r/63062/diff/3-4/


Testing
---

? ./gradlew ui:test
? ./gradlew ui:lint


File Attachments


Screen Shot 2017-10-16 at 6.27.44 PM.png
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/7fa71abf-f83e-412f-b554-dc880ccdb25c__Screen_Shot_2017-10-16_at_6.27.44_PM.png


Thanks,

Reza Motamedi

Re: Review Request 63157: Provide a formal way to disable offer declining

[Jordan Ly]

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




src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
Line 329 (original), 319-324 (patched)


Feels like a lot of stuff going on here.

I think it might be cleaner if the caller is forced to call `remove` on the 
offer from the same agent. Additionally, maybe name this `addIfNotPresent` and 
a small comment stating that this method will return a `HostOffer` if we 
already have one from the particular agent.


- Jordan Ly


On Oct. 19, 2017, 8:23 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63157/
> ---
> 
> (Updated Oct. 19, 2017, 8:23 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Increasing the offer hold time to effectively disable offer declines is a 
> trap, as the queue of asynchronous declines will grow without bound.  This 
> introduces a command line argument to explicitly disable declining.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/main/java/org/apache/aurora/scheduler/offers/Deferment.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
> 4c6fd546a450917b7542329b020f42ef6379f3b7 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 8f4f63c73c2c2133c7beaf22e1abccfd966f542c 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 815a7e8200ad7b25080556bddd54d407a74678cc 
> 
> 
> Diff: https://reviews.apache.org/r/63157/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63157: Provide a formal way to disable offer declining

[Bill Farner]

> On Oct. 19, 2017, 1:50 p.m., Jordan Ly wrote:
> > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
> > Lines 188-190 (original), 186-188 (patched)
> > 
> >
> > Actually one issue: we no longer have this guarantee if we never remove 
> > and decline offers.
> > 
> > We need a mechanism instead within addOffer to check if we already have 
> > an offer from a given agent and if so decline both.

Done - moved this into `HostOffers`, eliminating the check-then-act.


- Bill


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


On Oct. 19, 2017, 1:23 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63157/
> ---
> 
> (Updated Oct. 19, 2017, 1:23 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Increasing the offer hold time to effectively disable offer declines is a 
> trap, as the queue of asynchronous declines will grow without bound.  This 
> introduces a command line argument to explicitly disable declining.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferDecline.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
> 4c6fd546a450917b7542329b020f42ef6379f3b7 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 8f4f63c73c2c2133c7beaf22e1abccfd966f542c 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 815a7e8200ad7b25080556bddd54d407a74678cc 
> 
> 
> Diff: https://reviews.apache.org/r/63157/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63157: Provide a formal way to disable offer declining

[Bill Farner]

> On Oct. 19, 2017, 1:32 p.m., Jordan Ly wrote:
> > src/main/java/org/apache/aurora/scheduler/offers/OfferDecline.java
> > Lines 28 (patched)
> > 
> >
> > nit: `OfferDecline` seems a bit ambiguious to me at first glance. I 
> > would name this something like `OfferDecliner` or `OfferDeclineHandler`.

Went a more generic route and called it `Deferment`.  Let me know if that still 
doesn't sit well.


- Bill


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


On Oct. 19, 2017, 1:23 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63157/
> ---
> 
> (Updated Oct. 19, 2017, 1:23 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Increasing the offer hold time to effectively disable offer declines is a 
> trap, as the queue of asynchronous declines will grow without bound.  This 
> introduces a command line argument to explicitly disable declining.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferDecline.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
> 4c6fd546a450917b7542329b020f42ef6379f3b7 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 8f4f63c73c2c2133c7beaf22e1abccfd966f542c 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 815a7e8200ad7b25080556bddd54d407a74678cc 
> 
> 
> Diff: https://reviews.apache.org/r/63157/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Aurora ReviewBot]

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



This patch does not apply cleanly against master (b9ede2f), do you need to 
rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 18, 2017, 11:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 11:11 a.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 254c9b7a3fa0b8c3fb47a351748baf9b77f2f2e4 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> df37fb764b3efb7ef97c1a63bdef638c06aea61c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   src/main/java/org/apache/aurora/scheduler/stora

Re: Review Request 63169: Add an example of using the UI plugin mechanism

[Aurora ReviewBot]

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


Ship it!




Master (b9ede2f) 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 Oct. 19, 2017, 9:23 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63169/
> ---
> 
> (Updated Oct. 19, 2017, 9:23 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an example of using the UI plugin mechanism
> 
> 
> Diffs
> -
> 
>   build.gradle fa3ed0d88bf6491007eabb2c44dfe78317fd87cf 
>   ui/package.json 38950567f1c3259f62bb53352477fb2d429c3276 
>   ui/plugin/js/components/PluginExample.js PRE-CREATION 
>   ui/plugin/js/components/plugin.css PRE-CREATION 
>   ui/src/main/js/components/Home.js 619440f46375eb8e3a32eedcda6f74f3fc58f288 
>   ui/src/main/js/components/PluginExample.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Home-test.js 
> dde72fc2bad7d7db314191402ab39341ad78cdab 
> 
> 
> Diff: https://reviews.apache.org/r/63169/diff/2/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63169: Add an example of using the UI plugin mechanism

[David McLaughlin]

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

(Updated Oct. 19, 2017, 9:23 p.m.)


Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.


Changes
---

Home.js -> PluginExample.js


Repository: aurora


Description
---

Add an example of using the UI plugin mechanism


Diffs (updated)
-

  build.gradle fa3ed0d88bf6491007eabb2c44dfe78317fd87cf 
  ui/package.json 38950567f1c3259f62bb53352477fb2d429c3276 
  ui/plugin/js/components/PluginExample.js PRE-CREATION 
  ui/plugin/js/components/plugin.css PRE-CREATION 
  ui/src/main/js/components/Home.js 619440f46375eb8e3a32eedcda6f74f3fc58f288 
  ui/src/main/js/components/PluginExample.js PRE-CREATION 
  ui/src/main/js/components/__tests__/Home-test.js 
dde72fc2bad7d7db314191402ab39341ad78cdab 


Diff: https://reviews.apache.org/r/63169/diff/2/

Changes: https://reviews.apache.org/r/63169/diff/1-2/


Testing
---

./gradlew ui:lint
./gradlew ui:test


Thanks,

David McLaughlin

Re: Review Request 63135: Refactor Job Page to make it more pluggable

[Aurora ReviewBot]

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


Ship it!




Master (b9ede2f) 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 Oct. 19, 2017, 9:06 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63135/
> ---
> 
> (Updated Oct. 19, 2017, 9:06 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Refactor Job Page to create cleaner abstractions. Also refactors Tabs to make 
> them easier to write tests for (using shallow from enzyme). 
> 
> Also improved Pagination code (was generating a warning) now that React can 
> handle returning null from components (and doing the right thing).
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/JobHistory.js PRE-CREATION 
>   ui/src/main/js/components/JobOverview.js PRE-CREATION 
>   ui/src/main/js/components/JobStatus.js PRE-CREATION 
>   ui/src/main/js/components/Pagination.js 
> 094ef2b248bd802ec63f562a978fd1b4d4bb5eb8 
>   ui/src/main/js/components/Tabs.js e382aa74e0f73e78c8092a58a605bffc54d15a81 
>   ui/src/main/js/components/__tests__/JobHistory-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/JobStatus-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Tabs-test.js 
> 252e5e001478b5a727e208b6b2d09d10128e83b9 
>   ui/src/main/js/pages/Job.js 070f1e43e4fe9c9eac688a9514c14e3296ef41af 
>   ui/src/main/js/pages/__tests__/Job-test.js 
> 03ef20ddefcc915ed4bd6242de3fe8686efdc202 
> 
> 
> Diff: https://reviews.apache.org/r/63135/diff/2/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63169: Add an example of using the UI plugin mechanism

[Santhosh Kumar Shanmugham]

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


Ship it!




Probably rename the `plugin/Home.js` as `plugin/Example.js`.

- Santhosh Kumar Shanmugham


On Oct. 19, 2017, 2:05 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63169/
> ---
> 
> (Updated Oct. 19, 2017, 2:05 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an example of using the UI plugin mechanism
> 
> 
> Diffs
> -
> 
>   build.gradle fa3ed0d88bf6491007eabb2c44dfe78317fd87cf 
>   ui/package.json 38950567f1c3259f62bb53352477fb2d429c3276 
>   ui/plugin/js/components/Home.js PRE-CREATION 
>   ui/plugin/js/components/home.css PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63169/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63135: Refactor Job Page to make it more pluggable

[David McLaughlin]

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

(Updated Oct. 19, 2017, 9:06 p.m.)


Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.


Repository: aurora


Description
---

Refactor Job Page to create cleaner abstractions. Also refactors Tabs to make 
them easier to write tests for (using shallow from enzyme). 

Also improved Pagination code (was generating a warning) now that React can 
handle returning null from components (and doing the right thing).


Diffs (updated)
-

  ui/src/main/js/components/JobHistory.js PRE-CREATION 
  ui/src/main/js/components/JobOverview.js PRE-CREATION 
  ui/src/main/js/components/JobStatus.js PRE-CREATION 
  ui/src/main/js/components/Pagination.js 
094ef2b248bd802ec63f562a978fd1b4d4bb5eb8 
  ui/src/main/js/components/Tabs.js e382aa74e0f73e78c8092a58a605bffc54d15a81 
  ui/src/main/js/components/__tests__/JobHistory-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/JobStatus-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/Tabs-test.js 
252e5e001478b5a727e208b6b2d09d10128e83b9 
  ui/src/main/js/pages/Job.js 070f1e43e4fe9c9eac688a9514c14e3296ef41af 
  ui/src/main/js/pages/__tests__/Job-test.js 
03ef20ddefcc915ed4bd6242de3fe8686efdc202 


Diff: https://reviews.apache.org/r/63135/diff/2/

Changes: https://reviews.apache.org/r/63135/diff/1-2/


Testing
---

./gradlew ui:lint
./gradlew ui:test


Thanks,

David McLaughlin

Review Request 63169: Add an example of using the UI plugin mechanism

[David McLaughlin]

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

Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.


Repository: aurora


Description
---

Add an example of using the UI plugin mechanism


Diffs
-

  build.gradle fa3ed0d88bf6491007eabb2c44dfe78317fd87cf 
  ui/package.json 38950567f1c3259f62bb53352477fb2d429c3276 
  ui/plugin/js/components/Home.js PRE-CREATION 
  ui/plugin/js/components/home.css PRE-CREATION 


Diff: https://reviews.apache.org/r/63169/diff/1/


Testing
---

./gradlew ui:lint
./gradlew ui:test


Thanks,

David McLaughlin

Re: Review Request 63135: Refactor Job Page to make it more pluggable

[Aurora ReviewBot]

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



Master (b9ede2f) is red with this patch.
  ./build-support/jenkins/build.sh

✖ 1 problem (1 error, 0 warnings)


npm ERR! Linux 4.4.0-89-generic
npm ERR! argv 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/ui/.gradle/nodejs/node-v6.9.1-linux-x64/bin/node"
 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/ui/.gradle/nodejs/node-v6.9.1-linux-x64/bin/npm"
 "run" "lint"
npm ERR! node v6.9.1
npm ERR! npm  v3.10.8
npm ERR! code ELIFECYCLE
npm ERR! apache-aurora@1.0.0 lint: `eslint src/main/js --ext .js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the apache-aurora@1.0.0 lint script 'eslint src/main/js 
--ext .js'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the apache-aurora 
package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR! eslint src/main/js --ext .js
npm ERR! You can get information on how to open an issue for this project with:
npm ERR! npm bugs apache-aurora
npm ERR! Or if that isn't available, you can get their info via:
npm ERR! npm owner ls apache-aurora
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR! /home/jenkins/jenkins-slave/workspace/AuroraBot/ui/npm-debug.log
:ui:lint FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':ui:lint'.
> Process 'command 
> '/home/jenkins/jenkins-slave/workspace/AuroraBot/ui/.gradle/nodejs/node-v6.9.1-linux-x64/bin/npm''
>  finished with non-zero exit value 1

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 5m 23s
58 actionable tasks: 47 executed, 11 up-to-date


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 19, 2017, 4 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63135/
> ---
> 
> (Updated Oct. 19, 2017, 4 a.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Refactor Job Page to create cleaner abstractions. Also refactors Tabs to make 
> them easier to write tests for (using shallow from enzyme). 
> 
> Also improved Pagination code (was generating a warning) now that React can 
> handle returning null from components (and doing the right thing).
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/JobHistory.js PRE-CREATION 
>   ui/src/main/js/components/JobOverview.js PRE-CREATION 
>   ui/src/main/js/components/JobStatus.js PRE-CREATION 
>   ui/src/main/js/components/Pagination.js 
> 094ef2b248bd802ec63f562a978fd1b4d4bb5eb8 
>   ui/src/main/js/components/Tabs.js e382aa74e0f73e78c8092a58a605bffc54d15a81 
>   ui/src/main/js/components/__tests__/JobHistory-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/JobStatus-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Tabs-test.js 
> 252e5e001478b5a727e208b6b2d09d10128e83b9 
>   ui/src/main/js/pages/Job.js 070f1e43e4fe9c9eac688a9514c14e3296ef41af 
>   ui/src/main/js/pages/__tests__/Job-test.js 
> 03ef20ddefcc915ed4bd6242de3fe8686efdc202 
> 
> 
> Diff: https://reviews.apache.org/r/63135/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63165: Add banner pointing to new UI

[Aurora ReviewBot]

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



Master (a9827fe) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 19, 2017, 8:18 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63165/
> ---
> 
> (Updated Oct. 19, 2017, 8:18 p.m.)
> 
> 
> Review request for Aurora, Santhosh Kumar Shanmugham and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We'd like to direct all of our users to the new UI in order to do a thorough 
> beta test. It's a little ham-handed but the idea is this would be removed and 
> the old UI completely replaced by the next official Scheduler release. So 
> only those brave souls deploying from master would see this.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/css/app.css 
> 67a0ac5b3fa6372867e6bc2d963a3867dda6748b 
>   src/main/resources/scheduler/assets/scheduler/index.html 
> 98e465a46c006bcfea765a4530fc5d6d67bd1539 
> 
> 
> Diff: https://reviews.apache.org/r/63165/diff/1/
> 
> 
> Testing
> ---
> 
> See screenshot.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-19 at 1.16.35 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/19/9dd8eccb-c983-427a-a41e-d236f1f3b2a5__Screen_Shot_2017-10-19_at_1.16.35_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63135: Refactor Job Page to make it more pluggable

[David McLaughlin]

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



@ReviewBot retry

- David McLaughlin


On Oct. 19, 2017, 4 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63135/
> ---
> 
> (Updated Oct. 19, 2017, 4 a.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Refactor Job Page to create cleaner abstractions. Also refactors Tabs to make 
> them easier to write tests for (using shallow from enzyme). 
> 
> Also improved Pagination code (was generating a warning) now that React can 
> handle returning null from components (and doing the right thing).
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/JobHistory.js PRE-CREATION 
>   ui/src/main/js/components/JobOverview.js PRE-CREATION 
>   ui/src/main/js/components/JobStatus.js PRE-CREATION 
>   ui/src/main/js/components/Pagination.js 
> 094ef2b248bd802ec63f562a978fd1b4d4bb5eb8 
>   ui/src/main/js/components/Tabs.js e382aa74e0f73e78c8092a58a605bffc54d15a81 
>   ui/src/main/js/components/__tests__/JobHistory-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/JobStatus-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Tabs-test.js 
> 252e5e001478b5a727e208b6b2d09d10128e83b9 
>   ui/src/main/js/pages/Job.js 070f1e43e4fe9c9eac688a9514c14e3296ef41af 
>   ui/src/main/js/pages/__tests__/Job-test.js 
> 03ef20ddefcc915ed4bd6242de3fe8686efdc202 
> 
> 
> Diff: https://reviews.apache.org/r/63135/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63135: Refactor Job Page to make it more pluggable

[David McLaughlin]

> On Oct. 19, 2017, 8:24 p.m., Kai Huang wrote:
> > Fix the gradle build error:
> > ```
> > * Exception is:
> > org.gradle.api.tasks.TaskExecutionException: Execution failed for task 
> > ':ui:test'.
> > at 
> > org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeActions(ExecuteActionsTaskExecuter.java:100)
> > ```

I cannot recreate this locally. Are you sure this isn't another problem with 
your local environment?


- David


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


On Oct. 19, 2017, 4 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63135/
> ---
> 
> (Updated Oct. 19, 2017, 4 a.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Refactor Job Page to create cleaner abstractions. Also refactors Tabs to make 
> them easier to write tests for (using shallow from enzyme). 
> 
> Also improved Pagination code (was generating a warning) now that React can 
> handle returning null from components (and doing the right thing).
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/JobHistory.js PRE-CREATION 
>   ui/src/main/js/components/JobOverview.js PRE-CREATION 
>   ui/src/main/js/components/JobStatus.js PRE-CREATION 
>   ui/src/main/js/components/Pagination.js 
> 094ef2b248bd802ec63f562a978fd1b4d4bb5eb8 
>   ui/src/main/js/components/Tabs.js e382aa74e0f73e78c8092a58a605bffc54d15a81 
>   ui/src/main/js/components/__tests__/JobHistory-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/JobStatus-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Tabs-test.js 
> 252e5e001478b5a727e208b6b2d09d10128e83b9 
>   ui/src/main/js/pages/Job.js 070f1e43e4fe9c9eac688a9514c14e3296ef41af 
>   ui/src/main/js/pages/__tests__/Job-test.js 
> 03ef20ddefcc915ed4bd6242de3fe8686efdc202 
> 
> 
> Diff: https://reviews.apache.org/r/63135/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63157: Provide a formal way to disable offer declining

[Jordan Ly]

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




src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
Lines 188-190 (original), 186-188 (patched)


Actually one issue: we no longer have this guarantee if we never remove and 
decline offers.

We need a mechanism instead within addOffer to check if we already have an 
offer from a given agent and if so decline both.


- Jordan Ly


On Oct. 19, 2017, 8:23 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63157/
> ---
> 
> (Updated Oct. 19, 2017, 8:23 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Increasing the offer hold time to effectively disable offer declines is a 
> trap, as the queue of asynchronous declines will grow without bound.  This 
> introduces a command line argument to explicitly disable declining.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferDecline.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
> 4c6fd546a450917b7542329b020f42ef6379f3b7 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 8f4f63c73c2c2133c7beaf22e1abccfd966f542c 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 815a7e8200ad7b25080556bddd54d407a74678cc 
> 
> 
> Diff: https://reviews.apache.org/r/63157/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63157: Provide a formal way to disable offer declining

[Aurora ReviewBot]

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


Ship it!




Master (a9827fe) 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 Oct. 19, 2017, 4:23 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63157/
> ---
> 
> (Updated Oct. 19, 2017, 4:23 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Increasing the offer hold time to effectively disable offer declines is a 
> trap, as the queue of asynchronous declines will grow without bound.  This 
> introduces a command line argument to explicitly disable declining.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferDecline.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
> 4c6fd546a450917b7542329b020f42ef6379f3b7 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 8f4f63c73c2c2133c7beaf22e1abccfd966f542c 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 815a7e8200ad7b25080556bddd54d407a74678cc 
> 
> 
> Diff: https://reviews.apache.org/r/63157/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63157: Provide a formal way to disable offer declining

[Jordan Ly]

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


Ship it!




Overall LGTM; small nit on naming but it is non-blocking.


src/main/java/org/apache/aurora/scheduler/offers/OfferDecline.java
Lines 28 (patched)


nit: `OfferDecline` seems a bit ambiguious to me at first glance. I would 
name this something like `OfferDecliner` or `OfferDeclineHandler`.


- Jordan Ly


On Oct. 19, 2017, 8:23 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63157/
> ---
> 
> (Updated Oct. 19, 2017, 8:23 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Increasing the offer hold time to effectively disable offer declines is a 
> trap, as the queue of asynchronous declines will grow without bound.  This 
> introduces a command line argument to explicitly disable declining.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferDecline.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
> 4c6fd546a450917b7542329b020f42ef6379f3b7 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 8f4f63c73c2c2133c7beaf22e1abccfd966f542c 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 815a7e8200ad7b25080556bddd54d407a74678cc 
> 
> 
> Diff: https://reviews.apache.org/r/63157/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63135: Refactor Job Page to make it more pluggable

[Kai Huang]

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



Fix the gradle build error:
```
* Exception is:
org.gradle.api.tasks.TaskExecutionException: Execution failed for task 
':ui:test'.
at 
org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeActions(ExecuteActionsTaskExecuter.java:100)
```

- Kai Huang


On Oct. 19, 2017, 4 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63135/
> ---
> 
> (Updated Oct. 19, 2017, 4 a.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Refactor Job Page to create cleaner abstractions. Also refactors Tabs to make 
> them easier to write tests for (using shallow from enzyme). 
> 
> Also improved Pagination code (was generating a warning) now that React can 
> handle returning null from components (and doing the right thing).
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/JobHistory.js PRE-CREATION 
>   ui/src/main/js/components/JobOverview.js PRE-CREATION 
>   ui/src/main/js/components/JobStatus.js PRE-CREATION 
>   ui/src/main/js/components/Pagination.js 
> 094ef2b248bd802ec63f562a978fd1b4d4bb5eb8 
>   ui/src/main/js/components/Tabs.js e382aa74e0f73e78c8092a58a605bffc54d15a81 
>   ui/src/main/js/components/__tests__/JobHistory-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/JobStatus-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Tabs-test.js 
> 252e5e001478b5a727e208b6b2d09d10128e83b9 
>   ui/src/main/js/pages/Job.js 070f1e43e4fe9c9eac688a9514c14e3296ef41af 
>   ui/src/main/js/pages/__tests__/Job-test.js 
> 03ef20ddefcc915ed4bd6242de3fe8686efdc202 
> 
> 
> Diff: https://reviews.apache.org/r/63135/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Review Request 63157: Provide a formal way to disable offer declining

[Bill Farner]

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

Review request for Aurora and Jordan Ly.


Repository: aurora


Description
---

Increasing the offer hold time to effectively disable offer declines is a trap, 
as the queue of asynchronous declines will grow without bound.  This introduces 
a command line argument to explicitly disable declining.


Diffs
-

  RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
7d3766829161457b1b3ba50bce128047d10b2c58 
  src/main/java/org/apache/aurora/scheduler/offers/OfferDecline.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
e8334310a2a46a0ccb09ee6e4122c515892d3996 
  src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
4c6fd546a450917b7542329b020f42ef6379f3b7 
  src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 
  src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
8f4f63c73c2c2133c7beaf22e1abccfd966f542c 
  src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
815a7e8200ad7b25080556bddd54d407a74678cc 


Diff: https://reviews.apache.org/r/63157/diff/1/


Testing
---


Thanks,

Bill Farner

Re: Review Request 63165: Add banner pointing to new UI

[Santhosh Kumar Shanmugham]

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


Ship it!




Ship It!

- Santhosh Kumar Shanmugham


On Oct. 19, 2017, 1:18 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63165/
> ---
> 
> (Updated Oct. 19, 2017, 1:18 p.m.)
> 
> 
> Review request for Aurora, Santhosh Kumar Shanmugham and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We'd like to direct all of our users to the new UI in order to do a thorough 
> beta test. It's a little ham-handed but the idea is this would be removed and 
> the old UI completely replaced by the next official Scheduler release. So 
> only those brave souls deploying from master would see this.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/css/app.css 
> 67a0ac5b3fa6372867e6bc2d963a3867dda6748b 
>   src/main/resources/scheduler/assets/scheduler/index.html 
> 98e465a46c006bcfea765a4530fc5d6d67bd1539 
> 
> 
> Diff: https://reviews.apache.org/r/63165/diff/1/
> 
> 
> Testing
> ---
> 
> See screenshot.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-19 at 1.16.35 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/19/9dd8eccb-c983-427a-a41e-d236f1f3b2a5__Screen_Shot_2017-10-19_at_1.16.35_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63135: Refactor Job Page to make it more pluggable

[Kai Huang]

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


Ship it!




LGTM


ui/src/main/js/components/Tabs.js
Lines 7 (patched)


s/his/this/


- Kai Huang


On Oct. 19, 2017, 4 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63135/
> ---
> 
> (Updated Oct. 19, 2017, 4 a.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Refactor Job Page to create cleaner abstractions. Also refactors Tabs to make 
> them easier to write tests for (using shallow from enzyme). 
> 
> Also improved Pagination code (was generating a warning) now that React can 
> handle returning null from components (and doing the right thing).
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/JobHistory.js PRE-CREATION 
>   ui/src/main/js/components/JobOverview.js PRE-CREATION 
>   ui/src/main/js/components/JobStatus.js PRE-CREATION 
>   ui/src/main/js/components/Pagination.js 
> 094ef2b248bd802ec63f562a978fd1b4d4bb5eb8 
>   ui/src/main/js/components/Tabs.js e382aa74e0f73e78c8092a58a605bffc54d15a81 
>   ui/src/main/js/components/__tests__/JobHistory-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/JobStatus-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Tabs-test.js 
> 252e5e001478b5a727e208b6b2d09d10128e83b9 
>   ui/src/main/js/pages/Job.js 070f1e43e4fe9c9eac688a9514c14e3296ef41af 
>   ui/src/main/js/pages/__tests__/Job-test.js 
> 03ef20ddefcc915ed4bd6242de3fe8686efdc202 
> 
> 
> Diff: https://reviews.apache.org/r/63135/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Review Request 63165: Add banner pointing to new UI

[David McLaughlin]

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

Review request for Aurora, Santhosh Kumar Shanmugham and Bill Farner.


Repository: aurora


Description
---

We'd like to direct all of our users to the new UI in order to do a thorough 
beta test. It's a little ham-handed but the idea is this would be removed and 
the old UI completely replaced by the next official Scheduler release. So only 
those brave souls deploying from master would see this.


Diffs
-

  src/main/resources/scheduler/assets/css/app.css 
67a0ac5b3fa6372867e6bc2d963a3867dda6748b 
  src/main/resources/scheduler/assets/scheduler/index.html 
98e465a46c006bcfea765a4530fc5d6d67bd1539 


Diff: https://reviews.apache.org/r/63165/diff/1/


Testing
---

See screenshot.


File Attachments


Screen Shot 2017-10-19 at 1.16.35 PM.png
  
https://reviews.apache.org/media/uploaded/files/2017/10/19/9dd8eccb-c983-427a-a41e-d236f1f3b2a5__Screen_Shot_2017-10-19_at_1.16.35_PM.png


Thanks,

David McLaughlin

Re: Review Request 63135: Refactor Job Page to make it more pluggable

[Santhosh Kumar Shanmugham]

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


Ship it!




Ship It!

- Santhosh Kumar Shanmugham


On Oct. 18, 2017, 9 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63135/
> ---
> 
> (Updated Oct. 18, 2017, 9 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Refactor Job Page to create cleaner abstractions. Also refactors Tabs to make 
> them easier to write tests for (using shallow from enzyme). 
> 
> Also improved Pagination code (was generating a warning) now that React can 
> handle returning null from components (and doing the right thing).
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/JobHistory.js PRE-CREATION 
>   ui/src/main/js/components/JobOverview.js PRE-CREATION 
>   ui/src/main/js/components/JobStatus.js PRE-CREATION 
>   ui/src/main/js/components/Pagination.js 
> 094ef2b248bd802ec63f562a978fd1b4d4bb5eb8 
>   ui/src/main/js/components/Tabs.js e382aa74e0f73e78c8092a58a605bffc54d15a81 
>   ui/src/main/js/components/__tests__/JobHistory-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/JobStatus-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Tabs-test.js 
> 252e5e001478b5a727e208b6b2d09d10128e83b9 
>   ui/src/main/js/pages/Job.js 070f1e43e4fe9c9eac688a9514c14e3296ef41af 
>   ui/src/main/js/pages/__tests__/Job-test.js 
> 03ef20ddefcc915ed4bd6242de3fe8686efdc202 
> 
> 
> Diff: https://reviews.apache.org/r/63135/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63130: Use LockStore only for backwards compatibility

[David McLaughlin]

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


Ship it!




Ship It!

- David McLaughlin


On Oct. 18, 2017, 11:18 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63130/
> ---
> 
> (Updated Oct. 18, 2017, 11:18 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Enter backwards compatibility mode for LockStore.  This means we will restore 
> and acquire locks as before, but
> will ignore them otherwise.  Following the next release, `LockStore` will be 
> removed.
> 
> Please note that `JobUpdateController` already provides the one-at-a-time 
> update semantic in addition to using the legacy lock system for the same 
> purpose.
> 
> This can be seen as a prerequisite for https://reviews.apache.org/r/62869/, 
> which will allow for a less tricky implementation of `JobUpdateStore`.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> db2220d619910ba07fdae7f04dcafbaa5b19 
>   src/main/java/org/apache/aurora/scheduler/state/LockManager.java 
> 2723306d61162cc8071dab9f61c7828c452dcce0 
>   src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java 
> 632d25664b2864c67b6a6c27b12895cc8b8b158f 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 52c3c6618a3cf1009435ca8a9cece36365913e55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> cbe5a0deff1ebc38a9618e7d89ab073dfaf78d36 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  222ac2d1442ede17ec088363d41b99953601ab29 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> d145259653dd4df90e3877fc3e744e24c7a15d13 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  b7d9281cc9821e5eb768becd9ab181d6c069e404 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 
> c2ec1b36a90265f73eaf66e37218af81d0a71fcf 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  54202359382bfe39e7cbaec0bf4c7d65d10ca13b 
>   src/main/python/apache/aurora/client/cli/context.py 
> 9cf58396dc266e97f18f761d110e564ac02bb15d 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  f56ad67cbd659d99e68dd28efe1ef0a3625c08b8 
>   src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
> 19f9de312596395eff81bbfd073a5f617d2ef84c 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  8fca54becde34a0d10d60e05d3809fc1c41233bf 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  b48477a61eb8e8bfa4c328a1401908ad6f4a0719 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 453366ef4e4b6db45643b1927887771dbd795bf9 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> e029ada2c69ba95b04e83bb442ccb2bdadfb2dd3 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> 269b56630f912d839410395d1ece50a37d4de05c 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 8b90885214591060ca76fb2530f5eecaa08a2c29 
>   src/test/python/apache/aurora/client/cli/util.py 
> 43db828ca1cccd91c88f016b4c994fef33182fbf 
>   src/test/python/apache/aurora/client/test_base.py 
> 691ed6791de8a7e1c4a071951e2dd170090b7522 
> 
> 
> Diff: https://reviews.apache.org/r/63130/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63132: Cosmetic changes to Navigation and task metadata

[Jordan Ly]

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


Ship it!




Ship It!

- Jordan Ly


On Oct. 18, 2017, 11:38 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63132/
> ---
> 
> (Updated Oct. 18, 2017, 11:38 p.m.)
> 
> 
> Review request for Aurora and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> More beta testing feedback: cosmetic changes to Navigation and task metadata.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskConfigSummary.js 
> 2a325d94ee3553000527641af6fd6e17c4c1238c 
>   ui/src/main/sass/components/_job-page.scss 
> 91b731f282c14cd0088f7e9085361a96fd62ade3 
>   ui/src/main/sass/components/_navigation.scss 
> ecf0ca6dd7128d90f367253e2403d22b269fa830 
> 
> 
> Diff: https://reviews.apache.org/r/63132/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-18 at 4.36.43 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/18/668c2fc0-6061-4fce-a676-ccd2a326f05a__Screen_Shot_2017-10-18_at_4.36.43_PM.png
> Screen Shot 2017-10-18 at 4.37.21 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/18/682ea38b-ed9f-460b-a1e2-f3133d3a5871__Screen_Shot_2017-10-18_at_4.37.21_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 60942: Remove task level resource fields from thrift interface and db

[Joshua Cohen]

> On Oct. 18, 2017, 7:38 p.m., Nicolás Donatucci wrote:
> > I found an issue a while back after talking to Joshua. 
> > It is regarding the downgrade script for my db migration.
> > It works just fine assuming there is only one entry of ram, cpu and disk 
> > per taskId on the task_resource table.
> > The thing is, during one of the tests, after upgrading from 18 to patched 
> > version and then downgrading to 18, the migration failed because that table 
> > had two entries of resources for some of the tasks, all of them with value 
> > of 0.
> >  
> > I was not able to tell how those entries got into the task_resource table 
> > but, for the patch's sake, values of ramMb, diskMb and numCpus in the 
> > task_configs table are no longer used in 0.18.0. In 0.17.0, those fields 
> > are backfilled if the new Resources are set.
> > 
> > One possible approach is to have the downgrade script only create the old 
> > fields and leave them in 0.
> > Another approach is ignoring entries with value 0 when backfilling in the 
> > downgrade script.
> > The best thing would be to know why there are nultiple entries in the 
> > task_resource table. Any ideas?
> 
> Bill Farner wrote:
> I suggest you wait for https://reviews.apache.org/r/62869/, which should 
> land soon.  This might make it easier to think about the upgrade path (i.e. 
> no need to think about SQL).

+1, once the above diff lands, the entirety of the DB migration problem in this 
diff goes away.


- Joshua


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


On Aug. 24, 2017, 7:05 p.m., Nicolás Donatucci wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60942/
> ---
> 
> (Updated Aug. 24, 2017, 7:05 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1707
> https://issues.apache.org/jira/browse/AURORA-1707
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Removed task level resource fields from the DB and the thrift-API.
> To do this, a new DB migration was added. When upgrading, it just drops the 
> task level resource fields. When downgrading, it creates the fields again and 
> populates them with information from the task_resource table.
> 
> IMPORTANT: One of the python client tests is failing (test_cron_diff). This 
> is not serious, I think it is a problem with the ordering of the elements of 
> Resources (had similar problems with other python client tests that instead 
> of comparing the Resources as a set, compared them as lists and thus order 
> mattered). Nevertheless, I could not fully understand the code of that test. 
> I was hoping someone could give me a hand with that. 
> But then again, it is a smaller issue and so the patch can start being 
> reviewed.
> 
> Issue Related: AURORA-1707
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md fd2618fee8ef05091849e177bd99fc321548be90 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3749531b5412d7ca217736aa85eed8e6606225ad 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> d2eb6aa6e4a155b2d28debab2ca10dfc76d57213 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> cda55c55680a19ed421299a8949299b21949787b 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V004_CreateTaskResourceTable.java
>  af106a8a9ee8c14122e98bcc0ec44b616f21d61f 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V011_DropResourceFields.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> 138cd5316adc73eed24fc7accc53885dd5d5bee5 
>   src/main/python/apache/aurora/client/cli/diff_formatter.py 
> 78717774aa3fbaf83a5fb850bc9f9f4a4038d70f 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> b79ae56bee0e5692cacf1e66f4a4126b06aaffdc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  5422183e4a1fe122fc0e1aa871aa75ae102e322d 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 7a86f47af67adb3e488381d30ddf424549deefbc 
>   src/test/java/org/apache/aurora/scheduler/http/TestUtils.java 
> 689482c9f6c49bcca781834566edeb975d2f3af2 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
>  caaba9b6dff46ff0b037759f1c817a321ae15ee4 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> 43e32eede27bbf26363a3fd1ca34ffe6f8c01a73 
>   
> src/test/java/org/apache/aurora/scheduler/updater/UpdateAgentReserverImplTest.java
>  1bc2a778ad3f1543

Re: Review Request 63130: Use LockStore only for backwards compatibility

[Stephan Erb]

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


Ship it!




Ship It!

- Stephan Erb


On Oct. 19, 2017, 1:18 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63130/
> ---
> 
> (Updated Oct. 19, 2017, 1:18 a.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Enter backwards compatibility mode for LockStore.  This means we will restore 
> and acquire locks as before, but
> will ignore them otherwise.  Following the next release, `LockStore` will be 
> removed.
> 
> Please note that `JobUpdateController` already provides the one-at-a-time 
> update semantic in addition to using the legacy lock system for the same 
> purpose.
> 
> This can be seen as a prerequisite for https://reviews.apache.org/r/62869/, 
> which will allow for a less tricky implementation of `JobUpdateStore`.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> db2220d619910ba07fdae7f04dcafbaa5b19 
>   src/main/java/org/apache/aurora/scheduler/state/LockManager.java 
> 2723306d61162cc8071dab9f61c7828c452dcce0 
>   src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java 
> 632d25664b2864c67b6a6c27b12895cc8b8b158f 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 52c3c6618a3cf1009435ca8a9cece36365913e55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> cbe5a0deff1ebc38a9618e7d89ab073dfaf78d36 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  222ac2d1442ede17ec088363d41b99953601ab29 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> d145259653dd4df90e3877fc3e744e24c7a15d13 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  b7d9281cc9821e5eb768becd9ab181d6c069e404 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 
> c2ec1b36a90265f73eaf66e37218af81d0a71fcf 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  54202359382bfe39e7cbaec0bf4c7d65d10ca13b 
>   src/main/python/apache/aurora/client/cli/context.py 
> 9cf58396dc266e97f18f761d110e564ac02bb15d 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  f56ad67cbd659d99e68dd28efe1ef0a3625c08b8 
>   src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
> 19f9de312596395eff81bbfd073a5f617d2ef84c 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  8fca54becde34a0d10d60e05d3809fc1c41233bf 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  b48477a61eb8e8bfa4c328a1401908ad6f4a0719 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 453366ef4e4b6db45643b1927887771dbd795bf9 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> e029ada2c69ba95b04e83bb442ccb2bdadfb2dd3 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> 269b56630f912d839410395d1ece50a37d4de05c 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 8b90885214591060ca76fb2530f5eecaa08a2c29 
>   src/test/python/apache/aurora/client/cli/util.py 
> 43db828ca1cccd91c88f016b4c994fef33182fbf 
>   src/test/python/apache/aurora/client/test_base.py 
> 691ed6791de8a7e1c4a071951e2dd170090b7522 
> 
> 
> Diff: https://reviews.apache.org/r/63130/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63121: Remove static bans for task groups that are no longer pending

[Stephan Erb]

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


Ship it!




Ship It!

- Stephan Erb


On Oct. 19, 2017, 2:04 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63121/
> ---
> 
> (Updated Oct. 19, 2017, 2:04 a.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This alleviates a (slow) memory leak in static offer bans, as entries are only
> removed when an offer is removed.  If a pending task group is depleted
> (either by fully scheduling the group, or terminating the job), the entry
> remains.  This issue is exacerbated when offers are held for a longer 
> duration,
> as is proposed in https://reviews.apache.org/r/62956/.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 
> 0637eb7f85125cf70b588d56fa7dc88130947837 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskGroups.java 
> 2d3492d05986ef65519fd7a8c71396d055b6881f 
>   src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java 
> 6e77857fcf209d3fe70fbd30cfd8484ea0414ee2 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 2cfdc090ff75a63111ae146c9fe7b3542e7ac83f 
>   src/test/java/org/apache/aurora/scheduler/scheduling/TaskGroupsTest.java 
> b88d5f13889b81ba4b0171efaf6c759d23976a39 
> 
> 
> Diff: https://reviews.apache.org/r/63121/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>