Transaction conditions are true but transaction fails

[GoogleCodeExporter]

Hi,

I've noticed that sometimes, when running a lot of conditional transactions, 
the output of the Execute() method return false, but none of the conditions 
evaluate to false.

This only happens once in about 100 transactions, so it's difficult to 
reproduce it.
The issue only occurs when the transaction has multiple conditions (the more 
the better).

This is the code I used:

//Create a whole bunch of keys...

for(int i = 0; i < 1000; i++)
{
       using(var tx = conn.CreateTransaction())
       {
              var cond1 = tx.AddCondition(Condition.KeyExists(0, "anExistingKey1");
              var cond2 = tx.AddCondition(Condition.KeyExists(0, "anExistingKey2");
              var cond3 = tx.AddCondition(Condition.KeyExists(0, "anExistingKey3");

              tx.Eval(...);

              var txRes = tx.Execute();

              Assert.IsTrue(cond1);  //--> ok
              Assert.IsTrue(cond2);  //--> ok
              Assert.IsTrue(cond3);  //--> ok
              Assert.IsTrue(txRes);   //--> not ok: false
       }
}

I had a look at the method "MultiMessage.ExecutePreconditions". It looks like 
only the result of the last condition's task is waited for. If that one returns 
false, the transaction fails and "unwatch" is called.

When I change the code to wait for the result of all condition's tasks 
(Task.WaitAll), it works fine.

Is there any guarantee that the last condition sent will contains the first 
task that finishes? It seems to me that there's no way to be sure the 
TaskScheduler finishes that task first.

Am I doing something wrong here?

Gert

Original issue reported on code.google.com by g...@stylelabs.com on 26 Apr 2013 at 1:12

[GoogleCodeExporter]

I will investigate

Original comment by marc.gravell on 26 Apr 2013 at 2:53

[GoogleCodeExporter]

I will investigate

Original comment by marc.gravell on 26 Apr 2013 at 2:53

[GoogleCodeExporter]

I will try to repro - if I could use your *actual* code that reproduces this it 
would be handy, but I have a suspicion the error is not *necessarily* what you 
think it is. In particular, there are *other* reasons a transaction can be 
aborted *even if all the preconditions are met* - specifically, if another 
connection touches any of the WATCH keys for the transaction, then the 
transaction will be aborted. This is simply how redis transactions are 
implemented. The pre-conditions take out WATCH keys to ensure that the 
pre-condition doesn't *become false silently* between the check and the 
execution.

---

For info, I believe the "It looks like only the result of the last condition's 
task is waited for" is a red-herring; the point here is that these tasks are 
known not to have continuations (since they are only used internally to the 
library), so the code uses:

    msg.ForceSync();

on each; this means that the callbacks are executed *in order* directly on the 
reader-thread. So it waits for the last one simply because if the last one has 
a response then they *all* have their responses (order of commands/responses is 
 retained by redis). If you notice, *after* it has waited, it then asks each 
condition separately whether it got the response it wanted:

            foreach (var cond in conditions)
            {
                if (!cond.Validate()) return false;
            }

            return true;

In the case of KeyExists, this is basically querying whether `EXISTS` returns 1 
/ 0.

Original comment by marc.gravell on 26 Apr 2013 at 6:25

[GoogleCodeExporter]

Edit: I have a successful repro - investigating, thanks - definitely something 
wrong here - sorry if I sounded dubious, but I like to think about things 
before diving in!

Original comment by marc.gravell on 26 Apr 2013 at 6:32

[GoogleCodeExporter]

And indeed, the TaskStatus is WaitingForActivation; the real error was 
CreateMessages() in ExistsCondition - should have specified 
ExecuteSynchronously - my mistake, sorry. Fixing.

Original comment by marc.gravell on 26 Apr 2013 at 6:46

[GoogleCodeExporter]

Fixed in 1.3.35

Original comment by marc.gravell on 26 Apr 2013 at 6:53

• Changed state: Fixed