-
Notifications
You must be signed in to change notification settings - Fork 598
Add signal handling to stream manager #2950
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,12 @@ DEFINE_string(ckptmgr_id, "", "The id of the local ckptmgr"); | |
DEFINE_int32(ckptmgr_port, 0, "The port of the local ckptmgr"); | ||
DEFINE_string(metricscachemgr_mode, "disabled", "MetricsCacheMgr mode, default `disabled`"); | ||
|
||
EventLoopImpl ss; | ||
|
||
void sigtermHandler(int signum) { | ||
ss.loopExit(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am a bit concerned that if loopExit() is safe to call here.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the sigtermHandler should run the in the same thread as any other callback function and loopExit() directly calls event_base_loopbreak() which I think should be safe to call. Here in the libevent book they call event_base_loopbreak() in a very similar way. But maybe someone with more experience with libevent could give some input on that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nwangtw - stmgr runs in a single thread. hence loopExit is fine in the context of sigterm handler. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nwangtw - stmgr runs in a single thread. hence loopExit is fine in the context of sigterm handler. |
||
} | ||
|
||
int main(int argc, char* argv[]) { | ||
gflags::ParseCommandLineFlags(&argc, &argv, true); | ||
|
||
|
@@ -56,8 +62,6 @@ int main(int argc, char* argv[]) { | |
} | ||
std::vector<std::string> instances = StrUtils::split(FLAGS_instance_ids, ","); | ||
|
||
EventLoopImpl ss; | ||
|
||
// Read heron internals config from local file | ||
// Create the heron-internals-config-reader to read the heron internals config | ||
heron::config::HeronInternalsConfigReader::Create(&ss, | ||
|
@@ -85,6 +89,7 @@ int main(int argc, char* argv[]) { | |
FLAGS_shell_port, FLAGS_ckptmgr_port, FLAGS_ckptmgr_id, | ||
high_watermark, low_watermark, FLAGS_metricscachemgr_mode); | ||
mgr.Init(); | ||
ss.registerSignal(SIGTERM, &sigtermHandler); | ||
ss.loop(); | ||
return 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this function hooked up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is not actually used yet. I included it, because other events also have a unregister function and it might be useful sometime in the future.
I can also remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kk. It is ok to leave it there to me. Let's see if @srkukarni has any input.