msgr: half-assed fix to msgr race vs reset issue

diff --git a/src/msg/SimpleMessenger.cc b/src/msg/SimpleMessenger.cc
index ac73f74a0c1ab04c0b3d07110eef325ca3ea58a6..129361d9a9751a60608fb6deb910a8235a6c51b4 100644 (file)
--- a/src/msg/SimpleMessenger.cc
+++ b/src/msg/SimpleMessenger.cc
@@ -798,35 +798,30 @@ int Rank::Pipe::accept()
       }
 
       if (peer_cseq < existing->connect_seq) {
-       if (false &&
-           /*
-            * FIXME: protocol spec is flawed here.  we can't
-            * distinguish between a remote reset or a slow remote
-            * connect race (where the remote connect arrives _after_
-            * our outgoing connection gets a READY reply).
-            *
-            * BUT, this doesn't happen in practice, yet.  the "reset"
-            * case comes up in two situations:
-            *
-            * - mds resets connection to client.  it should _never_
-            * talk to that client after that, unless the client
-            * initiates the connection.
-            *
-            * - mon restarts.  it'll talk to the client.  but, the client
-            * doesn't need the peer_reset calback in that case.  faling into the 
-            * RETRY case is harmless.
-            *
-            * blah!
-            */
-           peer_cseq == 0) {
-         dout(10) << "accept peer reset, then tried to connect to us, replacing" << dendl;
+       /*
+        * in order to distinguish between a slow remote connect race
+        * (where the remote connect arrives _after_ our outgoing
+        * connection gets a READY reply) and a remote reset, we make
+        * the remote node retry a second time with peer_cseq == 0.
+        *
+        * this is a hack.. it's possible to have a second race.  just
+        * rare.  what we really need is a full seq # for remote
+        * connection attempts.. or something along those lines.
+        */
+       if (peer_cseq == 0 && did_zero_retry) {
+         dout(10) << "accept peer reset, then tried to connect to us (2x), replacing" << dendl;
          existing->was_session_reset(); // this resets out_queue, msg_ and connect_seq #'s
          goto replace;
        } else {
          // old attempt, or we sent READY but they didn't get it.
+         if (did_zero_retry) {
+           connect_seq = existing->connect_seq;  // so we can send it below..
+         } else {
+           connect_seq = 0;
+           did_zero_retry = true;
+         }
          dout(10) << "accept existing " << existing << ".cseq " << existing->connect_seq
-                  << " > " << peer_cseq << ", RETRY" << dendl;
-         connect_seq = existing->connect_seq;  // so we can send it below..
+                  << " > " << peer_cseq << ", RETRY " << connect_seq << dendl;
          existing->lock.Unlock();
          rank.lock.Unlock();
          char tag = CEPH_MSGR_TAG_RETRY;
@@ -1061,7 +1056,7 @@ int Rank::Pipe::connect()
        dout(0) << "connect got RETRY, but connection race or something, failing" << dendl;
        goto stop_locked;
       }
-      assert(cseq > connect_seq);
+      assert(cseq >= connect_seq);
       dout(10) << "connect got RETRY " << connect_seq << " -> " << cseq << dendl;
       connect_seq = cseq;
     }

diff --git a/src/msg/SimpleMessenger.h b/src/msg/SimpleMessenger.h
index acaf6dbd24eb05606a920ff4ffcfe274544ea16a..63f7bdf78ee12cea9473783ed9a0419b9ecc2284 100644 (file)
--- a/src/msg/SimpleMessenger.h
+++ b/src/msg/SimpleMessenger.h
@@ -113,6 +113,8 @@ private:
     Mutex lock;
     int state;
 
+    bool did_zero_retry;
+
   protected:
 
     utime_t first_fault;   // time of original failure
@@ -167,7 +169,7 @@ private:
   public:
     Pipe(int st) : 
       sd(-1),
-      state(st), 
+      state(st), did_zero_retry(false),
       reader_running(false), writer_running(false),
       connect_seq(0),
       out_seq(0), in_seq(0), in_seq_acked(0),

diff --git a/src/vstartnew.sh b/src/vstartnew.sh
index 0a981c95027e5f87b5ed45f23392b7c559639365..d6a7c0cb96e684dd147d3db5aa5db45faa205b5d 100755 (executable)
--- a/src/vstartnew.sh
+++ b/src/vstartnew.sh
@@ -27,7 +27,7 @@ $CEPH_BIN/monmaptool --create --clobber --add $IP:12345 --add $IP:12346 --add $I
 for f in 0 1 2
 do
  $CEPH_BIN/mkmonfs --clobber mondata/mon$f --mon $f --monmap .ceph_monmap
- $CEPH_BIN/cmon -d mondata/mon$f --debug_mon 20 --debug_ms 1 --debug_paxos 10
+ $CEPH_BIN/cmon -d mondata/mon$f --debug_mon 20 --debug_ms 20 --debug_paxos 10
 done
 
 # build and inject an initial osd map