From befa83338513bd8296551c57261ac8d505df9ecc Mon Sep 17 00:00:00 2001
From: Simon Willnauer <simonw@apache.org>
Date: Mon, 7 Apr 2014 22:05:47 +0200
Subject: [PATCH] Make sure successful operations are correct if second phase
 is fast

In TransportSearchTypeAction we  need to increment successful ops
first before we increment and compare the exit condition otherwise if we
are fast we could concurrently update totalOps but then preempt one
of the threads which can cause the successor to read a wrong value from
successfulOps if second phase is very fast ie. searchType == count etc.
This can cause wrong success stats in the search response.
---
 .../action/search/type/TransportSearchTypeAction.java     | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/elasticsearch/action/search/type/TransportSearchTypeAction.java b/src/main/java/org/elasticsearch/action/search/type/TransportSearchTypeAction.java
index 6b5897602b9..54e98ebe8cf 100644
--- a/src/main/java/org/elasticsearch/action/search/type/TransportSearchTypeAction.java
+++ b/src/main/java/org/elasticsearch/action/search/type/TransportSearchTypeAction.java
@@ -246,11 +246,13 @@ public abstract class TransportSearchTypeAction extends TransportAction<SearchRe
         void onFirstPhaseResult(int shardIndex, ShardRouting shard, FirstResult result, ShardIterator shardIt) {
             result.shardTarget(new SearchShardTarget(shard.currentNodeId(), shard.index(), shard.id()));
             processFirstPhaseResult(shardIndex, shard, result);
-
+            // we need to increment successful ops first before we compare the exit condition otherwise if we
+            // are fast we could concurrently update totalOps but then preempt one of the threads which can
+            // cause the successor to read a wrong value from successfulOps if second phase is very fast ie. count etc.
+            successulOps.incrementAndGet();
             // increment all the "future" shards to update the total ops since we some may work and some may not...
             // and when that happens, we break on total ops, so we must maintain them
-            int xTotalOps = totalOps.addAndGet(shardIt.remaining() + 1);
-            successulOps.incrementAndGet();
+            final int xTotalOps = totalOps.addAndGet(shardIt.remaining() + 1);
             if (xTotalOps == expectedTotalOps) {
                 try {
                     innerMoveToSecondPhase();