net: KeepAlive should be backward compatible in 1.23

[jpalermo]

Go version

1.22 and 1.23

Output of go env in your module/workspace:

n/a

What did you do?

https://go.dev/doc/go1.23#netpkgnet

We upgrade our tests to use golang 1.23 and started getting failures because our assertions that the KeepAlive for a connection we create are the same as what we configure in our DialContext started failing.

The KeepAliveConfig addition in 1.23 is great, but the fact that KeepAlive is now ignored in 1.23 is problematic.

For library authors that are trying to support 1.22 as a minimum version, they can't use the new KeepAliveConfig, but if they continue to use KeepAlive consumers importing their package and using 1.23 will not have the correct behavior.

What did you see happen?

KeepAlive on a DialContext in 1.23 seems to be ignored

What did you expect to see?

KeepAlive should still work in 1.23, and if both KeepAlive and KeepAliveConfig are specified, KeepAliveConfig is used.

[gabyhelp]

Related Issues and Documentation

• net: DialTCP doesn't set TCP keepalive (unlike Dialer.Dial) #49345 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[seankhliao]

please include a test that reproduces the issue

[ianlancetaylor]

Yes, it would be very helpful to see a test. The package is intended to work as you describe: if KeepAliveConfig is not set, then KeepAlive is used. Please give us more details about what is going wrong. Thanks.

[jpalermo]

Looks like I was wrong, and just misunderstanding a different part of this change.

A colleague pointed me at the original issue #62254 which had some additional details that didn't make it into the release notes.

Our test was fairly low level and was doing a syscall against a TCP socket and checking the value of syscall.TCP_KEEPINTVL.

Part of the #62254 work was changing KeepAlive to instead set syscall.TCP_KEEPIDLE.

So it is probably at least mostly identical behaviorally, and fairly simple to update or tests to check for this different property that's being set now instead.

Sorry for the noise and thanks for the help. I'll go ahead and close this.

Here's the original failing test if anybody is interested in details.

[ianlancetaylor]

Thanks for following up.

[meandercloud-z]

We upgraded to go1.23.1 to utilize net.KeepAliveConfig for changing TCP keep-alive probe count, which we couldn't do at the go process level before and is a great change.

We noticed the same change in behavior for SetKeepAlivePeriod as mentioned here, where:

• in go1.22.2, SetKeepAlivePeriod sets both TCP_KEEPINTVL and TCP_KEEPALIVE.
• in go1.23.1, SetKeepAlivePeriod sets only TCP_KEEPALIVE.
  If I understand correctly, this means that users who had called SetKeepAlivePeriod before go1.23.1, after they upgrade, probe idle send time would still follow the user provided duration, however the probe send interval would change back to the default 15 seconds.

While I see this was one of the proposed changes in #62254 and understand the author's rationale where a user who sets SetKeepAlivePeriod would mostly expect the connection to terminate within a timeframe relatable to the duration that is explicitly set, other users like myself had intended to use SetKeepAlivePeriod for reducing probe interval, e.g. to conserve client device battery. For me, this is a subtle change in behavior and would seem like a regression without prior knowledge of the above proposal, so documenting this observation here in case others run into this issue.

[ianlancetaylor]

@panjf2000 We should expand the release notes to describe these kinds of changes. Would you able to write a patch for golang.org/x/website/_content/doc/go1.23.md? Thanks.

[panjf2000]

Sure, I'll send a patch updating the release notes of go1.23 later.