Skip to content
This repository has been archived by the owner on Nov 11, 2020. It is now read-only.

EC2 Enhancements #201

Merged
merged 15 commits into from
Jan 8, 2019
Merged

EC2 Enhancements #201

merged 15 commits into from
Jan 8, 2019

Conversation

ansoncfit
Copy link
Member

@ansoncfit ansoncfit commented Dec 18, 2018

Questions

  • Refactor overloaded createWorkersInCategory?
  • What should be synchronized (note FIXME in BrokerController)
  • Is the try/catch block in BrokerController unnecessary?
  • Should we just automatically add spot workers after the first few regional tasks are complete?

@ansoncfit ansoncfit requested a review from abyrd December 18, 2018 22:20
@codecov-io
Copy link

codecov-io commented Dec 18, 2018

Codecov Report

Merging #201 into dev will decrease coverage by 0.61%.
The diff coverage is 3.3%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #201      +/-   ##
============================================
- Coverage     24.35%   23.73%   -0.62%     
  Complexity      103      103              
============================================
  Files            59       61       +2     
  Lines          2275     2330      +55     
  Branches        206      211       +5     
============================================
- Hits            554      553       -1     
- Misses         1687     1745      +58     
+ Partials         34       32       -2
Impacted Files Coverage Δ Complexity Δ
...m/conveyal/taui/analysis/broker/WorkerCatalog.java 25% <0%> (-0.65%) 2 <0> (ø)
...com/conveyal/taui/analysis/broker/EC2Launcher.java 0% <0%> (ø) 0 <0> (?)
.../taui/analysis/broker/EC2RequestConfiguration.java 0% <0%> (ø) 0 <0> (?)
...in/java/com/conveyal/taui/analysis/broker/Job.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...om/conveyal/taui/controllers/BrokerController.java 21.78% <0%> (ø) 6 <0> (ø) ⬇️
...java/com/conveyal/taui/analysis/broker/Broker.java 15.32% <14.28%> (+2.82%) 4 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2366db0...aa561f6. Read the comment docs.

@ansoncfit
Copy link
Member Author

@abyrd This should be ready for review. Our decisions on the questions above have been incorporated, and testing on staging has gone well.

Also, I wonder if using the synchronizedMap will resolve #172

@ansoncfit
Copy link
Member Author

To answer the last question: yes, these changes now include having the broker automatically start spot instances after receiving a few regional tasks. Note TARGET_TASKS_PER_WORKER and AUTO_START_SPOT_INSTANCES_AT_TASK.

@ansoncfit ansoncfit merged commit 4133422 into dev Jan 8, 2019
@ansoncfit ansoncfit deleted the spot-requests branch January 8, 2019 02:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants