Configure idle keepalive timers to traverse nat

[mgaffigan]

Avoid NAT state timeouts which can cause issues like those quoted in #495. Includes vmoptions prop that can be used to back out the change - no identified reason to do so, since it should not have negative effects. Increased bandwidth use should be negligible (1.7 KB/h/connection vs 58 B/h/connection).

Feature seems to work on linux, mac, windows since 1709. Maintains old behavior on old unsupported versions of windows.

[github-actions]

Test Results

128 files  ±0  128 suites  ±0   2m 58s ⏱️ +20s
729 tests ±0  729 ✅ ±0  0 💤 ±0  0 ❌ ±0 
819 runs  ±0  813 ✅ ±0  6 💤 ±0  0 ❌ ±0 

Results for commit 03bd5e6. ± Comparison against base commit f9ba73e.

[tonygermano]

Thanks for this. Fixing #495 is worthwhile. The usual 2-hour OS keepalive default is far longer than common NAT and load-balancer idle timeouts (Azure LB 4 min, AWS NAT GW 350s), and 120s stays under both. Keepalive is still only turned on where it was before, and jdk.net is available in both the install4j bundle (Adoptium JDK 21) and the Temurin Docker images. I'd like a few changes before merging.

Required changes

1. Rename the property to oie.tcp.keepalive.idle

We aren't adding new options with "mirth" in the name, because it's a trademarked term. oie. is already used by the smoketest harness (oie.baseUrl, oie.db.url, ...). It's short and isn't tied to a Java package namespace, so I'd like it to be the convention for new properties. The rest of the name maps directly to TCP_KEEPIDLE and leaves room for .interval and .count later.

Keeping this a system property is right. I considered mirth.properties, but no plugin has its own key there, and ConfigurationController has no generic accessor for mirth.properties values. Adding a TCP-specific getter to it would put knowledge of the TCP connector into core. WebServiceDispatcher.MAX_REDIRECTS (http.maxRedirects) is the precedent for a connector reading a system property once into a static final.

2. Check support per socket; don't log a stack trace on every connection

Where the JDK/OS doesn't support TCP_KEEPIDLE, setOption throws UnsupportedOperationException. The current code then logs a WARN with a stack trace for every new socket. On a busy listener that floods the log. Windows support for this option arrived in JDK 22 (JDK-8308593), so whether JDK 17/21 on Windows is affected depends on whether the vendor build carries the backport.

Please check socket.supportedOptions().contains(ExtendedSocketOptions.TCP_KEEPIDLE) before calling setOption. It's cheap, it asks the actual socket (so it's correct for any Socket subclass), and it needs no shared mutable state.

Separately, the OS can reject a value that the JDK accepts (e.g. Linux limits TCP_KEEPIDLE to 32767). That case should warn once and log later failures at DEBUG, not log a stack trace per socket. Like the rest of the PR, a failure here should never break the connection.

3. Parse the property once

Integer.parseInt(System.getProperty(...)) currently runs for every connection, so a bad value like 120s logs a stack trace per connection. Please parse it once into a static final, using the default of 120 when the property is unset. For an invalid value, log a single warning that includes the bad value and the default being used. A value of 0 or less keeps meaning "don't set TCP_KEEPIDLE", which is the old behavior.

Suggested shape

This also takes care of the explicit setKeepAlive(false) that the PR removed. Passing the flag in keeps the old behavior exactly, and the call sites become SocketUtil.setKeepAlive(socket, props.isKeepConnectionOpen()) with no if. The dispatcher's server-mode initSocket() passes true, as it does today.

private static final String KEEPALIVE_IDLE_PROPERTY = "oie.tcp.keepalive.idle";
private static final int DEFAULT_KEEPALIVE_IDLE = 120;
private static final int KEEPALIVE_IDLE = readKeepAliveIdle();
private static final AtomicBoolean keepAliveIdleWarned = new AtomicBoolean();

private static int readKeepAliveIdle() {
    String value = System.getProperty(KEEPALIVE_IDLE_PROPERTY);
    if (value == null) {
        return DEFAULT_KEEPALIVE_IDLE;
    }

    try {
        return Integer.parseInt(value.trim());
    } catch (NumberFormatException e) {
        logger.warn("Invalid value '" + value + "' for " + KEEPALIVE_IDLE_PROPERTY + "; using " + DEFAULT_KEEPALIVE_IDLE + " seconds.");
        return DEFAULT_KEEPALIVE_IDLE;
    }
}

public static void setKeepAlive(Socket socket, boolean keepAlive) throws SocketException {
    socket.setKeepAlive(keepAlive);

    if (keepAlive && KEEPALIVE_IDLE > 0 && socket.supportedOptions().contains(ExtendedSocketOptions.TCP_KEEPIDLE)) {
        try {
            socket.setOption(ExtendedSocketOptions.TCP_KEEPIDLE, KEEPALIVE_IDLE);
        } catch (IOException | IllegalArgumentException e) {
            if (keepAliveIdleWarned.compareAndSet(false, true)) {
                logger.warn("Failed to set TCP_KEEPIDLE to " + KEEPALIVE_IDLE + " seconds; using the OS default.", e);
            } else {
                logger.debug("Failed to set TCP_KEEPIDLE to " + KEEPALIVE_IDLE + " seconds.", e);
            }
        }
    }
}

Suggestions (non-blocking)

4. Release notes. Every existing install gets a 120s keepalive idle with no opt-in. I agree with the default, but the release notes should say so and explain how to turn it off.
5. Documentation. Please add a commented example to server/conf/custom.vmoptions, e.g. #-Doie.tcp.keepalive.idle=120, with a note that the value is in seconds and that 0 or less uses the OS default.
6. Commit message. It's currently just the title plus Signed-off-by. Please add a body that references #495 and describes the property, its unit, and the new default. It should also say why this is a system property and not a mirth.properties setting (see item 1), so the reasoning is in the history for anyone who later wonders why. In the title, "nat" should be "NAT".
7. Test. A small Linux-only unit test would be cheap: run setKeepAlive(socket, true) on an unconnected Socket, then check getKeepAlive() and getOption(ExtendedSocketOptions.TCP_KEEPIDLE) == 120. Also check that setKeepAlive(socket, false)` leaves keepalive off.

Out of scope

A per-connector keepalive setting could come later if anyone needs it. A global property is fine for now.

---

🤖 Drafted with Claude Code

[mgaffigan]

@tonygermano, I do not plan on making further changes. Anyone is welcome to take over the PR if they are required. Or feel free to close.