Re: Review Request 42041: Enable H2 query statistics collection.

2016-01-09 Thread John Sirois

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

Ship it!


Ship It!

- John Sirois


On Jan. 8, 2016, 2:52 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 8, 2016, 2:52 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> Benchmarks
> 
> Master (c595228):
> Benchmark 
> (numPendingTasks)   Mode  Cnt  Score  Error  Units
> SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark 
>   N/A  thrpt   10  64138.084 ± 6732.130  ops/s
> SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>   N/A  thrpt   10  23863.861 ± 2101.622  ops/s
> SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10   2228.883 ±  311.434  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
> 1  thrpt   10 50.914 ±2.488  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>10  thrpt   10 43.729 ±3.038  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>   100  thrpt   10 44.409 ±4.426  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>  1000  thrpt   10 40.429 ±7.526  ops/s
> SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10  22942.538 ± 1281.331  ops/s
> 
> 
> This change:
> Benchmark 
> (numPendingTasks)   Mode  Cnt  Score  Error  Units
> SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark 
>   N/A  thrpt   10  65285.628 ± 2422.816  ops/s
> SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>   N/A  thrpt   10  24573.332 ± 1332.474  ops/s
> SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10   2430.402 ±  258.860  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
> 1  thrpt   10 43.810 ±2.669  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>10  thrpt   10 37.378 ±   14.637  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>   100  thrpt   10 40.180 ±9.738  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>  1000  thrpt   10 24.130 ±   15.746  ops/s
> SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10  18429.830 ± 3077.426  ops/s
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 42041: Enable H2 query statistics collection.

2016-01-09 Thread John Sirois


