Support external configuration of HTTP-related timeouts when invoking microservice endpoints.

[emetsger]

What does this Pull Request do?

At Johns Hopkins we've found that derivative generation, especially for TIFFs and video files exceeding 500MiB, can exceed the default timeouts of various components in the Islandora stack including: Nginx, PHP-FPM, and Apache HttpClient. Configuring the timeouts of Nginx and PHP-FPM are relatively easy, but unfortunately Alpaca is slightly more challenging.

There are two options that were explored:

• Add additional parameters to the Camel route URIs to configure timeouts
• Wire in a custom HttpClientConfigurer to configure timeouts

The former option would be considered a breaking change: the Alpaca upgrade would not be a drop-in replacement; adopters would need to update their blueprint configurations to supply default values. (AFAICT, the simple EL doesn't provide a way in-line default values).

The latter approach is drop-in. If adopters do not update their blueprint configuration, then the custom HttpClientConfigurer is simply unused. (Note that DerivativeConnectorTest passes without any modifications to it or the blueprint).

What's new?

This PR introduces a custom HttpClientConfigurer (RequestConfigConfigurer) which is used by Camel to create a default instances of RequestConfig, which carries the timeout values described above.

For the Islandora stack, this means updating the Karaf artifacts by amending blueprint.xml to add reasonable default values, and optionally supporting custom values using Karaf cfg files. For example, ISLE users will need to apply something like the following:

--- /Users/esm/workspaces/idc/Alpaca/islandora-connector-derivative/src/test/resources/OSGI-INF/blueprint/blueprint-test.xml	2021-06-07 14:02:50.000000000 -0400
+++ /Users/esm/workspaces/idc/Alpaca/islandora-connector-derivative/src/test/resources/OSGI-INF/blueprint/blueprint-httpconfigurer-test.xml	2021-06-07 15:56:54.000000000 -0400
@@ -16,8 +16,19 @@
     </cm:default-properties>
   </cm:property-placeholder>
 
-  <bean id="http" class="org.apache.camel.component.http4.HttpComponent"/>
-  <bean id="https" class="org.apache.camel.component.http4.HttpComponent"/>
+  <bean id="requestConfigConfigurer" class="ca.islandora.alpaca.connector.derivative.RequestConfigConfigurer">
+    <property name="connectionRequestTimeoutMs" value="10000"/>
+    <property name="connectTimeoutMs" value="10000"/>
+    <property name="socketTimeoutMs" value="10000"/>
+  </bean>
+
+  <bean id="http" class="org.apache.camel.component.http4.HttpComponent">
+    <property name="httpClientConfigurer" ref="requestConfigConfigurer"/>
+  </bean>
+
+  <bean id="https" class="org.apache.camel.component.http4.HttpComponent">
+    <property name="httpClientConfigurer" ref="requestConfigConfigurer"/>
+  </bean>
 
   <camelContext id="IslandoraTriplestoreIndexer" xmlns="http://camel.apache.org/schema/blueprint">
     <package>ca.islandora.alpaca.connector.derivative</package>

Note that ISLE would probably prefer to obtain these values from the environment. How these values are set depends on the runtime environment and use of, e.g. confd, and are considered out of scope for this PR.

How should this be tested?

Run tests and see that they pass. A full end-to-end test (e.g. ingesting a large video file into Islandora and witnessing a service derivative being generated by homarus) may be possible depending on available resources and the alignment of HTTP-related timeouts throughout the stack.

Interested parties

@Islandora/8-x-committers

[codecov]

Codecov Report

Merging #79 (ed34e02) into dev (07d1459) will increase coverage by 0.68%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                dev      #79      +/-   ##
============================================
+ Coverage     90.00%   90.68%   +0.68%     
- Complexity       73       82       +9     
============================================
  Files            10       11       +1     
  Lines           300      322      +22     
  Branches          1        1              
============================================
+ Hits            270      292      +22     
  Misses           29       29              
  Partials          1        1              

Impacted Files	Coverage Δ
.../connector/derivative/RequestConfigConfigurer.java	100.00% <0.00%> (ø)

---

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 07d1459...ed34e02. Read the comment docs.

[manez]

Thank you for this, @emetsger ! Alas, I'm afraid we have one small hoop to clear for it to be mergeable:

Before you set out to contribute code you will need to have completed a Contributor License Agreement or be covered by a Corporate Contributor License Agreement. The signed copy of the license agreement should be sent to mailto:community@islandora.ca

This license is for your protection as a contributor as well as the protection of the Foundation and its users; it does not change your rights to use your own contributions for any other purpose.

[emetsger]

Hi @manez,

I sent a CLA to community@ on June 17, so it should be waiting for you! Let me know if you don't see it and I can re-send.

Thank you for this, @emetsger ! Alas, I'm afraid we have one small hoop to clear for it to be mergeable:

Before you set out to contribute code you will need to have completed a Contributor License Agreement or be covered by a Corporate Contributor License Agreement. The signed copy of the license agreement should be sent to mailto:community@islandora.ca

This license is for your protection as a contributor as well as the protection of the Foundation and its users; it does not change your rights to use your own contributions for any other purpose.

[mjordan]

@emetsger we have it, sorry for the confusion.

[dannylamb]

Sry that slipped through the cracks. I'll try testing now 🚀

This came up during our testing sprint so we'd like to get this in sooner rather than later.

[dannylamb]

This seems to work for me. I'm having a hard time recreating the timeout fails, but I can confirm that deploying new artifacts with these changes work, they do not affect existing routes, and that routes that wish to use them can configuring the http and https beans appropriately.

Big 👍 for the test, too. Let's do this 🚀

[dannylamb]

We'll have to update the blueprints that both the playbook and ISLE deploy to take advantage of the timeouts.