Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

osd: optimize send_message to peers #30968

Merged
merged 6 commits into from Nov 19, 2019

Conversation

majianpeng
Copy link
Member

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs

@majianpeng
Copy link
Member Author

send_message_osd_cluster is a hot fun which used by MOSDRepOP or MOSDECSubOpRead or MOSDSubOpWrite. I did some optimization for this function.

@majianpeng
Copy link
Member Author

jenkins retest this please

@majianpeng
Copy link
Member Author

retest this please

@majianpeng
Copy link
Member Author

@liewegas please review ! Thanks

Copy link
Member

@liewegas liewegas left a comment

Choose a reason for hiding this comment

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

these changes look good!

@@ -110,6 +110,7 @@ class AsyncConnection : public Connection {
AsyncConnection(CephContext *cct, AsyncMessenger *m, DispatchQueue *q,
Worker *w, bool is_msgr2, bool local);
~AsyncConnection() override;
bool unregisted = false;
Copy link
Member

Choose a reason for hiding this comment

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

nit: unregistered

@@ -813,7 +813,7 @@ bool ECBackend::_handle_message(
handle_sub_read(op->op.from, op->op, &(reply->op), _op->pg_trace);
reply->trace = _op->pg_trace;
get_parent()->send_message_osd_cluster(
Copy link
Member

Choose a reason for hiding this comment

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

I think an even better improvement here would be op->op->get_connection()->send_message(...)

Copy link
Member Author

Choose a reason for hiding this comment

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

In ECBackend/ReplicatedBackend, it always use get_parent()->send_message_osd_cluster(). In order to be consistent, still unchanged. But we change PrimaryLogPG::send_message_osd_cluster into inline func to get the same aim.
I added this patch: 693c9a0

@liewegas liewegas changed the title Osd optimze send message osd cluster osd: optimize send_message to peers Oct 22, 2019
@tchaikov
Copy link
Contributor

jenkins test make check

@majianpeng majianpeng force-pushed the osd-optimze-send_message_osd_cluster branch from 31794f8 to 693c9a0 Compare October 23, 2019 14:10
@majianpeng
Copy link
Member Author

jenkins test make check

@majianpeng
Copy link
Member Author

retest this please

@tchaikov
Copy link
Contributor

132/186 Test  #15: smoke.sh ................................***Failed  176.97 sec

jenkins test make check

@majianpeng
Copy link
Member Author

@tchaikov . why always failed? because my pr or?

@tchaikov
Copy link
Contributor

136/183 Test  #15: smoke.sh ................................***Failed  483.46 sec

jenkins test make check arm64

@tchaikov
Copy link
Contributor

@majianpeng no idea. probably you could try to run smoke.sh in your testbed?

@majianpeng
Copy link
Member Author

@tchaikov .i'll try in my host

@majianpeng
Copy link
Member Author

@tchaikov , in my test, w/o this pr smoke.sh still failed.

@majianpeng
Copy link
Member Author

@tchaikov. omit my previous message. this bug caused by 97716c1.

@majianpeng majianpeng force-pushed the osd-optimze-send_message_osd_cluster branch from 693c9a0 to 1f8d9f6 Compare October 25, 2019 02:38
@majianpeng
Copy link
Member Author

@tchaikov . fix bug and pass all checks.

@liewegas
Copy link
Member

Seeing lots of failures like this:

2019-10-28T03:23:17.527 INFO:tasks.ceph.osd.4.smithi203.stderr:*** Caught signal (Segmentation fault) **
2019-10-28T03:23:17.528 INFO:tasks.ceph.osd.4.smithi203.stderr: in thread 7f9f22622700 thread_name:safe_timer
2019-10-28T03:23:17.530 INFO:tasks.ceph.osd.4.smithi203.stderr: ceph version 15.0.0-6532-g37ccb12 (37ccb123c3fc8f973766084a4e44b4fbf16df8bf) octopus (dev)
2019-10-28T03:23:17.530 INFO:tasks.ceph.osd.4.smithi203.stderr: 1: (()+0xf630) [0x7f9f2ee78630]
2019-10-28T03:23:17.530 INFO:tasks.ceph.osd.4.smithi203.stderr: 2: (AsyncConnection::send_message(Message*)+0x201) [0x55e3269087b1]
2019-10-28T03:23:17.530 INFO:tasks.ceph.osd.4.smithi203.stderr: 3: (OSDService::send_message_osd_cluster(std::vector, std::allocator > >&, unsigned int)+0xec) [0x55e32601ff6c]
2019-10-28T03:23:17.530 INFO:tasks.ceph.osd.4.smithi203.stderr: 4: (PG::scrub_reserve_replicas()+0x2e2) [0x55e3260cbff2]
2019-10-28T03:23:17.531 INFO:tasks.ceph.osd.4.smithi203.stderr: 5: (PG::sched_scrub()+0x592) [0x55e3260cc982]
2019-10-28T03:23:17.531 INFO:tasks.ceph.osd.4.smithi203.stderr: 6: (OSD::sched_scrub()+0x4fc) [0x55e32602443c]
2019-10-28T03:23:17.531 INFO:tasks.ceph.osd.4.smithi203.stderr: 7: (OSD::tick_without_osd_lock()+0x650) [0x55e3260323c0]
2019-10-28T03:23:17.531 INFO:tasks.ceph.osd.4.smithi203.stderr: 8: (Context::complete(int)+0x9) [0x55e3260624a9]
2019-10-28T03:23:17.531 INFO:tasks.ceph.osd.4.smithi203.stderr: 9: (SafeTimer::timer_thread()+0x1a8) [0x55e32660b268]
2019-10-28T03:23:17.531 INFO:tasks.ceph.osd.4.smithi203.stderr: 10: (SafeTimerThread::entry()+0xd) [0x55e32660c65d]
2019-10-28T03:23:17.531 INFO:tasks.ceph.osd.4.smithi203.stderr: 11: (()+0x7ea5) [0x7f9f2ee70ea5]
2019-10-28T03:23:17.532 INFO:tasks.ceph.osd.4.smithi203.stderr: 12: (clone()+0x6d) [0x7f9f2dd348cd]
2019-10-28T03:23:17.532 INFO:tasks.ceph.osd.4.smithi203.stderr:2019-10-28T03:23:17.533+0000 7f9f22622700 -1 *** Caught signal (Segmentation fault) **

/a/sage-2019-10-28_02:58:26-rados-wip-sage-testing-2019-10-27-1006-distro-basic-smithi/4450173

lots of others (with coredumps etc) in that same test run.

@majianpeng majianpeng force-pushed the osd-optimze-send_message_osd_cluster branch from 1f8d9f6 to 04e80a8 Compare October 29, 2019 12:23
Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
@majianpeng
Copy link
Member Author

retest this please

@majianpeng
Copy link
Member Author

@liewegas . i found the reason and fixed.

@liewegas
Copy link
Member

liewegas commented Nov 6, 2019

2019-11-05T20:52:33.898 INFO:tasks.ceph.osd.4.smithi134.stderr: ceph version 15.0.0-6875-g6fbd5b8 (6fbd5b8f6235494ccd5d67cf76110c824622eacb) octopus (dev)
2019-11-05T20:52:33.898 INFO:tasks.ceph.osd.4.smithi134.stderr: 1: (()+0x12890) [0x7fa4a36b4890]
2019-11-05T20:52:33.898 INFO:tasks.ceph.osd.4.smithi134.stderr: 2: (AsyncConnection::send_message(Message*)+0x492) [0x5638cf4c88d2]
2019-11-05T20:52:33.899 INFO:tasks.ceph.osd.4.smithi134.stderr: 3: (OSDService::send_message_osd_cluster(std::vector<std::pair<int, Message*>, std::allocator<std::pair<int, Message*> > >&, unsigned int)+0x13c) [0x5638ceb492cc]
2019-11-05T20:52:33.899 INFO:tasks.ceph.osd.4.smithi134.stderr: 4: (ECBackend::do_read_op(ECBackend::ReadOp&)+0xca1) [0x5638ceebf821]
2019-11-05T20:52:33.899 INFO:tasks.ceph.osd.4.smithi134.stderr: 5: (ECBackend::start_read_op(int, std::map<hobject_t, std::set<int, std::less<int>, std::allocator<int> >, std::less<hobject_t>, std::allocator<std::pair<hobject_t const, std::set<int, std::less<int>, std::allocator<int> > > > >&, std::map<hobject_t, ECBackend::read_request_t, std::less<hobject_t>, std::allocator<std::pair<hobject_t const, ECBackend::read_request_t> > >&, boost::intrusive_ptr<OpRequest>, bool, bool)+0x465) [0x5638ceec0025]
2019-11-05T20:52:33.899 INFO:tasks.ceph.osd.4.smithi134.stderr: 6: (ECBackend::objects_read_and_reconstruct(std::map<hobject_t, std::__cxx11::list<boost::tuples::tuple<unsigned long, unsigned long, unsigned int, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type>, std::allocator<boost::tuples::tuple<unsigned long, unsigned long, unsigned int, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type> > >, std::less<hobject_t>, std::allocator<std::pair<hobject_t const, std::__cxx11::list<boost::tuples::tuple<unsigned long, unsigned long, unsigned int, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type>, std::allocator<boost::tuples::tuple<unsigned long, unsigned long, unsigned int, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type> > > > > > const&, bool, std::unique_ptr<GenContext<std::map<hobject_t, std::pair<int, interval_map<unsigned long, ceph::buffer::v14_2_0::list, bl_split_merge> >, std::less<hobject_t>, std::allocator<std::pair<hobject_t const, std::pair<int, interval_map<unsigned long, ceph::buffer::v14_2_0::list, bl_split_merge> > > > >&&>, std::default_delete<GenContext<std::map<hobject_t, std::pair<int, interval_map<unsigned long, ceph::buffer::v14_2_0::list, bl_split_merge> >, std::less<hobject_t>, std::allocator<std::pair<hobject_t const, std::pair<int, interval_map<unsigned long, ceph::buffer::v14_2_0::list, bl_split_merge> > > > >&&> > >&&)+0x7f2) [0x5638ceec14a2]
2019-11-05T20:52:33.899 INFO:tasks.ceph.osd.4.smithi134.stderr: 7: (ECBackend::objects_read_async(hobject_t const&, std::__cxx11::list<std::pair<boost::tuples::tuple<unsigned long, unsigned long, unsigned int, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type>, std::pair<ceph::buffer::v14_2_0::list*, Context*> >, std::allocator<std::pair<boost::tuples::tuple<unsigned long, unsigned long, unsigned int, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type, boost::tuples::null_type>, std::pair<ceph::buffer::v14_2_0::list*, Context*> > > > const&, Context*, bool)+0x369) [0x5638ceec1b09]
2019-11-05T20:52:33.899 INFO:tasks.ceph.osd.4.smithi134.stderr: 8: (PrimaryLogPG::OpContext::start_async_reads(PrimaryLogPG*)+0xe8) [0x5638cec46048]
2019-11-05T20:52:33.900 INFO:tasks.ceph.osd.4.smithi134.stderr: 9: (PrimaryLogPG::execute_ctx(PrimaryLogPG::OpContext*)+0x3ea) [0x5638cecc15ca]
2019-11-05T20:52:33.900 INFO:tasks.ceph.osd.4.smithi134.stderr: 10: (PrimaryLogPG::do_op(boost::intrusive_ptr<OpRequest>&)+0x2f5d) [0x5638cecc5c0d]
2019-11-05T20:52:33.900 INFO:tasks.ceph.osd.4.smithi134.stderr: 11: (PrimaryLogPG::do_request(boost::intrusive_ptr<OpRequest>&, ThreadPool::TPHandle&)+0xcf1) [0x5638cecd1b81]
2019-11-05T20:52:33.902 INFO:tasks.ceph.osd.4.smithi134.stderr: 12: (OSD::dequeue_op(boost::intrusive_ptr<PG>, boost::intrusive_ptr<OpRequest>, ThreadPool::TPHandle&)+0x17b) [0x5638ceb57a8b]
2019-11-05T20:52:33.902 INFO:tasks.ceph.osd.4.smithi134.stderr: 13: (ceph::osd::scheduler::PGOpItem::run(OSD*, OSDShard*, boost::intrusive_ptr<PG>&, ThreadPool::TPHandle&)+0x67) [0x5638cedb26c7]
2019-11-05T20:52:33.902 INFO:tasks.ceph.osd.4.smithi134.stderr: 14: (OSD::ShardedOpWQ::_process(unsigned int, ceph::heartbeat_handle_d*)+0x90c) [0x5638ceb751fc]
2019-11-05T20:52:33.902 INFO:tasks.ceph.osd.4.smithi134.stderr: 15: (ShardedThreadPool::shardedthreadpool_worker(unsigned int)+0x4ac) [0x5638cf1ae8dc]
2019-11-05T20:52:33.902 INFO:tasks.ceph.osd.4.smithi134.stderr: 16: (ShardedThreadPool::WorkThreadSharded::entry()+0x10) [0x5638cf1b1a50]
2019-11-05T20:52:33.902 INFO:tasks.ceph.osd.4.smithi134.stderr: 17: (()+0x76db) [0x7fa4a36a96db]
2019-11-05T20:52:33.902 INFO:tasks.ceph.osd.4.smithi134.stderr: 18: (clone()+0x3f) [0x7fa4a244988f]
2019-11-05T20:52:33.903 INFO:tasks.ceph.osd.4.smithi134.stderr:2019-11-05T20:52:33.897+0000 7fa47d75c700 -1 *** Caught signal (Segmentation fault) **

/a/sage-2019-11-05_17:51:43-rados-wip-sage-testing-2019-11-05-0856-distro-basic-smithi/4475123

lots of similar failures in this run

@majianpeng majianpeng force-pushed the osd-optimze-send_message_osd_cluster branch from 04e80a8 to 0b542de Compare November 7, 2019 03:24
…essage*>>& messages, epoch_t from_epoch).

Batch send message to osd cluster.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
… MOSDECSubOpReadReply.

Currently, MOSDECSubOpReadReply use send_message_osd_cluster(int, Message *, epoch_t) which is lookup for Connection.
So we use func send_message_osd_cluster(Message*, const ConnectionRef) to avoid lookup.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
Only has pre_publish_waiter, it call notify.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
…conns.

We don't use deleted_lock to protect func is_unregistered. This because if
race occur, func send_message still check state of AsyncConnection and
skip this message.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
…line func.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
@majianpeng
Copy link
Member Author

@liewegas . Bug fixed and I hope this can pass all the tests. Thanks!

@majianpeng
Copy link
Member Author

@liewegas . ping

@liewegas
Copy link
Member

yep, it's marked... should get it on my next run

@tchaikov tchaikov merged commit 1772803 into ceph:master Nov 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants