Scripted CREST connector: every operation opens new TCP/TLS connections and takes a 401 round trip, and query results are handled on the HTTP I/O thread #163

Summary

ScriptedCRESTConnector sends every request with Connection: close, so it never reuses a connection. With the default defaultAuthMethod = BASIC, every operation also gets a 401 first and repeats the request on a second new connection. One getObject therefore costs two TCP connections, two HTTP exchanges and, over HTTPS, two TLS handshakes. Over HTTPS on loopback, getObject took 3–4× as long as with keep-alive and preemptive auth (see Reproducer). Over a real network, each extra connection also adds round trips.

Query results have a second problem. The whole response body is buffered in memory before parsing starts. Parsing and the script's result handler then run on an I/O dispatcher thread of the async HTTP client. A script that makes a synchronous CREST call from inside a result handler hangs as soon as that call needs a connection on the same dispatcher. In the reproducer, with the default BASIC, this happened after the 3rd nested call.

Line references are to 1abfe74. The counts and timings come from a throwaway harness; everything else comes from reading the code.

1. Connection: close on every request

AbstractRemoteConnection.convert ends with (L502):

rq.setHeader(HttpHeaders.CONNECTION, "close, TE");

The server closes the connection after each response. The default customizer configures a pooling connection manager (200 connections in total, 50 for the configured host, 20 for any other route, L71-L80), but no connection ever comes back to it for reuse. Every request pays a TCP handshake and, over HTTPS, a full TLS handshake.

The header was added in f2c64c27 (2014, "OPENIDM-2604 CR-5689 Fix the ScriptedCREST sample, the connection to OpenDJ occasionally times out"), and it is the only change in that commit. Removing it therefore needs a replacement for whatever it worked around (see section 3 and Proposed fix).

2. A 401 round trip on every operation with the default BASIC auth

defaultAuthMethod defaults to BASIC (L74). For BASIC, the default customizer sets credentials but no AuthCache (L154-L167). ScriptedCRESTConfiguration.execute creates a fresh HttpClientContext for every request (L187). beforeRequest copies propertyBag[AUTH_CACHE] into it (L182-L186), but that entry is set only for BASIC_PREEMPTIVE (L146-L152). For BASIC it is null.

Every request is therefore sent without credentials, gets a 401, and is repeated with credentials. With Connection: close, the repeat goes over a second new connection. The client caches the successful auth scheme only in the per-request context, and that context is then discarded.

3. Query results are buffered in full and handled on the I/O dispatcher thread

AbstractJsonValueResponseHandler copies the whole response body into a SimpleInputBuffer (L580-L599). Parsing starts in buildResult (L660-L665), only after the last byte has arrived. StreamingJsonSlurper hands the results to the handler one at a time, but by then the whole body is already on the heap. A query without paging holds the entire result set as bytes; the objects are built and handed over one at a time.

The async client calls buildResult on the I/O dispatcher thread that owns the connection. So parseQueryResponse (L127-L143) and the script's handleResource run on that thread. The ICF ResultsHandler runs there too when producerBufferSize is 0. With the default of 100, the framework's BufferedResultsProxy hands results to the caller's thread through a bounded queue (BufferedResultsProxy.java#L87-L93), but the dispatcher still blocks once that queue is full. In the reproducer (producerBufferSize = 0), the ICF handler ran on I/O dispatcher 8 while the caller waited on main. Two consequences:

• While a handler is busy, the dispatcher serves no other connection assigned to it.
• A synchronous CREST call made from inside the script's handler, or from an ICF handler when producerBufferSize is 0 or the buffer is full, can deadlock:
  • The calling thread is the dispatcher itself. Every synchronous method of AbstractRemoteConnection blocks in promise.getOrThrow(), e.g. query (L211-L224) and read (L331-L335).
  • New connections are assigned to dispatchers round-robin. Once the nested call's connection lands on the blocked dispatcher, nothing processes it.
  • getOrThrow() has no timeout, and the default operation timeout is NO_TIMEOUT (APIConfigurationImpl.java#L194-L198), so the call waits forever.
  • With Connection: close, every nested call opens a new connection, so the hang comes within at most ioThreadCount nested calls. With keep-alive, a single-threaded nested loop reuses one pooled connection and completes. It can still hang whenever the pool has to open a new connection while a dispatcher is blocked, because every ioThreadCount-th new connection lands on the blocked one.

The shipped crest_sample/SyncDJScript.groovy makes exactly such a call: connection.read inside handleResource (L110). (As shipped, it fails before it sends any request; see #164.) With keep-alive, an occasional hang like this would look like a hang or, under a caller-side timeout, like "the connection to OpenDJ occasionally times out". I have not confirmed that this was the 2014 cause.

StreamingJsonSlurper also ignores the value returned by the handler (L124-L126). A false from handleResource does not stop the parse, so a query cannot end early.

Reproducer

The harness uses the same setup as RESTTestBase and the CREST test environment:

• Jetty 11 in-process with BasicAuthenticator and the CREST memory backend from TestHttpApplication, served over HTTPS with a self-signed certificate.
• ScriptedCRESTConnector with the default customizer and defaultAuthMethod set to BASIC or BASIC_PREEMPTIVE.
• TCP connections are counted with Jetty ConnectionStatistics; HTTP exchanges and 401 responses with an HttpChannel.Listener.
• "Keep-alive" means the same AbstractRemoteConnection with L502 removed, placed first on the classpath.

Per-operation cost. Create one user, run 50 getObject calls to warm up, then count and time 1,000 getObject calls. Results at 1abfe74 (JDK 26, macOS, 8 CPUs). Timings are from two runs each and vary by 30–50% between sessions; a later re-run of three rows gave 18.5, 11.3 and 4.4 ms, in the same order. The counts do not vary.

Over plain HTTP on loopback, the counts are the same but the timing differences are within noise (3.9–5.1 ms per call), because a loopback TCP handshake is cheap. The cost of the extra connections grows with TLS and with the round-trip time to the server.

Nested call from a result handler. Create 40 users and run a search that returns them, over plain HTTP. The ICF ResultsHandler calls getObject for each result. With producerBufferSize = 0, the handler runs on the same thread as a script's handleResource, which is where SyncDJScript makes its nested call. The machine has 8 CPUs, so the client has 8 I/O dispatchers. The CREST_SAMPLE test configuration uses BASIC_PREEMPTIVE (config.groovy#L119), which corresponds to the second row.

In the hung runs, a thread dump taken after 30 s shows one I/O dispatcher thread WAITING in PromiseImpl.getOrThrow, called from AbstractRemoteConnection.

Proposed fix

1. Remove Connection: close. Stale pooled connections are presumably what the 2014 change worked around, so cover them explicitly instead:
   • set connect, socket and connection-request timeouts in the default customizer (all three are commented out today, L84-L93);
   • close idle and expired pooled connections periodically.
2. Stop paying a 401 on every operation with BASIC. Either share one AuthCache across requests, filled after the first successful challenge, or make BASIC_PREEMPTIVE the default. BASIC_PREEMPTIVE already shares one BasicAuthCache across all requests. BasicAuthCache is thread-safe only from HttpClient 4.4, and the module compiles against 4.3.5, so on older runtimes a shared cache needs synchronisation.
3. Do not run result handling on the I/O dispatcher:
   • The response consumer should only collect the body, or expose it as a stream.
   • query() should parse it and call the handler on the caller's thread. queryAsync then has to chain parsing onto the returned promise, so its callers still receive results.
   • This removes the nested-call deadlock and keeps a slow handler from stalling the other connections on its dispatcher.
   • Streaming the body to the caller's thread instead of buffering it would also bound memory for large unpaged queries.
4. Stop parsing and release the response when the handler returns false.

ScriptedCRESTConnectorTest already runs against Jetty with BASIC. Counting connections and 401 responses per operation there would pin items 1 and 2.

Related observations (not covered by the fix above)

• The default REST customizer has the same BASIC setup without an AuthCache (scriptedrest/CustomizerScript.groovy#L97-L110). A script that sends requests through connection without its own context gets a 401 on each request. I did not measure this, because it depends on how the scripts call the client.