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

mon, osd: misc fixes and cleanups #16160

Merged
merged 9 commits into from Jul 10, 2017

Conversation

Projects
None yet
3 participants
@xiexingguo
Member

xiexingguo commented Jul 6, 2017

No description provided.

mon/Monitor: kill get_paxos_service_by_name()
(1) The new PAXOS_MGRSTAT is excluded and may cause
    problems
(2) This method has no consumer

Considering (2), it's better to be removed than to be fixed...

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>

@xiexingguo xiexingguo requested a review from liewegas Jul 6, 2017

@gregsfortytwo

Mostly looks good, but a few important bits.

@@ -9375,8 +9380,6 @@ const char** OSD::get_tracked_conf_keys() const
"osd_recovery_delay_start",
"osd_client_message_size_cap",
"osd_client_message_cap",
"osd_heartbeat_min_size",
"osd_heartbeat_interval",

This comment has been minimized.

@gregsfortytwo

gregsfortytwo Jul 6, 2017

Member

Some of these are in the list to indicate to users that they can be changed at runtime (because they're just read out of the struct), instead of having that scary message about it not being changeable.

@@ -2082,12 +2082,12 @@ class OSD : public Dispatcher,
void handle_pg_stats_ack(class MPGStatsAck *ack);
void flush_pg_stats();
ceph::coarse_mono_clock::time_point last_sent_beacon;
utime_t last_sent_beacon;

This comment has been minimized.

@gregsfortytwo

gregsfortytwo Jul 6, 2017

Member

This isn't going to work, we're trying to move to the standards-provided time structures. Why do you want it the other way around?

This comment has been minimized.

@xiexingguo

xiexingguo Jul 7, 2017

Member

To avoid the potential expensive clock_time system call whenever it is possible. But I think it is unnecessary now since we have already moved the send_beacon() logic into the tick_without_osd_lock thread(which does not require the big osd_lock anymore) and we are planning to migrate to the standards-provided time structures now.

All fixed and repushed. Thanks for the review!

xiexingguo added some commits Jul 5, 2017

osd/OSD.cc: fix tracked keys
osd_max_trimming_pgs, osd_op_history_slow_op_size and
osd_op_history_slow_op_threshold shall be in the tracked key list but they are not.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
osd/OSD: fix race condition
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
crush: fix misuse of some ASCII chars
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
osd/OSD: fix another race(clear outstanding_pg_stats)
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
osd/OSD: move send_beacon into tick_without_osd_lock
So we don't ask for the big osd_lock, which is potential expensive
and shall be avoided under any cases.

Also re-use min_last_epoch_clean_lock
to access last_sent_beacon safely(we have two callers - ms_handle_connect and
tick_without_osd_lock, hence may have potential races).

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
osd/OSD: fix blacklist dump format
Now:
[
    {
        "entity_addr_t": {
            "nonce": 2338822185,
            "addr": "127.0.0.1:6814"
        },
        "expire_time": "2017-07-06 14:57:29.097542"
    },
    {
        "entity_addr_t": {
            "nonce": 0,
            "addr": "10.118.20.136:12345"
        },
        "expire_time": "2017-07-06 15:27:21.490347"
    }
]

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
mon/OSDMonitor: fix error message
Actually this is not reachable...

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
mon/OSDMonitor: drop unnecessary parameter now from force_failure()
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
@gregsfortytwo

This comment has been minimized.

Member

gregsfortytwo commented Jul 7, 2017

Reviewed-by: Greg Farnum gfarnum@redhat.com

@liewegas liewegas merged commit 5a197c5 into ceph:master Jul 10, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

@xiexingguo xiexingguo deleted the xiexingguo:wip-misc2 branch Jul 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment