PYTHON-5215 Add an asyncio.Protocol implementation for KMS

[blink1073]

See benchmark gist.

Benchmark Results:

Before: 4.93s, 5.26s
After: 4.93s, 5.05s

Depends on mongodb-labs/drivers-evergreen-tools#679

[blink1073]

I'm debugging two failures:

test.asynchronous.test_connection_monitoring.AsyncTestCMAP.test_connection_monitoring_pool_clear_interrupting_pending_connections_clear_with_interruptInUseConnections___true_closes_pending_connections
test.asynchronous.test_connection_monitoring.AsyncTestCMAP.test_connection_monitoring_pool_create_min_size_error_error_during_minPoolSize_population_clears_pool

[blink1073]

I realized the benchmark test wasn't actually triggering the protocol -> I'm tweaking things locally

[blink1073]

If we're setting sizehint to be at least 16384 always, is this check worth doing in the first place? I'd expect us to rarely reuse the active buffer since we'll usually have a buffer of size 16384 and a sizehint of 16384.

The actual sizehint in practice was on the order of the bytes being read from the buffer (typically less than 1000). Using the buffered protocol at all here is a bit of a mismatch imho.

[NoahStapp]

If we're setting sizehint to be at least 16384 always, is this check worth doing in the first place? I'd expect us to rarely reuse the active buffer since we'll usually have a buffer of size 16384 and a sizehint of 16384.

The actual sizehint in practice was on the order of the bytes being read from the buffer (typically less than 1000). Using the buffered protocol at all here is a bit of a mismatch imho.

How long would refactoring to not use buffered take? No reason to use the lower-level API if we don't need to.

[blink1073]

How long would refactoring to not use buffered take? No reason to use the lower-level API if we don't need to.

It's actually dead simple, I did it along the way when I was debugging a race condition.

[blink1073]

It's actually dead simple, I did it along the way when I was debugging a race condition.

I'll push a commit in the morning for comparison, we can always revert.

[blink1073]

I'm happy with the simplification. The tests are passing locally, this is ready for another look.

[NoahStapp]

Can you schedule a full Evergreen run? We should ensure there's no regressions introduced here by accident.

Are the benchmark results for KMS significantly different between the two Protocol implementations?

[blink1073]

Full patch build: https://spruce.mongodb.com/version/689f5483e112170007b0ce9f/tasks?sorts=STATUS%3AASC%3BBASE_STATUS%3ADESC

I updated the timings in the PR description, there no significant change.

[blink1073]

Okay there is one legit bug in test.test_connection_logging.TestConnectionLoggingConnectionLogging.test_Connection_checkout_fails_due_to_error_establishing_connection. I'll defer looking at that until next week to focus on greener build tasks.

[blink1073]

Here's a new patch build, failures are existing flakiness issues or tracked in PYTHON-5502.

https://spruce.mongodb.com/version/68a3434b33ee5e00072767c9/tasks?sorts=STATUS%3AASC%3BBASE_STATUS%3ADESC

[NoahStapp]

We don't utilize any of the BaseConnection abstractions for the sync API, correct? The change is purely for compatibility with the async API?

[blink1073]

No it is used by both sync and async KMS, it is what gets returned by the _connect_kms helper function.

[NoahStapp]

I meant that we don't have a separate KMS networking interface for sync. Just wanted to confirm my understanding.

[blink1073]

I had to fix merge conflicts