> On Jan. 7, 2016, 4:21 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 126
> > 
> >
> > Are there any perf implications from having it ON by default? Should it 
> > be a configurable flag instead?
> 
> Zameer Manji wrote:
> Good question, I'll run benchmarks and update the testing done.
> 
> John Sirois wrote:
> And if there are perf implications, there is always [`SET 
> QUERY_STATISTICS`](http://www.h2database.com/html/grammar.html?highlight=QUERY_STATISTICS=QUERY_STATISTICS#set_query_statistics)
>  via the /h2console - and maybe that could just be documented as an ad-hoc 
> option.
> 
> Zameer Manji wrote:
> John, using `SET QUERY_STATISTICS` will not persist across failovers and 
> is not retroactive. This means operators need to run `SET QUERY_STATISTICS` 
> and wait for data to be collected if they notice the scheduler is slower than 
> expected. If this option is set when the database is created the data 
> collected should reflect all operations done by the scheduler and that is 
> more useful for triaging slow scheduling in large clusters.
> 
> I think providing a flag or enabling it by default (pending results of 
> benchmarks) are better solutions.
> 
> John Sirois wrote:
> Makes sense.  Have you thought about the existing slow query logging and 
> how that evolves with this?  Probably fine side-by-side, but maybe confusing 
> and less valuable once the mem store is gone and the db store is all we have.
> 
> Zameer Manji wrote:
> I have updated the testing done with benchmarks and the change appears to 
> be small for scheduling. I think the utility of query statistics justifies 
> turning it on by default.
> 
> John, I think slow query logging can be removed once the mem store has 
> been removed.

Sounds good to me.


- John


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


On Jan. 8, 2016, 2:52 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 8, 2016, 2:52 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> Benchmarks
> 
> Master (c595228):
> Benchmark 
> (numPendingTasks)   Mode  Cnt  Score  Error  Units
> SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark 
>   N/A  thrpt   10  64138.084 ± 6732.130  ops/s
> SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>   N/A  thrpt   10  23863.861 ± 2101.622  ops/s
> SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10   2228.883 ±  311.434  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
> 1  thrpt   10 50.914 ±2.488  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>10  thrpt   10 43.729 ±3.038  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>   100  thrpt   10 44.409 ±4.426  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>  1000  thrpt   10 40.429 ±7.526  ops/s
> SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10  22942.538 ± 1281.331  ops/s
> 
> 
> This change:
> Benchmark 
> (numPendingTasks)   Mode  Cnt  Score  Error  Units
> SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark 
>   N/A  thrpt   10  65285.628 ± 2422.816  ops/s
> SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>   N/A  thrpt   10  24573.332 ± 1332.474  ops/s
> 

Re: Review Request 42041: Enable H2 query statistics collection.

2016-01-08 Thread Zameer Manji


> On Jan. 7, 2016, 3:21 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 126
> > 
> >
> > Are there any perf implications from having it ON by default? Should it 
> > be a configurable flag instead?
> 
> Zameer Manji wrote:
> Good question, I'll run benchmarks and update the testing done.
> 
> John Sirois wrote:
> And if there are perf implications, there is always [`SET 
> QUERY_STATISTICS`](http://www.h2database.com/html/grammar.html?highlight=QUERY_STATISTICS=QUERY_STATISTICS#set_query_statistics)
>  via the /h2console - and maybe that could just be documented as an ad-hoc 
> option.
> 
> Zameer Manji wrote:
> John, using `SET QUERY_STATISTICS` will not persist across failovers and 
> is not retroactive. This means operators need to run `SET QUERY_STATISTICS` 
> and wait for data to be collected if they notice the scheduler is slower than 
> expected. If this option is set when the database is created the data 
> collected should reflect all operations done by the scheduler and that is 
> more useful for triaging slow scheduling in large clusters.
> 
> I think providing a flag or enabling it by default (pending results of 
> benchmarks) are better solutions.
> 
> John Sirois wrote:
> Makes sense.  Have you thought about the existing slow query logging and 
> how that evolves with this?  Probably fine side-by-side, but maybe confusing 
> and less valuable once the mem store is gone and the db store is all we have.

I have updated the testing done with benchmarks and the change appears to be 
small for scheduling. I think the utility of query statistics justifies turning 
it on by default.

John, I think slow query logging can be removed once the mem store has been 
removed.


- Zameer


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


On Jan. 8, 2016, 1:52 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 8, 2016, 1:52 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> Benchmarks
> 
> Master (c595228):
> Benchmark 
> (numPendingTasks)   Mode  Cnt  Score  Error  Units
> SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark 
>   N/A  thrpt   10  64138.084 ± 6732.130  ops/s
> SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>   N/A  thrpt   10  23863.861 ± 2101.622  ops/s
> SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10   2228.883 ±  311.434  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
> 1  thrpt   10 50.914 ±2.488  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>10  thrpt   10 43.729 ±3.038  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>   100  thrpt   10 44.409 ±4.426  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>  1000  thrpt   10 40.429 ±7.526  ops/s
> SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10  22942.538 ± 1281.331  ops/s
> 
> 
> This change:
> Benchmark 
> (numPendingTasks)   Mode  Cnt  Score  Error  Units
> SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark 
>   N/A  thrpt   10  65285.628 ± 2422.816  ops/s
> SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>   N/A  thrpt   10  24573.332 ± 1332.474  ops/s
> SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt 

Re: Review Request 42041: Enable H2 query statistics collection.

2016-01-08 Thread Zameer Manji

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

(Updated Jan. 8, 2016, 1:52 p.m.)


Review request for Aurora, John Sirois and Maxim Khutornenko.


Repository: aurora


Description
---

With this enabled operators can visit the H2 console at /h2console and run 
queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
e3efbdb80d8b18312bf1589eef883cdeee65b225 

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


Testing (updated)
---

Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.

Benchmarks

Master (c595228):
Benchmark 
(numPendingTasks)   Mode  Cnt  Score  Error  Units
SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark   
N/A  thrpt   10  64138.084 ± 6732.130  ops/s
SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark  
N/A  thrpt   10  23863.861 ± 2101.622  ops/s
SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
N/A  thrpt   10   2228.883 ±  311.434  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
  1  thrpt   10 50.914 ±2.488  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
 10  thrpt   10 43.729 ±3.038  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
100  thrpt   10 44.409 ±4.426  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
   1000  thrpt   10 40.429 ±7.526  ops/s
SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
N/A  thrpt   10  22942.538 ± 1281.331  ops/s


This change:
Benchmark 
(numPendingTasks)   Mode  Cnt  Score  Error  Units
SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark   
N/A  thrpt   10  65285.628 ± 2422.816  ops/s
SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark  
N/A  thrpt   10  24573.332 ± 1332.474  ops/s
SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
N/A  thrpt   10   2430.402 ±  258.860  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
  1  thrpt   10 43.810 ±2.669  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
 10  thrpt   10 37.378 ±   14.637  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
100  thrpt   10 40.180 ±9.738  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
   1000  thrpt   10 24.130 ±   15.746  ops/s
SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
N/A  thrpt   10  18429.830 ± 3077.426  ops/s


Thanks,

Zameer Manji



Re: Review Request 42041: Enable H2 query statistics collection.

2016-01-08 Thread Maxim Khutornenko

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

Ship it!


Ship It!

- Maxim Khutornenko


On Jan. 8, 2016, 9:52 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 8, 2016, 9:52 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> Benchmarks
> 
> Master (c595228):
> Benchmark 
> (numPendingTasks)   Mode  Cnt  Score  Error  Units
> SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark 
>   N/A  thrpt   10  64138.084 ± 6732.130  ops/s
> SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>   N/A  thrpt   10  23863.861 ± 2101.622  ops/s
> SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10   2228.883 ±  311.434  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
> 1  thrpt   10 50.914 ±2.488  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>10  thrpt   10 43.729 ±3.038  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>   100  thrpt   10 44.409 ±4.426  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>  1000  thrpt   10 40.429 ±7.526  ops/s
> SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10  22942.538 ± 1281.331  ops/s
> 
> 
> This change:
> Benchmark 
> (numPendingTasks)   Mode  Cnt  Score  Error  Units
> SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark 
>   N/A  thrpt   10  65285.628 ± 2422.816  ops/s
> SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>   N/A  thrpt   10  24573.332 ± 1332.474  ops/s
> SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10   2430.402 ±  258.860  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
> 1  thrpt   10 43.810 ±2.669  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>10  thrpt   10 37.378 ±   14.637  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>   100  thrpt   10 40.180 ±9.738  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>  1000  thrpt   10 24.130 ±   15.746  ops/s
> SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10  18429.830 ± 3077.426  ops/s
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 42041: Enable H2 query statistics collection.

2016-01-07 Thread John Sirois


> On Jan. 7, 2016, 4:21 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 126
> > 
> >
> > Are there any perf implications from having it ON by default? Should it 
> > be a configurable flag instead?
> 
> Zameer Manji wrote:
> Good question, I'll run benchmarks and update the testing done.
> 
> John Sirois wrote:
> And if there are perf implications, there is always [`SET 
> QUERY_STATISTICS`](http://www.h2database.com/html/grammar.html?highlight=QUERY_STATISTICS=QUERY_STATISTICS#set_query_statistics)
>  via the /h2console - and maybe that could just be documented as an ad-hoc 
> option.
> 
> Zameer Manji wrote:
> John, using `SET QUERY_STATISTICS` will not persist across failovers and 
> is not retroactive. This means operators need to run `SET QUERY_STATISTICS` 
> and wait for data to be collected if they notice the scheduler is slower than 
> expected. If this option is set when the database is created the data 
> collected should reflect all operations done by the scheduler and that is 
> more useful for triaging slow scheduling in large clusters.
> 
> I think providing a flag or enabling it by default (pending results of 
> benchmarks) are better solutions.

Makes sense.  Have you thought about the existing slow query logging and how 
that evolves with this?  Probably fine side-by-side, but maybe confusing and 
less valuable once the mem store is gone and the db store is all we have.


- John


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


On Jan. 7, 2016, 4:14 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 7, 2016, 4:14 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 42041: Enable H2 query statistics collection.

2016-01-07 Thread Maxim Khutornenko

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



src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java (line 126)


Are there any perf implications from having it ON by default? Should it be 
a configurable flag instead?


- Maxim Khutornenko


On Jan. 7, 2016, 11:14 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 7, 2016, 11:14 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 42041: Enable H2 query statistics collection.

2016-01-07 Thread Zameer Manji


> On Jan. 7, 2016, 3:21 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 126
> > 
> >
> > Are there any perf implications from having it ON by default? Should it 
> > be a configurable flag instead?
> 
> Zameer Manji wrote:
> Good question, I'll run benchmarks and update the testing done.
> 
> John Sirois wrote:
> And if there are perf implications, there is always [`SET 
> QUERY_STATISTICS`](http://www.h2database.com/html/grammar.html?highlight=QUERY_STATISTICS=QUERY_STATISTICS#set_query_statistics)
>  via the /h2console - and maybe that could just be documented as an ad-hoc 
> option.

John, using `SET QUERY_STATISTICS` will not persist across failovers and is not 
retroactive. This means operators need to run `SET QUERY_STATISTICS` and wait 
for data to be collected if they notice the scheduler is slower than expected. 
If this option is set when the database is created the data collected should 
reflect all operations done by the scheduler and that is more useful for 
triaging slow scheduling in large clusters.

I think providing a flag or enabling it by default (pending results of 
benchmarks) are better solutions.


- Zameer


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


On Jan. 7, 2016, 3:14 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 7, 2016, 3:14 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 42041: Enable H2 query statistics collection.

2016-01-07 Thread Zameer Manji


> On Jan. 7, 2016, 3:21 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 126
> > 
> >
> > Are there any perf implications from having it ON by default? Should it 
> > be a configurable flag instead?

Good question, I'll run benchmarks and update the testing done.


- Zameer


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


On Jan. 7, 2016, 3:14 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 7, 2016, 3:14 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 42041: Enable H2 query statistics collection.

2016-01-07 Thread Aurora ReviewBot

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


Master (206a48b) 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 Jan. 7, 2016, 11:14 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 7, 2016, 11:14 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 42041: Enable H2 query statistics collection.

2016-01-07 Thread John Sirois


> On Jan. 7, 2016, 4:21 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 126
> > 
> >
> > Are there any perf implications from having it ON by default? Should it 
> > be a configurable flag instead?
> 
> Zameer Manji wrote:
> Good question, I'll run benchmarks and update the testing done.

And if there are perf implications, there is always [`SET 
QUERY_STATISTICS`](http://www.h2database.com/html/grammar.html?highlight=QUERY_STATISTICS=QUERY_STATISTICS#set_query_statistics)
 via the /h2console - and maybe that could just be documented as an ad-hoc 
option.


- John


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


On Jan. 7, 2016, 4:14 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 7, 2016, 4:14 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>