Wip async abstract backend #10264
Merged
Wip async abstract backend #10264
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yuyuyu101
force-pushed
the
wip-async-abstract-backend
branch
7 times, most recently
from
14821a4
to
26c0ff6
Compare
yuyuyu101
force-pushed
the
wip-async-abstract-backend
branch
2 times, most recently
from
8d4daf0
to
1b9ef14
Compare
| @@ -23,6 +23,7 @@ libmsg_la_SOURCES += \ | |||
| msg/async/Event.cc \ | |||
| msg/async/net_handler.cc \ | |||
| msg/async/Stack.cc \ | |||
| msg/async/PosixStack.cc \ | |||
There was a problem hiding this comment.
should squash this change with the previous two commits.
yuyuyu101
force-pushed
the
wip-async-abstract-backend
branch
2 times, most recently
from
be20e88
to
ea92bdb
Compare
| class C_barrier : public EventCallback { | ||
| Mutex barrier_lock; | ||
| Cond barrier_cond; | ||
| atomic_t barrier_count; |
There was a problem hiding this comment.
might want to switch to std::atomic<>?
|
i will look at your changes more closely tmr . |
|
passed http://pulpito.ceph.com/haomai-2016-07-30_09:57:47-rados-wip-haomai-testing-distro-basic-vps most are network problems, others related to http://tracker.ceph.com/issues/15368 . |
| // find worker with least references | ||
| // tempting case is returning on references == 0, but in reality | ||
| // this will happen so rarely that there's no need for special case. | ||
| for (unsigned i = 0; i < num_workers; ++i) { |
There was a problem hiding this comment.
could use std::min_element(), see https://www.sgi.com/tech/stl/min_element.html.
There was a problem hiding this comment.
I will modify this logic later, so let me keep this now.
| @@ -25,11 +25,10 @@ | |||
| #undef dout_prefix | |||
| #define dout_prefix *_dout << "stack " | |||
|
|
|||
| void NetworkStack::add_thread(unsigned i) | |||
| void NetworkStack::add_thread(unsigned i, std::function<void ()> &thread) | |||
There was a problem hiding this comment.
nit, i think this will be RVO'ed.
|
retest this please svp. |
yuyuyu101
force-pushed
the
wip-async-abstract-backend
branch
from
fef99de
to
30cc070
Compare
|
retest this please |
yuyuyu101
force-pushed
the
wip-async-abstract-backend
branch
from
30cc070
to
568bc5b
Compare
| assert(current_best); | ||
| ++current_best->references; | ||
| return current_best; | ||
| } | ||
|
|
||
| void NetworkStack::stop() | ||
| { | ||
| simple_spin_lock(&pool_spin); | ||
| Spinlock::Locker l(pool_spin); |
There was a problem hiding this comment.
could you squash this change into 9922600?
Stack is a network IO framework which encapsulates all necessary basic network interface, then it manages threads to work. Different network backend like posix, dpdk even RDMA need to inherit Stack class to implement necessary interfaces. So it will make ease for other network backend to integrated into ceph. Otherwise, each backend need to implement the whole Messenger logics like reconnect, policy handle, session maintain... Signed-off-by: Haomai Wang <haomai@xsky.com>
Add default posix backend support, it should be the full replacement for the original AsyncMessenger IO logics. Signed-off-by: Haomai Wang <haomai@xsky.com>
Signed-off-by: Haomai Wang <haomai@xsky.com>
1. replace sd to ConnectedSocket 2. Replace WorkerPool with Stack 3. Use Stack worker Signed-off-by: Haomai Wang <haomai@xsky.com>
Signed-off-by: Haomai Wang <haomai@xsky.com>
Now only dispatch external event will wakeup event thread(previously delete_time_event will call wakeup), we only need to use "external_num_events" to indicate whether we have extra events. Signed-off-by: Haomai Wang <haomai@xsky.com>
AsyncMessenger will try to loop the bind port range, so it will produce some addr inuse errors which is not abnormal. Signed-off-by: Haomai Wang <haomai@xsky.com>
New async msgr runtime need to spawn threads when binding, but ceph-osd will call daemon() after binding port. So we need to respawn threads if forked. Then thread spawn delay will increase complexity for this change and it's really a simple strategy which help less, we disable auto spawn now. Signed-off-by: Haomai Wang <haomai@xsky.com>
Because daemon() will termniate all existing threads, it will make something go wrong. So we want to add hook at CephContext, do something before/after fork. Signed-off-by: Haomai Wang <haomai@xsky.com>
Signed-off-by: Haomai Wang <haomai@xsky.com>
If daemonlize we need to respawn event threads, it need to allow set_owner again Signed-off-by: Haomai Wang <haomai@xsky.com>
Signed-off-by: Haomai Wang <haomai@xsky.com>
When the connection is lossy and enter fault, it will dispatch reset event. If cleanup handler is executed as well as ms_handle_reset call mark_down, it may exists racing for "cs". cleanup handler will reset "cs" but _conn_prefix in mark_down will access "cs". Signed-off-by: Haomai Wang <haomai@xsky.com>
Signed-off-by: Haomai Wang <haomai@xsky.com>
In tests we allow to reset EventCenter instance in the same CephContext, so it may let global_centers->centers to set the same position multi times. Signed-off-by: Haomai Wang <haomai@xsky.com>
Otherwise in slow runner, ms_dispatch may not executed when assert Signed-off-by: Haomai Wang <haomai@xsky.com>
yuyuyu101
force-pushed
the
wip-async-abstract-backend
branch
from
568bc5b
to
f6c3fd9
Compare
|
lgtm once jenkins is green. |
|
retest this please |
|
thanks for reviewing! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.