Skip to content

msg: add some anonymous connection infrastructure#30223

Merged
liewegas merged 2 commits intoceph:masterfrom
liewegas:wip-msgr-anon
Sep 27, 2019
Merged

msg: add some anonymous connection infrastructure#30223
liewegas merged 2 commits intoceph:masterfrom
liewegas:wip-msgr-anon

Conversation

@liewegas
Copy link
Member

@liewegas liewegas commented Sep 6, 2019

  • allow clients to create multiple outgoing lossy connections to the same server
  • allow servers to accept multiple incoming lossy connections from the same client

The goal is to allow the same client to open multiple connections to the same server. From the server's perspective, we are in lossy mode, there is no reason for the server to register/track connections by client, or ensure that there is only one connection per client: each connection is independent and the server is only every sending client messages via a particular connection handle.

From the client's perspective, this can be very useful. In particular, this enables the MonClient to use an independent Connection for sending 'tell' messages ('ceph tell mon.a') that does not interact with or interfere with the somewhat-complicated connection management that ensures the client is always connected to a mon, that we search for a new mon with multiple parallel connection attempts, etc. The old/existing mon tell code is a kludge that tries to force the automagic connection to connect to just a single mon, but it is only partially effective because it interferes with the existing mon connection management.

@liewegas liewegas added the messenger Issues involving one of the Ceph messenger implementations label Sep 6, 2019
@liewegas liewegas requested a review from rjfd September 12, 2019 21:33
@liewegas
Copy link
Member Author

retest this please

auto av = _filter_addrs(addrs);

if (anon) {
return create_connect(av, type, anon);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we need to also check if an anonymous connection to the same peer already exists, as we do with non-anonymous connections?

@rjfd
Copy link
Contributor

rjfd commented Sep 13, 2019

@liewegas can you give a short summary of the rational for this new type of connection?

@liewegas
Copy link
Member Author

@liewegas can you give a short summary of the rational for this new type of connection?

yep, see updated description!

@liewegas
Copy link
Member Author

retest this please

@gregsfortytwo
Copy link
Member

This makes sense to me. I'm really surprised the cleanup Just Works but I also haven't run through the AsyncMessenger's cleanup; it doesn't break things by commingling the anonymous connections even though they were never registered?

@liewegas
Copy link
Member Author

This makes sense to me. I'm really surprised the cleanup Just Works but I also haven't run through the AsyncMessenger's cleanup; it doesn't break things by commingling the anonymous connections even though they were never registered?

I rechecked the code and it looks like the existing cleanup paths (grep for deleted_conns) is fine for connections not in the normal conns map. And regardless of whether the connection is "registered" or not, the path to getting it deleted is the same: it faults and is lossy. So I think we're good!

If the connection mode is lossy, allow us to open a new connection to
a target, regardless of whether other such connections are already open.
This allows for single-use connections.  If you call this multiple times,
you'll get separate, distinct connections.

We are lucky that the cleanup infrastructure for AsyncMessenger just works
without modification.  :)

Signed-off-by: Sage Weil <sage@redhat.com>
If our policy is server + lossy, we do not need to track our incoming
client connections by address.  First, because it doesn't do us any good.
Second, it is nicer if we don't, because we can allow multiple incoming
connections from the same peer addr.

Update a couple of tests: one doesn't apply any more, and the other needs
a different way of getting the just-accepted con ref.

Signed-off-by: Sage Weil <sage@redhat.com>
Copy link
Member

@gregsfortytwo gregsfortytwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Greg Farnum gfarnum@redhat.com

liewegas added a commit that referenced this pull request Sep 27, 2019
* refs/pull/30223/head:
	msg/async: do not register lossy client connections
	msg/async: allow anonymous client-side connections

Reviewed-by: Greg Farnum <gfarnum@redhat.com>
@liewegas liewegas merged commit c48a29b into ceph:master Sep 27, 2019
@liewegas liewegas deleted the wip-msgr-anon branch September 27, 2019 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

messenger Issues involving one of the Ceph messenger implementations wip-sage-testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants