Buffered protocol changes

[mmaslankaprv]

When the request is buffered in the buffered protocol queue we do not
want to account that time for the overall timeout

Backports Required

• none - not a bug fix
• none - this is a backport
• none - issue does not exist in previous branches
• none - papercut/not impactful enough to backport
• v24.3.x
• v24.2.x
• v24.1.x

Release Notes

• none

[micheleRP]

looks good from docs

[vbotbuildovich]

CI test results

test results on build#60851

test_id	test_kind	job_url	test_status	passed
idempotency_tests_rpunit.idempotency_tests_rpunit	unit	https://buildkite.com/redpanda/redpanda/builds/60851#01946fd0-d399-4794-95da-0a1193035581	FLAKY	1/2
rptest.tests.data_migrations_api_test.DataMigrationsApiTest.test_higher_level_migration_api	ducktape	https://buildkite.com/redpanda/redpanda/builds/60851#01947013-69c9-49a2-811b-31318b51ec2d	FLAKY	5/6

[dotnwat]

When the request is buffered in the buffered protocol queue we do not
want to account that time for the overall timeout

why?

[mmaslankaprv]

When the request is buffered in the buffered protocol queue we do not
want to account that time for the overall timeout

why?

It does more harm than good, in a scenario with overloaded cluster the buffered requests are immediately timing out when sent through RPC layer, this makes the cluster unstable as leaders have more work and followers are unable to receive requests.

[dotnwat]

It does more harm than good, in a scenario with overloaded cluster the buffered requests are immediately timing out when sent through RPC layer, this makes the cluster unstable as leaders have more work and followers are unable to receive requests.

should the timeout be longer then? i mean generally it sounds fragile to just ignore real time that a request is alive, if it is a buffer or not. but maybe i don't fully understand the issue.

[bashtanov]

Otherwise LGTM

[mmaslankaprv]

It does more harm than good, in a scenario with overloaded cluster the buffered requests are immediately timing out when sent through RPC layer, this makes the cluster unstable as leaders have more work and followers are unable to receive requests.

should the timeout be longer then? i mean generally it sounds fragile to just ignore real time that a request is alive, if it is a buffer or not. but maybe i don't fully understand the issue.

It indeed is a buffer, i guess i can adjust the timeout at the caller, what i observed is that this is very fragile. The timeout is there to bound the network latency. This is why is decided to adjust the timeout. There are also situations in which the dispatch loop is sending a requests that has 5 ms left to be timed out. This case is the worst as the request will error out at the requester but it will be sent, this will force the leader to resend the message.

[dotnwat]

The offline discussion I had with Michal made it clearer what this change was doing. The timeout in question is not something chosen as part of initiating a raft replicate request, in which case yes, the buffering time should be accounted for in processing. Rather, this timeout is intended to detect problems with followers or network partitions etc... so it makes sense in this case that buffering time should be used as part of this particular timeout use case.