-
Notifications
You must be signed in to change notification settings - Fork 1.5k
v2.5 work: BacklogPolicy #1912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
v2.5 work: BacklogPolicy #1912
Changes from 155 commits
7eb4775
b3038df
50def47
69d907c
133c4bd
4f4be8b
265c028
f692312
a207d8b
4b5a410
0084522
75545f0
6c8edf6
c86c7d2
076bc00
3353ce2
9f86830
f7b0975
eb9c0c3
7e17689
dc32e0d
6f26239
2d15806
bed33f8
1a343c6
b7c3dff
dc3f9b3
fc8a100
77005e7
7b98bc2
5931685
e830cb0
ed60b39
a79c89d
fa5200c
fb6a135
3bd116b
0ca6c47
ff59515
7fccba9
0ed0c4c
410bf80
b364d47
b280c87
7b42366
784d78a
582e5f1
b8d2636
91339f1
3018442
ade853b
1f946d4
272f1a3
a4425ec
b0fb2a1
b567956
65d6268
990f5e8
38132a3
d552097
fac5a1b
1cb00ff
a2d0943
7fdb45a
85c5a4d
98701c9
377c813
d9c68e1
3f6e030
b63648a
25a7058
a38fac2
148c975
bf9fa07
e4e6d73
62c6b7d
5141cc8
532c01f
78ebf5e
78efb6b
d496348
bc11b96
47f0a12
60b3c42
8e168d9
412f9e8
21d36f0
49a917d
8f5bf58
3d20350
c958320
185c62c
2b57b0b
144c22e
92cecfc
7d7f020
f247980
6ecde2a
f91e4c5
bca9de0
5a6db1c
daa1b9c
f36b6d9
70e1735
a814231
1d4b4ad
5de45a2
64565dd
00f851c
029c2a3
341f532
48127a1
5dbf575
237848f
95b39b8
ab41493
51c927b
20d2d28
814b401
667aa34
cf96bba
a6be64a
42a9267
31b38dc
79e5090
aef7832
8ea3b6d
d9caedb
3fa77a4
04233fe
ff54012
dc26751
84439d4
60d9b80
c6fb569
c0206fa
b484e5f
837a654
b0001ab
1bc971d
907cd20
9c31958
eed3ba0
cd29add
2fe8d13
6f9ae70
d234235
33f52af
6140700
5d0ee35
f88e8e1
2185911
3f694c5
2edf892
221509a
b77d0a3
04ce70b
2c2b0ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
namespace StackExchange.Redis | ||
{ | ||
/// <summary> | ||
/// The backlog policy to use for commands. This policy comes into effect when a connection is unhealthy or unavailable. | ||
/// The policy can choose to backlog commands and wait to try them (within their timeout) against a connection when it comes up, | ||
/// or it could choose to fail fast and throw ASAP. Different apps desire different behaviors with backpressure and how to handle | ||
/// large amounts of load, so this is configurable to optimize the happy path but avoid spiral-of-death queue scenarios for others. | ||
/// </summary> | ||
public class BacklogPolicy | ||
{ | ||
/// <summary> | ||
/// Backlog behavior matching StackExchange.Redis's 2.x line, failing fast and not attempting to queue | ||
/// and retry when a connection is available again. | ||
/// </summary> | ||
public static BacklogPolicy FailFast { get; } = new() | ||
{ | ||
QueueWhileDisconnected = false, | ||
AbortPendingOnConnectionFailure = true, | ||
}; | ||
|
||
/// <summary> | ||
/// Default backlog policy which will allow commands to be issues against an endpoint and queue up. | ||
/// Commands are still subject to their async timeout (which serves as a queue size check). | ||
/// </summary> | ||
public static BacklogPolicy Default { get; } = new() | ||
{ | ||
QueueWhileDisconnected = true, | ||
AbortPendingOnConnectionFailure = false, | ||
}; | ||
|
||
/// <summary> | ||
/// Whether to queue commands while disconnected. | ||
/// True means queue for attempts up until their timeout. | ||
/// False means to fail ASAP and queue nothing. | ||
/// </summary> | ||
public bool QueueWhileDisconnected { get; init; } | ||
|
||
/// <summary> | ||
/// Whether to immediately abandon (with an exception) all pending commands when a connection goes unhealthy. | ||
/// </summary> | ||
public bool AbortPendingOnConnectionFailure { get; init; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -804,15 +804,15 @@ internal void OnHashSlotMoved(int hashSlot, EndPoint old, EndPoint @new) | |
/// <param name="key">The key to get a hash slot ID for.</param> | ||
public int HashSlot(RedisKey key) => ServerSelectionStrategy.HashSlot(key); | ||
|
||
internal ServerEndPoint AnyConnected(ServerType serverType, uint startOffset, RedisCommand command, CommandFlags flags) | ||
internal ServerEndPoint AnyServer(ServerType serverType, uint startOffset, RedisCommand command, CommandFlags flags, bool allowDisconnected) | ||
{ | ||
var tmp = GetServerSnapshot(); | ||
int len = tmp.Length; | ||
ServerEndPoint fallback = null; | ||
for (int i = 0; i < len; i++) | ||
{ | ||
var server = tmp[(int)(((uint)i + startOffset) % len)]; | ||
if (server != null && server.ServerType == serverType && server.IsSelectable(command)) | ||
if (server != null && server.ServerType == serverType && server.IsSelectable(command, allowDisconnected)) | ||
{ | ||
if (server.IsReplica) | ||
{ | ||
|
@@ -2169,6 +2169,12 @@ private bool PrepareToPushMessageToBridge<T>(Message message, ResultProcessor<T> | |
{ | ||
// Infer a server automatically | ||
server = SelectServer(message); | ||
|
||
// If we didn't find one successfully, and we're allowed, queue for any viable server | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I am understand it correctly, I think it will be interesting to test this logic with a clustered cache. In case of a failure, the logic will select any viable server. This viable server might return a moved exception and the message will get redirected to the right endpoint. It should be fine if eventually it gets to the right endpoint. Does that sound correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct! Let's make sure in tests though - I'll add some cluster tests around this to the PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added additional testing in 2c2b0ea! |
||
if (server == null && message != null && RawConfig.BacklogPolicy.QueueWhileDisconnected) | ||
{ | ||
server = ServerSelectionStrategy.Select(message, allowDisconnected: true); | ||
} | ||
} | ||
else // a server was specified; do we trust their choice, though? | ||
{ | ||
|
@@ -2186,7 +2192,9 @@ private bool PrepareToPushMessageToBridge<T>(Message message, ResultProcessor<T> | |
} | ||
break; | ||
} | ||
if (!server.IsConnected) | ||
|
||
// If we're not allowed to queue while disconnected, we'll bomb out below. | ||
if (!server.IsConnected && !RawConfig.BacklogPolicy.QueueWhileDisconnected) | ||
{ | ||
// well, that's no use! | ||
server = null; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this designed for inheritance? if not, recommend
sealed