Normally we never go from need_addr == false to need_addr == true.
It always starts out as true, so this else is useless on the first
call to Accepter::bind().
The only exception is rebind(). Add an unlearn_addr() that will clear
need_addr. This is almost unnecessary, but doing so fixes a small bug
where the local_connection->peer_addr doesn't get updated when we do a
rebind().
Drop now-unused set_need_addr(). We keep get_need_addr() only because
it is useful in the debug output and for the assert.
Signed-off-by: Sage Weil <sage@inktank.com>
If we 'ceph tell <foo> ...' to a non-monitor, we need to send keepalives to
ensure we detect a tcp drop. (Not so for monitors; monclient already does
its own keepalive thing.)
Signed-off-by: Sage Weil <sage@inktank.com>
send_command() was blocking for the osdmap, and also called from the
connect callback. Instead, re-call it from the handle_osd_map() callback
so that it never blocks.
This was easy to trigger with 'ceph osd tell osd.0 foo' and ms failure
injection.
Signed-off-by: Sage Weil <sage@inktank.com>
Mark our outgoing connection attempt if we send a WAIT in accept(). This
ensures we don't go to standby or closed in fault() on the outgoing
connection for any reason.
Signed-off-by: Sage Weil <sage@inktank.com>
We may have a sequence like:
- client does REQUEST_CLOSE
- mds sends reply
- connection faults, client does get reply
- mds closes out its connection
- client tries to reconnect/resend, gets RESET_SESSION
-> continues lamely waiting
If we get a session reset and we were asking to close the connection,
we are happy--it was closed.
This was exposed with ceph-fuse start/stop tests with socket failure
injection.
Signed-off-by: Sage Weil <sage@inktank.com>
In particular, lossless_peers should use STANDBY, but lossless_clients
should reconnect immediately since they are already doing their own session
management.
Specifically, this fixes the problem where the Client tries to open a
connection to the MDS and faults after delivering its OPEN_SESSION message
but before it gets the reply: the session isn't open yet, so it isn't
pinging. It could, but it is simpler and faster to make the msgr layer
keep the connection open instead of waiting for a periodic keepalive.
Fixes: #2824
Signed-off-by: Sage Weil <sage@inktank.com>
Go directly to the STANDBY state, and print a more accurate message.
Otherwise, we do the same check in writer() and go to STANDBY then. This
is less confusing.
Signed-off-by: Sage Weil <sage@inktank.com>
If we have an active peer whose Connection fails, open a new one. This
is necessary now that a lossy client connection does not automatically
reopen on its own (which is necessary to avoid races with session-based
lossy clients and the ms_handle_reset callback).
Signed-off-by: Sage Weil <sage@inktank.com>
This could null deref if the Pipe is registered but failed.
We need to loop here because the Pipe vs Connection stuff sucks; hopefully
this gets fixed up soonish.
Signed-off-by: Sage Weil <sage@inktank.com>
AFAICS these checks are pointless. There should be no harm in queueing
messages on a closed connection; they'll get cleaned up when it is
deregistered. Moreover, the *queuer* shouldn't be the one who has to
unregister a Pipe.
Signed-off-by: Sage Weil <sage@inktank.com>
There was a race where:
- sending stuff to a lossy Connection
- it fails, and queues itself for reap, queues a RESET event
- reaper clears the Pipe
- some thread queues new messages and the Pipe is reopened, messages sent
- RESET event delivered to dispatch, connection is closed and reopened.
The result was that messages got sent to the OSD out of order during the
window between the fault() and ms_handle_reset() getting called. This will
prevent that.
Signed-off-by: Sage Weil <sage@inktank.com>
This fixes a problem where:
- pipe faults, con->pipe is cleared
- ms_handle_reset tries to mark_down, but it doesn't know the pipe
Signed-off-by: Sage Weil <sage@inktank.com>
When we have a lossy connection failure, immediately disconnect the Pipe
and set the Connection failed flag. There is no reason to wait until the
reaper comes along.
Signed-off-by: Sage Weil <sage@inktank.com>
The locking was awkward with locally delivered messages.. we dropped dq
lock, inq lock, re-took dq lock, etc. We would also take + drop + retake
+ drop the dq lock when queuing events. Blech!
Instead:
* simplify the queueing of cons for the local_queue
* dequeue the con under the original dq lock
* queue events under a single dq lock interval, by telling
local_queue.queue() we already have it.
Signed-off-by: Sage Weil <sage@inktank.com>
We need to know whether the client is lossy before we connect to the peer
in order to know whether to deliver a RESET event or not on connection
failure. Lossy clients get one, lossless do not.
And in any case, we know ahead of time, so we may as well indicate as much
in the Policy.
Signed-off-by: Sage Weil <sage@inktank.com>
The IncomingQueue can live beyond the Pipe. In particular, there is no
reason not to deliver messages we've received on this connection even
though the socket has errored out.
Separate incoming queue discard from outgoing, and only do the latter in
the reaper.
Signed-off-by: Sage Weil <sage@inktank.com>
Use this pointer only for debug output prefix; do not dereference, as we
may live beyond the original parent.
Signed-off-by: Sage Weil <sage@inktank.com>
We change a couple of key things here:
* If there is a matching connect_seq and the existing connection is in OPEN (or
STANDBY; same thing + a failure), we send a RETRY_SESSION and ask the peer to
bump their connect_seq. This handles the case where there was a race, our
end successfully opened, but the peer's racing attempt was slowly processed.
* We always reply with connect_seq + 1. This handles the above case
more cleanly, and lets us use the same code path.
Also avoid duplicating the RETRY_SESSION path with a goto. Beautiful!
Signed-off-by: Sage Weil <sage@inktank.com>
We solve two problems with this patch. The first is that the messenger
will now reuse an existing session's Connection with a new connection,
which means that we don't want to change session->connection when we
are validating an authorizer. Instead, set (but do not change) it.
We also want to avoid a race where:
- mds recovers, replays Sessions with no con's
- multiple connection attempts for the same session race in the msgr
- both are authorized, but out of order
- Session->connection gets set to the losing attempt's Connection*
Instead, we take advantage of an accept event that is called only for
accepted winners.
Signed-off-by: Sage Weil <sage@inktank.com>
fixup
Create a new event type when we successfully accept a connection. This is
distinct from the authorizor verification, which may happen for multiple
racing connection attempts. In contrast, this will only happen on those
that win the race(s). I don't think this is that important for stateless
servers (OSD, MON), but it is important for the MDS to ensure that it keeps
its Session con reference pointing to the most recently-successful
connection attempt.
Signed-off-by: Sage Weil <sage@inktank.com>
This used to be necessary because the pipe_lock was used when queueing
the pipe in the dispatch queue. Now that is handled by IncomingQueue's
own lock, so these can be removed.
By no longer dropping the lock, we eliminate a whole category of potential
hard-to-debug races. (Not that any were observed, but now we dno't need to
worry about them.)
Signed-off-by: Sage Weil <sage@inktank.com>
The DispatchQueue class now completely owns message delivery. This is
cleaner and lets us drop the redundant destination_stopped flag from
msgr (DQ has its own stop flag).
Signed-off-by: Sage Weil <sage@inktank.com>
Looking through git history it is not clear exactly how these checks
came to be. They seem to have grown during the multiple-entity-per-rank
transition a few years back. I'm not fully convinced they are necessary,
but we will keep them regardless.
Push checks into DispatchQueue and look at the local stop flag to
determine whether these events should be queued. This moves us away from
the kludgey SimpleMessenger::destination_stopped flag (which will soon
be removed).
Also move the refcount futzing into the DispatchQueue methods. This makes
the callers much simpler.
Signed-off-by: Sage Weil <sage@inktank.com>
We don't need to worry about racing with shutdown here; the cleanup
procedure will stop the accepter thread before cleaning up all the
pipes.
Signed-off-by: Sage Weil <sage@inktank.com>
