conn pool: default enable runtime feature conn_pool_delete_when_idle

[ggreenway]

Signed-off-by: Greg Greenway ggreenway@apple.com

Commit Message: This enables the new behavior (clean up conn pools when they're idle, to avoid leaking memory in some configurations) from #17403 by default. It can still be disabled by setting runtime feature envoy.reloadable_features.conn_pool_delete_when_idle to false.
Additional Description:
Risk Level: Medium
Testing: Test coverage was added in #17403.
Docs Changes:
Release Notes: Added in #17403
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

[alyssawilk]

have we smoke tested somewhere yet?

[ggreenway]

have we smoke tested somewhere yet?

It's still the same code (minus one possible crash when a cluster is removed via CDS) that @rgs1 smoke tested awhile ago.

[rgs1]

have we smoke tested somewhere yet?

It's still the same code (minus one possible crash when a cluster is removed via CDS) that @rgs1 smoke tested awhile ago.

... tested with the new tcp conn pool, whereas the additional crashers were with the old pool fwiw ...

[alyssawilk]

Ah cool, didn't realize the prior version had been canaried.
Just to check my memory, the folks encountering tcp proxy crashes didn't provide additional data, and agreed they should switch back to the new pool in any case right? If so LGTM-as-long-as-you-cc-them because it's as safe as it's going to get (folks shouldn't be using the old pool without informing us the new one is problematic)

[ggreenway]

@bianpengyuan FYI this change, that you reported a crash in #16948, is being reintroduced.

Looking at that report again, it's very possible that it was the same crash fixed in #17522. Not enough information to know for sure, but it's a possible match, so it may be fixed.

[ggreenway]

coverage test flake; unrelated:

2021-08-03T20:04:16.6349000Z test/extensions/transport_sockets/starttls/starttls_integration_test.cc:329: Failure
2021-08-03T20:04:16.6350199Z Value of: test_server_->server().listenerManager().numConnections()
2021-08-03T20:04:16.6350878Z Expected: is equal to 1
2021-08-03T20:04:16.6351556Z   Actual: 0 (of type unsigned long)
2021-08-03T20:04:16.6352267Z Stack trace:
2021-08-03T20:04:16.6352891Z   0x454827: (unknown)
2021-08-03T20:04:16.6353611Z   0x7f6ad1696d96: testing::internal::HandleSehExceptionsInMethodIfSupported<>()
2021-08-03T20:04:16.6354475Z   0x7f6ad167b701: testing::internal::HandleExceptionsInMethodIfSupported<>()
2021-08-03T20:04:16.6355200Z   0x7f6ad1663042: testing::Test::Run()
2021-08-03T20:04:16.6355864Z   0x7f6ad1663b58: testing::TestInfo::Run()
2021-08-03T20:04:16.6356462Z ... Google Test internal frames ...```

/retest

[repokitteh-read-only]

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17577 (comment) was created by @ggreenway.

see: more, trace.

[ggreenway]

/retest

[repokitteh-read-only]

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17577 (comment) was created by @ggreenway.

see: more, trace.