You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2017/09/27 00:10:07 UTC
Review Request 62604: Use a more efficient query for instance ID
collision detection
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62604/
-----------------------------------------------------------
Review request for Aurora and Jordan Ly.
Repository: aurora
Description
-------
Came across this while looking at storage access patterns. Not a particularly hot code path, but this is more efficient and concise.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6b780ec90fed846531162eed5e86336e598e33a3
Diff: https://reviews.apache.org/r/62604/diff/1/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 62604: Use a more efficient query for instance ID
collision detection
Posted by Bill Farner <wf...@apache.org>.
> On Sept. 26, 2017, 5:22 p.m., Jordan Ly wrote:
> > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
> > Line 122 (original), 122 (patched)
> > <https://reviews.apache.org/r/62604/diff/1/?file=1836412#file1836412line123>
> >
> > I would probably rename `existingTasks` -> `collidingInstances` or something else to show that "these are the instance IDs that already exist but you are trying to add them".
>
> Jordan Ly wrote:
> Oops made it an issue, but since it is a nit it is more personal preference. Obviously feel free to drop if you feel the name is concise/informative enough.
I hear you. I went with the more concise `collision` in the same vein; feel free to veto.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62604/#review186349
-----------------------------------------------------------
On Sept. 26, 2017, 5:10 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62604/
> -----------------------------------------------------------
>
> (Updated Sept. 26, 2017, 5:10 p.m.)
>
>
> Review request for Aurora and Jordan Ly.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Came across this while looking at storage access patterns. Not a particularly hot code path, but this is more efficient and concise.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6b780ec90fed846531162eed5e86336e598e33a3
>
>
> Diff: https://reviews.apache.org/r/62604/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 62604: Use a more efficient query for instance ID
collision detection
Posted by Jordan Ly <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62604/#review186349
-----------------------------------------------------------
Fix it, then Ship it!
Small naming nit but otherwise LGTM
src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
Line 122 (original), 122 (patched)
<https://reviews.apache.org/r/62604/#comment262895>
I would probably rename `existingTasks` -> `collidingInstances` or something else to show that "these are the instance IDs that already exist but you are trying to add them".
- Jordan Ly
On Sept. 27, 2017, 12:10 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62604/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2017, 12:10 a.m.)
>
>
> Review request for Aurora and Jordan Ly.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Came across this while looking at storage access patterns. Not a particularly hot code path, but this is more efficient and concise.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6b780ec90fed846531162eed5e86336e598e33a3
>
>
> Diff: https://reviews.apache.org/r/62604/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 62604: Use a more efficient query for instance ID
collision detection
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62604/#review186354
-----------------------------------------------------------
Master (79ff364) is red with this patch.
./build-support/jenkins/build.sh
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
DEBUG] /tmp/tmpzK3ZPT/checkpoints/hello_world/runner has no data (current offset = 618)
generated xml file: /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/aaf4d108c31293299a0839bdc404a91802f80937.xml
[1m[31m 1 failed, 796 passed, 6 skipped, 1 warnings in 345.72 seconds [0m
FAILURE
00:28:56 06:44 [complete][31m
FAILURE[0m
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Sept. 27, 2017, 12:25 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62604/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2017, 12:25 a.m.)
>
>
> Review request for Aurora and Jordan Ly.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Came across this while looking at storage access patterns. Not a particularly hot code path, but this is more efficient and concise.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6b780ec90fed846531162eed5e86336e598e33a3
>
>
> Diff: https://reviews.apache.org/r/62604/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 62604: Use a more efficient query for instance ID
collision detection
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62604/#review186355
-----------------------------------------------------------
@ReviewBot retry
Not sure if the above failure is transient, and i can't trivially reproduce at the moment. Will inspect more closely if this repros.
- Bill Farner
On Sept. 26, 2017, 5:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62604/
> -----------------------------------------------------------
>
> (Updated Sept. 26, 2017, 5:25 p.m.)
>
>
> Review request for Aurora and Jordan Ly.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Came across this while looking at storage access patterns. Not a particularly hot code path, but this is more efficient and concise.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6b780ec90fed846531162eed5e86336e598e33a3
>
>
> Diff: https://reviews.apache.org/r/62604/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 62604: Use a more efficient query for instance ID
collision detection
Posted by Reza Motamedi <re...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62604/#review186361
-----------------------------------------------------------
Ship it!
Ship It!
- Reza Motamedi
On Sept. 27, 2017, 12:25 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62604/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2017, 12:25 a.m.)
>
>
> Review request for Aurora and Jordan Ly.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Came across this while looking at storage access patterns. Not a particularly hot code path, but this is more efficient and concise.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6b780ec90fed846531162eed5e86336e598e33a3
>
>
> Diff: https://reviews.apache.org/r/62604/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 62604: Use a more efficient query for instance ID
collision detection
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62604/#review186360
-----------------------------------------------------------
Master (79ff364) 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 Sept. 27, 2017, 12:25 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62604/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2017, 12:25 a.m.)
>
>
> Review request for Aurora and Jordan Ly.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Came across this while looking at storage access patterns. Not a particularly hot code path, but this is more efficient and concise.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6b780ec90fed846531162eed5e86336e598e33a3
>
>
> Diff: https://reviews.apache.org/r/62604/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 62604: Use a more efficient query for instance ID
collision detection
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62604/
-----------------------------------------------------------
(Updated Sept. 26, 2017, 5:25 p.m.)
Review request for Aurora and Jordan Ly.
Repository: aurora
Description
-------
Came across this while looking at storage access patterns. Not a particularly hot code path, but this is more efficient and concise.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6b780ec90fed846531162eed5e86336e598e33a3
Diff: https://reviews.apache.org/r/62604/diff/2/
Changes: https://reviews.apache.org/r/62604/diff/1-2/
Testing
-------
Thanks,
Bill Farner