mon: OSDMonitor: do not assume a session exists in send_incremental()

We may not have an open session for a given osd. If we blindly assume we
do, we may end up trying to send incrementals we do not have to the osd.

And then we will crash.

This fixes a regression introduced by

  171fee1b82d2675e364da7f96dfb9dd286d9b6e6

which is meant as a backport of

  de43a02e06650a552f048dc8acd17f255126fed9

but so happens to intruduce a line that wasn't on the original patch. We
imagine it was meant to make the 's->osd_epoch' assignment work without
checking the session, as per the original patch, but the backporter must
have forgotten to also backport the assertion on the not-null session.
The unfortunate introduction of the check for a not-null session
triggered this regression.

The regression itself is due to enforcing that a session exists for the
osd we are sending the incrementals to. However, if we come via the
OSDMonitor::process_failures() path, that may very well not be the case,
as we are handling potentially-old MOSDFailure messages that may no
longer have an associated session. By enforcing the not-null session, we
don't check whether we have the requested versions (i.e., if
our_earliest_version <= requested_version), and thus we end up on the
path that assumes that we DO HAVE all the necessary versions -- when we
may not, thus finally asserting because we are reading blank
incremental versions.

Fixes: #14236
Signed-off-by: Joao Eduardo Luis <joao@suse.de>

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 30f20017cb712867bcc871ba1e51ea834adee4fc..968efc43759836590ce2b5eb70ef34e63583c8ad 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -2148,7 +2148,7 @@ void OSDMonitor::send_incremental(PaxosServiceMessage *req, epoch_t first)
       return;
   }
 
-  if (s && first < get_first_committed()) {
+  if (first < get_first_committed()) {
     first = get_first_committed();
     bufferlist bl;
     int err = get_version_full(first, bl);
@@ -2164,7 +2164,9 @@ void OSDMonitor::send_incremental(PaxosServiceMessage *req, epoch_t first)
     m->maps[first] = bl;
     mon->send_reply(req, m);
 
-    s->osd_epoch = osdmap.get_epoch();
+    if (s) {
+      s->osd_epoch = osdmap.get_epoch();
+    }
     return;
   }