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: remove redudant call of heartbeat_check #12130

Merged
merged 1 commit into from Nov 29, 2016
Merged

osd: remove redudant call of heartbeat_check #12130

merged 1 commit into from Nov 29, 2016

Conversation

liupan1111
Copy link
Contributor

…dy called in tick_without_osd_lock.

Signed-off-by: Pan Liu pan.liu@istuary.com

@liupan1111
Copy link
Contributor Author

@liewegas , please help take a look. thanks.

@ghost ghost added the core label Nov 23, 2016
@liewegas liewegas changed the title OSD: remove redudant call of heartbeat_check. Hearbeat_check is alrea… osd: remove redudant call of heartbeat_check Nov 23, 2016
@liewegas liewegas added this to the kraken milestone Nov 23, 2016
@yuyuyu101
Copy link
Member

@liupan1111 plz remove extra rocksdb submodule updaate

Copy link
Member

@yuyuyu101 yuyuyu101 left a comment

Choose a reason for hiding this comment

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

rocksdb submodule is updated by accident

…ady called in tick_without_osd_lock.

Signed-off-by: Pan Liu <pan.liu@istuary.com>
@liupan1111
Copy link
Contributor Author

@yuyuyu101, thanks a lot for your remind. I just revert the changes for rocksdb. Please take a look.

@yuyuyu101
Copy link
Member

@liewegas @tchaikov I give a look at another heartbeat_check, it's called under is_active and is_waiting_for_healthy. I'm concerning IS_BOOTING state also needs heartbeat_check... Although I don't know whether it's problem...

@tchaikov tchaikov self-assigned this Nov 24, 2016
@liupan1111
Copy link
Contributor Author

@tchaikov, any comment about this pr?

@tchaikov
Copy link
Contributor

I give a look at another heartbeat_check, it's called under is_active and is_waiting_for_healthy. I'm concerning IS_BOOTING state also needs heartbeat_check... Although I don't know whether it's problem...

this heartbeat_check() call is scheduled in the tick_without_osd_lock() with the interval of osd_heartbeat_interval, with its own thread.

while the heartbeat_check() call to be removed lives in osd->heartbeat_entry(), which is in turn called in heartbeat_thread at the interval of 0.5 + random(0.. osd_heartbeat_interval), with its own thread also.

both threads are started right after the monclient starts, and before the osdmap is read from disk. so removing the heartbeat_check() call in heartbeat() won't interfere with IS_BOOTING or the boot process. however, my only concern is, prior to this change, we check the heartbeat with a random interval between 0.5 and osd_heartbeat_interval. and after this change, we check the heartbeat with a fixed interval of osd_heartbeat_interval.

but since the default osd_heartbeat_grace (20) is far larger than the default osd_heartbeat_interval (6), i think this change does not impact the behavior a lot. moreover, we have osd_fast_fail_on_connection_refused. so i think it's a safe change.

@@ -4296,9 +4296,6 @@ void OSD::heartbeat()
now));
}

dout(30) << "heartbeat check" << dendl;
heartbeat_check();
Copy link
Contributor

Choose a reason for hiding this comment

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

just out of curiosity, why not removing the call in tick_without_osd_lock()?

@liupan1111 liupan1111 deleted the wip-osd-hearteat branch November 30, 2016 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants