Review Request 54967: AURORA-1856 Expose stats on deleted job updates in JobUpdateHistoryPruner

2016-12-21 Thread Mehrdad Nurolahzade 

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

Review request for Aurora, David McLaughlin and Joshua Cohen.


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


Repository: aurora


Description
---

AURORA-1856 Expose stats on deleted job updates in JobUpdateHistoryPruner


Diffs
-

  src/main/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPruner.java 
6ab39ca0a64dfab9fe2fdec79fef1b4d320b6dc6 
  
src/test/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPrunerTest.java
 20f790ef4d04cd8aaa7cdab4442040a31fa72838 

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


Testing
---


Thanks,

Mehrdad Nurolahzade

Re: Review Request 54959: AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector

2016-12-21 Thread Aurora ReviewBot 

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



Master (38b9311) 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 Dec. 22, 2016, 6:49 a.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54959/
> ---
> 
> (Updated Dec. 22, 2016, 6:49 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1842
> https://issues.apache.org/jira/browse/AURORA-1842
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
> 8cd7bfe18716e38da79df9c869a69bccfe1afe1b 
> 
> Diff: https://reviews.apache.org/r/54959/diff/
> 
> 
> Testing
> ---
> 
> ```
> curl 192.168.33.7:8081/vars | grep row_garbage_collector
>   % Total% Received % Xferd  Average Speed   TimeTime Time  
> Current
>  Dload  Upload   Total   SpentLeft  Speed
> 100 417610 417610 0  6407k  0 --:--:-- --:--:-- --:--:-- 6797k
> row_garbage_collector_deleted 0
> row_garbage_collector_run_events 1
> row_garbage_collector_run_events_per_sec 0.0
> row_garbage_collector_run_nanos_per_event 0.0
> row_garbage_collector_run_nanos_total 127851591
> row_garbage_collector_run_nanos_total_per_sec 0.0
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>

Re: Review Request 54959: AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector

2016-12-21 Thread Mehrdad Nurolahzade 

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

(Updated Dec. 21, 2016, 10:49 p.m.)


Review request for Aurora, Joshua Cohen and Stephan Erb.


Changes
---

Feedback


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


Repository: aurora


Description
---

AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
8cd7bfe18716e38da79df9c869a69bccfe1afe1b 

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


Testing
---

```
curl 192.168.33.7:8081/vars | grep row_garbage_collector
  % Total% Received % Xferd  Average Speed   TimeTime Time  Current
 Dload  Upload   Total   SpentLeft  Speed
100 417610 417610 0  6407k  0 --:--:-- --:--:-- --:--:-- 6797k
row_garbage_collector_deleted 0
row_garbage_collector_run_events 1
row_garbage_collector_run_events_per_sec 0.0
row_garbage_collector_run_nanos_per_event 0.0
row_garbage_collector_run_nanos_total 127851591
row_garbage_collector_run_nanos_total_per_sec 0.0
```


Thanks,

Mehrdad Nurolahzade

Re: Review Request 54959: AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector

2016-12-21 Thread Mehrdad Nurolahzade 


> On Dec. 21, 2016, 6:26 p.m., David McLaughlin wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java,
> >  lines 97-98
> > 
> >
> > Inline?

This was originally inline but threw PMD off somehow, refactoring to a variable 
fixed it (see my comment above on ReviewBot error message).
I just noticed that wrapping the expression in a `Long` also makes PMD happy.


- Mehrdad


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


On Dec. 21, 2016, 10:49 p.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54959/
> ---
> 
> (Updated Dec. 21, 2016, 10:49 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1842
> https://issues.apache.org/jira/browse/AURORA-1842
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
> 8cd7bfe18716e38da79df9c869a69bccfe1afe1b 
> 
> Diff: https://reviews.apache.org/r/54959/diff/
> 
> 
> Testing
> ---
> 
> ```
> curl 192.168.33.7:8081/vars | grep row_garbage_collector
>   % Total% Received % Xferd  Average Speed   TimeTime Time  
> Current
>  Dload  Upload   Total   SpentLeft  Speed
> 100 417610 417610 0  6407k  0 --:--:-- --:--:-- --:--:-- 6797k
> row_garbage_collector_deleted 0
> row_garbage_collector_run_events 1
> row_garbage_collector_run_events_per_sec 0.0
> row_garbage_collector_run_nanos_per_event 0.0
> row_garbage_collector_run_nanos_total 127851591
> row_garbage_collector_run_nanos_total_per_sec 0.0
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

2016-12-21 Thread David McLaughlin 

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



Is it even worth making this a controllable option? I think it's a bug that the 
existing client will retry a non-idempotent operation, especially one that can 
take down your cluster. 

Either way, we should be sure to default the option to False for anything that 
*shouldn't* be retried. And definitely for the admin options where the retries 
can take down your cluster.

- David McLaughlin


On Dec. 22, 2016, 12:06 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 22, 2016, 12:06 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54959: AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector

2016-12-21 Thread David McLaughlin 

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




src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
(line 67)


requireNonNull is just used to give an early and clear signal of where a 
null object was passed when it shouldn't be. If you're calling a method on the 
object in the constructor like this, it's redundant.



src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
(lines 97 - 98)


Inline?


- David McLaughlin


On Dec. 22, 2016, 1:11 a.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54959/
> ---
> 
> (Updated Dec. 22, 2016, 1:11 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1842
> https://issues.apache.org/jira/browse/AURORA-1842
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
> 8cd7bfe18716e38da79df9c869a69bccfe1afe1b 
> 
> Diff: https://reviews.apache.org/r/54959/diff/
> 
> 
> Testing
> ---
> 
> ```
> curl 192.168.33.7:8081/vars | grep row_garbage_collector
>   % Total% Received % Xferd  Average Speed   TimeTime Time  
> Current
>  Dload  Upload   Total   SpentLeft  Speed
> 100 417610 417610 0  6407k  0 --:--:-- --:--:-- --:--:-- 6797k
> row_garbage_collector_deleted 0
> row_garbage_collector_run_events 1
> row_garbage_collector_run_events_per_sec 0.0
> row_garbage_collector_run_nanos_per_event 0.0
> row_garbage_collector_run_nanos_total 127851591
> row_garbage_collector_run_nanos_total_per_sec 0.0
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>

Re: Review Request 54960: Exposed stats on number of offers rescinded and number of slaves lost.

2016-12-21 Thread Pradyumna Kaushik 


> On Dec. 22, 2016, 1:23 a.m., Mehrdad Nurolahzade wrote:
> > Could you please refactor away from `Stats`, as was suggested by @zmanji on 
> > a similar rb [https://reviews.apache.org/r/54959/]?

@Mehrdad will do.


- Pradyumna


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


On Dec. 22, 2016, 12:58 a.m., Pradyumna Kaushik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54960/
> ---
> 
> (Updated Dec. 22, 2016, 12:58 a.m.)
> 
> 
> Review request for Aurora and Mehrdad Nurolahzade.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Exposed stats on number of offers rescinded and number of slaves lost.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> d63bfc10442b791b423d0d8c83cc280610103299 
> 
> Diff: https://reviews.apache.org/r/54960/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Pradyumna Kaushik
> 
>

Re: Review Request 54959: AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector

2016-12-21 Thread Aurora ReviewBot 

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



Master (38b9311) 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 Dec. 22, 2016, 1:11 a.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54959/
> ---
> 
> (Updated Dec. 22, 2016, 1:11 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1842
> https://issues.apache.org/jira/browse/AURORA-1842
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
> 8cd7bfe18716e38da79df9c869a69bccfe1afe1b 
> 
> Diff: https://reviews.apache.org/r/54959/diff/
> 
> 
> Testing
> ---
> 
> ```
> curl 192.168.33.7:8081/vars | grep row_garbage_collector
>   % Total% Received % Xferd  Average Speed   TimeTime Time  
> Current
>  Dload  Upload   Total   SpentLeft  Speed
> 100 417610 417610 0  6407k  0 --:--:-- --:--:-- --:--:-- 6797k
> row_garbage_collector_deleted 0
> row_garbage_collector_run_events 1
> row_garbage_collector_run_events_per_sec 0.0
> row_garbage_collector_run_nanos_per_event 0.0
> row_garbage_collector_run_nanos_total 127851591
> row_garbage_collector_run_nanos_total_per_sec 0.0
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>

Re: Review Request 54960: Exposed stats on number of offers rescinded and number of slaves lost.

2016-12-21 Thread Mehrdad Nurolahzade 

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



Could you please refactor away from `Stats`, as was suggested by @zmanji on a 
similar rb [https://reviews.apache.org/r/54959/]?

- Mehrdad Nurolahzade


On Dec. 21, 2016, 4:58 p.m., Pradyumna Kaushik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54960/
> ---
> 
> (Updated Dec. 21, 2016, 4:58 p.m.)
> 
> 
> Review request for Aurora and Mehrdad Nurolahzade.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Exposed stats on number of offers rescinded and number of slaves lost.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> d63bfc10442b791b423d0d8c83cc280610103299 
> 
> Diff: https://reviews.apache.org/r/54960/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Pradyumna Kaushik
> 
>

Re: Review Request 54960: Exposed stats on number of offers rescinded and number of slaves lost.

2016-12-21 Thread Aurora ReviewBot 

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



Master (38b9311) 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 Dec. 22, 2016, 12:58 a.m., Pradyumna Kaushik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54960/
> ---
> 
> (Updated Dec. 22, 2016, 12:58 a.m.)
> 
> 
> Review request for Aurora and Mehrdad Nurolahzade.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Exposed stats on number of offers rescinded and number of slaves lost.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> d63bfc10442b791b423d0d8c83cc280610103299 
> 
> Diff: https://reviews.apache.org/r/54960/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Pradyumna Kaushik
> 
>

Re: Review Request 54959: AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector

2016-12-21 Thread Mehrdad Nurolahzade 

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

(Updated Dec. 21, 2016, 5:11 p.m.)


Review request for Aurora, Joshua Cohen and Stephan Erb.


Changes
---

- Replaced `Stats` usage with `StatsProvider`
- Refactored `LOG` statement to get around reported PMD problem


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


Repository: aurora


Description
---

AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
8cd7bfe18716e38da79df9c869a69bccfe1afe1b 

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


Testing
---

```
curl 192.168.33.7:8081/vars | grep row_garbage_collector
  % Total% Received % Xferd  Average Speed   TimeTime Time  Current
 Dload  Upload   Total   SpentLeft  Speed
100 417610 417610 0  6407k  0 --:--:-- --:--:-- --:--:-- 6797k
row_garbage_collector_deleted 0
row_garbage_collector_run_events 1
row_garbage_collector_run_events_per_sec 0.0
row_garbage_collector_run_nanos_per_event 0.0
row_garbage_collector_run_nanos_total 127851591
row_garbage_collector_run_nanos_total_per_sec 0.0
```


Thanks,

Mehrdad Nurolahzade

Re: Review Request 54959: AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector

2016-12-21 Thread Mehrdad Nurolahzade 


> On Dec. 21, 2016, 4:33 p.m., Aurora ReviewBot wrote:
> > Master (38b9311) is red with this patch.
> >   ./build-support/jenkins/build.sh
> > 
> > at 
> > org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
> > at 
> > org.gradle.launcher.daemon.server.exec.RequestStopIfSingleUsedDaemon.execute(RequestStopIfSingleUsedDaemon.java:34)
> > at 
> > org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
> > at 
> > org.gradle.launcher.daemon.server.exec.ForwardClientInput$2.call(ForwardClientInput.java:74)
> > at 
> > org.gradle.launcher.daemon.server.exec.ForwardClientInput$2.call(ForwardClientInput.java:72)
> > at org.gradle.util.Swapper.swap(Swapper.java:38)
> > at 
> > org.gradle.launcher.daemon.server.exec.ForwardClientInput.execute(ForwardClientInput.java:72)
> > at 
> > org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
> > at 
> > org.gradle.launcher.daemon.server.exec.LogAndCheckHealth.execute(LogAndCheckHealth.java:55)
> > at 
> > org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
> > at 
> > org.gradle.launcher.daemon.server.exec.LogToClient.doBuild(LogToClient.java:60)
> > at 
> > org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:36)
> > at 
> > org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
> > at 
> > org.gradle.launcher.daemon.server.exec.EstablishBuildEnvironment.doBuild(EstablishBuildEnvironment.java:72)
> > at 
> > org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:36)
> > at 
> > org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
> > at 
> > org.gradle.launcher.daemon.server.exec.HintGCAfterBuild.execute(HintGCAfterBuild.java:44)
> > at 
> > org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
> > at 
> > org.gradle.launcher.daemon.server.exec.StartBuildOrRespondWithBusy$1.run(StartBuildOrRespondWithBusy.java:50)
> > at 
> > org.gradle.launcher.daemon.server.DaemonStateCoordinator$1.run(DaemonStateCoordinator.java:293)
> > at 
> > org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:54)
> > at 
> > org.gradle.internal.concurrent.StoppableExecutorImpl$1.run(StoppableExecutorImpl.java:40)
> > at 
> > java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> > at 
> > java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> > at java.lang.Thread.run(Thread.java:745)
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java:95:
> >Missing arguments, expected 1 argument but have 0
> > :pmdMain FAILED
> > 
> > FAILURE: Build failed with an exception.
> > 
> > * What went wrong:
> > Execution failed for task ':pmdMain'.
> > > 1 PMD rule violations were found. See the report at: 
> > > file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/pmd/main.html
> > 
> > * Try:
> > Run with --stacktrace option to get the stack trace. Run with --info or 
> > --debug option to get more log output.
> > 
> > BUILD FAILED
> > 
> > Total time: 2 mins 27.377 secs
> > 
> > 
> > I will refresh this build result if you post a review containing 
> > "@ReviewBot retry"

This seems to be PMD bug in the rule 
https://pmd.github.io/pmd-5.5.2/pmd-java/xref/net/sourceforge/pmd/lang/java/rule/logging/InvalidSlf4jMessageFormatRule.html.
 Replacing the expression with a variable reference made it go away.


- Mehrdad


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


On Dec. 21, 2016, 4:30 p.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54959/
> ---
> 
> (Updated Dec. 21, 2016, 4:30 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1842
> https://issues.apache.org/jira/browse/AURORA-1842
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
> 8cd7bfe18716e38da79df9c869a69bccfe1afe1b 
> 
> Diff: https://reviews.apache.org/r/54959/diff/
> 
> 
> Testing
> ---
> 
> ```
> curl 192.168.33.7:8081/vars | grep

Review Request 54960: Exposed stats on number of offers rescinded and number of slaves lost.

2016-12-21 Thread Pradyumna Kaushik 

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

Review request for Aurora and Mehrdad Nurolahzade.


Repository: aurora


Description
---

Exposed stats on number of offers rescinded and number of slaves lost.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
d63bfc10442b791b423d0d8c83cc280610103299 

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


Testing
---

./build-support/jenkins/build.sh
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Pradyumna Kaushik

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

2016-12-21 Thread Karthik Anantha Padmanabhan 


> On Dec. 22, 2016, 12:54 a.m., Karthik Anantha Padmanabhan wrote:
> > src/main/python/apache/aurora/admin/aurora_admin.py, line 55
> > 
> >
> > In the current implementation we retry only when  a `TimeoutError` is 
> > raised.
> > 
> > And we raise `TimeoutError` when trying to construct the client and 
> > connecting to the leader/ZK. And this might involve things like unable to 
> > resolve the leader url etc.

So when an API call timesout we do not retry.


- Karthik


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


On Dec. 22, 2016, 12:06 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 22, 2016, 12:06 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

2016-12-21 Thread Karthik Anantha Padmanabhan 

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




src/main/python/apache/aurora/admin/aurora_admin.py (line 55)


In the current implementation we retry only when  a `TimeoutError` is 
raised.

And we raise `TimeoutError` when trying to construct the client and 
connecting to the leader/ZK. And this might involve things like unable to 
resolve the leader url etc.


- Karthik Anantha Padmanabhan


On Dec. 22, 2016, 12:06 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 22, 2016, 12:06 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

2016-12-21 Thread Santhosh Kumar Shanmugham 

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




src/main/python/apache/aurora/admin/aurora_admin.py (line 51)


Since we are merely exposing control over something that is already 
present, can we just call this `--retry` and default to `true`. I feel like it 
will be easier to read.



src/main/python/apache/aurora/admin/aurora_admin.py (line 55)


This is not true. We still retry on timeouts.

It is misleading to have a switch `--no-retry` and still retry on some 
situtations. Maybe it should be all or nothing.



src/test/python/apache/aurora/client/api/test_scheduler_client.py (line 250)


This the same test as above isn't it. Can we test when we are no requesting 
retries?


- Santhosh Kumar Shanmugham


On Dec. 21, 2016, 4:06 p.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 21, 2016, 4:06 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54959: AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector

2016-12-21 Thread Aurora ReviewBot 

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



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

at 
org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
at 
org.gradle.launcher.daemon.server.exec.RequestStopIfSingleUsedDaemon.execute(RequestStopIfSingleUsedDaemon.java:34)
at 
org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
at 
org.gradle.launcher.daemon.server.exec.ForwardClientInput$2.call(ForwardClientInput.java:74)
at 
org.gradle.launcher.daemon.server.exec.ForwardClientInput$2.call(ForwardClientInput.java:72)
at org.gradle.util.Swapper.swap(Swapper.java:38)
at 
org.gradle.launcher.daemon.server.exec.ForwardClientInput.execute(ForwardClientInput.java:72)
at 
org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
at 
org.gradle.launcher.daemon.server.exec.LogAndCheckHealth.execute(LogAndCheckHealth.java:55)
at 
org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
at 
org.gradle.launcher.daemon.server.exec.LogToClient.doBuild(LogToClient.java:60)
at 
org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:36)
at 
org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
at 
org.gradle.launcher.daemon.server.exec.EstablishBuildEnvironment.doBuild(EstablishBuildEnvironment.java:72)
at 
org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:36)
at 
org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
at 
org.gradle.launcher.daemon.server.exec.HintGCAfterBuild.execute(HintGCAfterBuild.java:44)
at 
org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
at 
org.gradle.launcher.daemon.server.exec.StartBuildOrRespondWithBusy$1.run(StartBuildOrRespondWithBusy.java:50)
at 
org.gradle.launcher.daemon.server.DaemonStateCoordinator$1.run(DaemonStateCoordinator.java:293)
at 
org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:54)
at 
org.gradle.internal.concurrent.StoppableExecutorImpl$1.run(StoppableExecutorImpl.java:40)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java:95:
   Missing arguments, expected 1 argument but have 0
:pmdMain FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':pmdMain'.
> 1 PMD rule violations were found. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/pmd/main.html

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

BUILD FAILED

Total time: 2 mins 27.377 secs


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

- Aurora ReviewBot


On Dec. 22, 2016, 12:30 a.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54959/
> ---
> 
> (Updated Dec. 22, 2016, 12:30 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1842
> https://issues.apache.org/jira/browse/AURORA-1842
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
> 8cd7bfe18716e38da79df9c869a69bccfe1afe1b 
> 
> Diff: https://reviews.apache.org/r/54959/diff/
> 
> 
> Testing
> ---
> 
> ```
> curl 192.168.33.7:8081/vars | grep row_garbage_collector
>   % Total% Received % Xferd  Average Speed   TimeTime Time  
> Current
>  Dload  Upload   Total   SpentLeft  Speed
> 100 417610 417610 0  6407k  0 --:--:-- --:--:-- --:--:-- 6797k
> row_garbage_collector_deleted 0
> row_garbage_collector_run_events 1
> row_garbage_collector_run_events_per_sec 0.0
> row_garbage_collector_run_nanos_per_event 0.0
> row_garbage_collector_run_nanos_total

Re: Review Request 54959: AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector

2016-12-21 Thread Zameer Manji 

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


Ship it!




LGTM modulo my `Stats` vs `StatsProvider` comment.


src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
(line 49)


I think the use of static methods like `exportLong` should be discouraged. 
It tightly binds us to the `Stats` class.

Would you mind instead adding `StatsProvider` to your constructor and using 
that instance to create the metrics?


- Zameer Manji


On Dec. 21, 2016, 4:30 p.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54959/
> ---
> 
> (Updated Dec. 21, 2016, 4:30 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1842
> https://issues.apache.org/jira/browse/AURORA-1842
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
> 8cd7bfe18716e38da79df9c869a69bccfe1afe1b 
> 
> Diff: https://reviews.apache.org/r/54959/diff/
> 
> 
> Testing
> ---
> 
> ```
> curl 192.168.33.7:8081/vars | grep row_garbage_collector
>   % Total% Received % Xferd  Average Speed   TimeTime Time  
> Current
>  Dload  Upload   Total   SpentLeft  Speed
> 100 417610 417610 0  6407k  0 --:--:-- --:--:-- --:--:-- 6797k
> row_garbage_collector_deleted 0
> row_garbage_collector_run_events 1
> row_garbage_collector_run_events_per_sec 0.0
> row_garbage_collector_run_nanos_per_event 0.0
> row_garbage_collector_run_nanos_total 127851591
> row_garbage_collector_run_nanos_total_per_sec 0.0
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>

Review Request 54959: AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector

2016-12-21 Thread Mehrdad Nurolahzade 

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

Review request for Aurora, Joshua Cohen and Stephan Erb.


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


Repository: aurora


Description
---

AURORA-1842 Expose stats on garbage collected rows in RowGarbageCollector


Diffs
-

  src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
8cd7bfe18716e38da79df9c869a69bccfe1afe1b 

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


Testing
---

```
curl 192.168.33.7:8081/vars | grep row_garbage_collector
  % Total% Received % Xferd  Average Speed   TimeTime Time  Current
 Dload  Upload   Total   SpentLeft  Speed
100 417610 417610 0  6407k  0 --:--:-- --:--:-- --:--:-- 6797k
row_garbage_collector_deleted 0
row_garbage_collector_run_events 1
row_garbage_collector_run_events_per_sec 0.0
row_garbage_collector_run_nanos_per_event 0.0
row_garbage_collector_run_nanos_total 127851591
row_garbage_collector_run_nanos_total_per_sec 0.0
```


Thanks,

Mehrdad Nurolahzade

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

2016-12-21 Thread Aurora ReviewBot 

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


Ship it!




Master (38b9311) 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 Dec. 22, 2016, 12:06 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 22, 2016, 12:06 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>