osdc/Objecter: no need null pointer check for op->session anymore

[runsisi]

9a5651b refactored the resend logic for linger op, op->session should be assigned by _recalc_linger_op_target if resend is needed

Signed-off-by: runsisi luo.runbing@zte.com.cn

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

[runsisi]

@liewegas since _recalc_linger_op_target could not handle the paused case, i.e., LingerOp::target.paused never touched by anyone (see Objecter::_send_linger), then the following bug exists

1. ceph osd pause
2. rados watch -p pool oid
3. ceph osd unpause
4. then the linger op will be in a lost state, i.e., osd op cancelled and linger op never resend

but i don't know how to fix it properly :(

[runsisi]

@liewegas i try to fix the bug described above in this pr: #25243, thanks :)

[tchaikov]

retest this please

[gregsfortytwo]

So _recalc_linger_op_target() invokes _session_linger_op_assign(), but only if _get_session() had a different answer than previously — and one of the allowed answers is "homeless". It looks to me like a newly-homeless op can still be put on the need_resend_linger list?
Although since this code doesn't handle that either, maybe it's not the case. Can we at least add an assert(session);?

[runsisi]

_recalc_linger_op_target only gets called by Objecter::_scan_requests which assures the checked linger op always has a OSDSession associated with it, and in _recalc_linger_op_target if _get_session has a different answer then it will assign a different (non-null) session.

It looks to me like a newly-homeless op can still be put on the need_resend_linger list?

yes, it's perfectly valid, so in Objecter::handle_osd_map before resending the linger op we have a homeless check.

Can we at least add an assert(session);?

sure :)

[liewegas]

@runsisi can you open a tracker ticket for this?

[runsisi]

@liewegas this pr is just a cleanup, i do not have the pemission to change the pr's label from bugfix to cleanup :(

the related bugfix is in this pr: #25243, please review, thanks~
and i have created a ticket for it: http://tracker.ceph.com/issues/37398

[tchaikov]

• http://pulpito.ceph.com/kchai-2018-11-29_01:22:06-rados-wip-kefu2-testing-2018-11-29-0053-distro-basic-smithi/
• http://pulpito.ceph.com/kchai-2018-11-29_08:14:41-rados-wip-kefu2-testing-2018-11-29-1356-distro-basic-smithi/

test failures are tracked / caused by

• osd: per-pool osd stats collection #19454
• http://tracker.ceph.com/issues/37391

[tchaikov]

@gregsfortytwo is this change good to merge?

[gregsfortytwo]

Reviewed-by: Greg Farnum gfarnum@redhat.com

[gregsfortytwo]

Oh but the commit should be amended to include

Fixes: http://tracker.ceph.com/issues/37398

before we merge! :)

[tchaikov]

@runsisi @gregsfortytwo i think it's #25243 which addresses http://tracker.ceph.com/issues/37398, right?

[runsisi]

@tchaikov yeah, this is pr is just a cleanup :)

and i will update the other one later~

[tchaikov]

@gregsfortytwo good to merge?

[gregsfortytwo]

Yeah sorry for my confusion there.