Honor select disposition in transactions #2322
Conversation
| @@ -117,20 +117,23 @@ private void QueueMessage(Message message) | |||
| { | |||
| (_pending ??= new List<QueuedMessage>()).Add(queued); | |||
|
|
|||
| switch (message.Command) | |||
There was a problem hiding this comment.
can we move the test inside the switch? I'd rather not execute the IsAvailable check etc unless we think there's a good need; other than that: makes sense
There was a problem hiding this comment.
not related to this change, but: unless I'm misreading the code, it looks like we'd issue this even on "cluster"; open question: should we also add a single-DB check? we usually know the DB count, either from server metadata (when available) or from the server type (note: DB count can be unknown)
There was a problem hiding this comment.
sounds good wrt to rearranging things.
WRT preventing an issue with cluster - what do you think of this new approach?
|
Dropping a reminder: Release notes please! Happy to add when ready at the end too, just making sure we remember :) |
sorry @NickCraver - added :) |
|
The more I look at this, the more I wonder whether this RediaTransaction might not need to do anything here, if we can refactor that linked code above a bit to handle sub-messages. I can look on Monday
|
I see - it does seem a tad bit out of place. The code does intersect there to set the database to dirty I think - so it might just work if we ripped this bit out. |
mgravell
deleted the
slorello89/honor-select-disposition-transactions
branch
Fixes #2321
For transactions, if you remove
SELECTfrom the command map, it will still try to send one if you issue an unknown/eval/evalsha command - this adds a bit of logic when enqueuing messages to interrogate the multiplexer to see if SELECT is in the command map.