conveyal / analysis-backend

Server component of Conveyal Analysis
http://conveyal.com/analysis
MIT License
23 stars 12 forks source link

Migrate EC2 Launcher to AWS SDK v2.x #228

Closed abyrd closed 5 years ago

abyrd commented 5 years ago

Older versions of the AWS SDK don't have the R5XLarge enum value for the instance type we're now using. We could bump to 1.11.584 to keep the old programming API but might as well start migrating to the 2.x SDK. The main difference for our purposes is that it uses builders to create immutable instances.

codecov-io commented 5 years ago

Codecov Report

Merging #228 into dev will increase coverage by 0.18%. The diff coverage is 2.7%.

Impacted file tree graph

@@             Coverage Diff             @@
##               dev     #228      +/-   ##
===========================================
+ Coverage     23.2%   23.39%   +0.18%     
- Complexity     104      106       +2     
===========================================
  Files           62       63       +1     
  Lines         2422     2475      +53     
  Branches       220      226       +6     
===========================================
+ Hits           562      579      +17     
- Misses        1825     1859      +34     
- Partials        35       37       +2
Impacted Files Coverage Δ Complexity Δ
...l/taui/controllers/RegionalAnalysisController.java 9.21% <0%> (ø) 2 <0> (ø) :arrow_down:
.../conveyal/taui/analysis/broker/RedeliveryTest.java 0% <0%> (ø) 0 <0> (ø) :arrow_down:
...om/conveyal/taui/controllers/BrokerController.java 19.46% <0%> (-0.18%) 6 <0> (ø)
...rc/main/java/com/conveyal/taui/AnalysisServer.java 41.53% <0%> (-4.34%) 5 <0> (ø)
...com/conveyal/taui/analysis/broker/EC2Launcher.java 0% <0%> (ø) 0 <0> (ø) :arrow_down:
.../taui/analysis/broker/EC2RequestConfiguration.java 0% <0%> (ø) 0 <0> (ø) :arrow_down:
.../com/conveyal/taui/analysis/broker/WorkerTags.java 0% <0%> (ø) 0 <0> (?)
...in/java/com/conveyal/taui/analysis/broker/Job.java 0% <0%> (ø) 0 <0> (ø) :arrow_down:
...java/com/conveyal/taui/analysis/broker/Broker.java 15.07% <16.66%> (ø) 4 <0> (ø) :arrow_down:
...n/java/com/conveyal/taui/AnalysisServerConfig.java 75% <33.33%> (-3%) 3 <0> (ø)
... and 3 more

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 23befd1...97b0375. Read the comment docs.

abyrd commented 5 years ago

Once up and running on staging, we saw the following exception:

Caused by: java.lang.invoke.LambdaConversionException: Invalid receiver type interface org.apache.http.Header; not a subtype of implementation type interface org.apache.http.NameValuePair
        at java.lang.invoke.AbstractValidatingLambdaMetafactory.validateMetafactoryArgs(AbstractValidatingLambdaMetafactory.java:233)
        at java.lang.invoke.LambdaMetafactory.metafactory(LambdaMetafactory.java:303)

This was due to a version mismatch in Maven dependencies on Apache httpclient. The new AWS SDK depended on 4.5.6, but we were declaring a dependency in the analysis-backend POM on an older version, and some other dependencies also had transitive dependencies on older versions of httpclient that were closer to the root of the dependency tree and won according to Maven priority rules. Changing our POM's dependency to 4.5.6 effectively forces this version and seems to solve the problem in testing.

Just to be sure, as part of the final testing before deployment to production I will perform the following test: