You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2022/01/20 02:34:06 UTC
[GitHub] [trafficserver] masaori335 opened a new pull request #8179: Add a new --enable-event-tracker configure option
masaori335 opened a new pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179
# Feature
When this option is enabled, events track who scheduled them.
# Motivation
This would give us some clues when we face some crashes on the event handler. Especially, the Continuation doesn't have history.
# Usage
To get symbol, convert the address by `addr2line(1)` (Linux) or `atos(1)` (BSD) ( assuming ATS has debug info) like below.
```
» lldb /opt/ats/bin/traffic_server
(lldb) target create "/opt/ats/bin/traffic_server"
Current executable set to '/opt/ats/bin/traffic_server' (x86_64).
...
traffic_server was compiled with optimization - stepping may behave oddly; variables may not be available.
frame #6: 0x00000001002e2135 traffic_server`EThread::process_event(this=0x0000000018038000, e=0x0000000000830b80, calling_code=2) at UnixEThread.cc:164 [opt]
161 // Restore the client IP debugging flags
162 set_cont_flags(e->continuation->control_flags);
163
-> 164 e->continuation->handleEvent(calling_code, e);
165 ink_assert(!e->in_the_priority_queue);
166 ink_assert(c_temp == e->continuation);
167 MUTEX_RELEASE(lock);
(lldb) p e->_location
(const void *) $0 = 0x00000001001a94ac
----
» atos -o /opt/ats/bin/traffic_server 0x00000001001a94ac
HostDBProcessor::start(int, unsigned long) (in traffic_server) (HostDB.cc:425)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on a change in pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
masaori335 commented on a change in pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179#discussion_r804269600
##########
File path: iocore/eventsystem/UnixEvent.cc
##########
@@ -104,3 +106,19 @@ Event::schedule_every(ink_hrtime aperiod, int acallback_event)
ethread->EventQueueExternal.enqueue_local(this);
}
}
+
+#ifdef ENABLE_EVENT_TRACKER
+
+void
+Event::set_location()
+{
+ _location = ink_backtrace(3);
+}
+
+const void *
+Event::get_location() const
Review comment:
Right. It's just the buddy of `set_location` for now.
A possible use case is printing the location on debug log instead of using `add2line` command.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179#issuecomment-1017060715
[approve ci]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 merged pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
masaori335 merged pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] github-actions[bot] commented on pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179#issuecomment-1011661696
This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179#issuecomment-1042425474
Cherry-picked to v9.2.x
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] github-actions[bot] closed pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on a change in pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
masaori335 commented on a change in pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179#discussion_r681354583
##########
File path: iocore/eventsystem/UnixEvent.cc
##########
@@ -104,3 +106,19 @@ Event::schedule_every(ink_hrtime aperiod, int acallback_event)
ethread->EventQueueExternal.enqueue_local(this);
}
}
+
+#ifdef ENABLE_EVENT_TRACKER
+
+void
+Event::set_location()
+{
+ _location = ink_backtrace(3);
Review comment:
This is doing pretty tricky. Assuming this is called by `schedule_*` functions and records where they're called (the 3rd frame of the call stack).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] rob05c commented on a change in pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179#discussion_r804088228
##########
File path: iocore/eventsystem/UnixEvent.cc
##########
@@ -104,3 +106,19 @@ Event::schedule_every(ink_hrtime aperiod, int acallback_event)
ethread->EventQueueExternal.enqueue_local(this);
}
}
+
+#ifdef ENABLE_EVENT_TRACKER
+
+void
+Event::set_location()
+{
+ _location = ink_backtrace(3);
+}
+
+const void *
+Event::get_location() const
Review comment:
I don't see anything calling `get_location`. Does anything use the location yet? Or is the plan to do that in a future PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on a change in pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
masaori335 commented on a change in pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179#discussion_r804272172
##########
File path: iocore/eventsystem/UnixEvent.cc
##########
@@ -104,3 +106,19 @@ Event::schedule_every(ink_hrtime aperiod, int acallback_event)
ethread->EventQueueExternal.enqueue_local(this);
}
}
+
+#ifdef ENABLE_EVENT_TRACKER
+
+void
+Event::set_location()
+{
+ _location = ink_backtrace(3);
Review comment:
I though `backtrace(3)` doesn't have noticeable overhead. Because it just refers address of current call stack. I'll measure on/off at production.
https://man7.org/linux/man-pages/man3/backtrace.3.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] rob05c commented on a change in pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179#discussion_r804079032
##########
File path: iocore/eventsystem/UnixEvent.cc
##########
@@ -104,3 +106,19 @@ Event::schedule_every(ink_hrtime aperiod, int acallback_event)
ethread->EventQueueExternal.enqueue_local(this);
}
}
+
+#ifdef ENABLE_EVENT_TRACKER
+
+void
+Event::set_location()
+{
+ _location = ink_backtrace(3);
Review comment:
Any idea how performant this is? I.e. is it going to significantly impact how much traffic a production cache can take, and/or TTMS? Or is it very small, could we just leave this on in Production?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on a change in pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
masaori335 commented on a change in pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179#discussion_r681354583
##########
File path: iocore/eventsystem/UnixEvent.cc
##########
@@ -104,3 +106,19 @@ Event::schedule_every(ink_hrtime aperiod, int acallback_event)
ethread->EventQueueExternal.enqueue_local(this);
}
}
+
+#ifdef ENABLE_EVENT_TRACKER
+
+void
+Event::set_location()
+{
+ _location = ink_backtrace(3);
Review comment:
This is doing pretty tricky. Assuming this is called by `schedule_*` functions to record where they're called (the 3rd frame of the call stack).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 merged pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
masaori335 merged pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] rob05c commented on a change in pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179#discussion_r804211783
##########
File path: iocore/eventsystem/UnixEvent.cc
##########
@@ -104,3 +106,19 @@ Event::schedule_every(ink_hrtime aperiod, int acallback_event)
ethread->EventQueueExternal.enqueue_local(this);
}
}
+
+#ifdef ENABLE_EVENT_TRACKER
+
+void
+Event::set_location()
+{
+ _location = ink_backtrace(3);
Review comment:
> the 3rd frame of the call stack
Yeah, the whole "3" thing is awkward, but that's pretty common when looking back on the stack. It seems fragile, but it's also unlikely to change. Meh.
##########
File path: iocore/eventsystem/UnixEvent.cc
##########
@@ -104,3 +106,19 @@ Event::schedule_every(ink_hrtime aperiod, int acallback_event)
ethread->EventQueueExternal.enqueue_local(this);
}
}
+
+#ifdef ENABLE_EVENT_TRACKER
+
+void
+Event::set_location()
+{
+ _location = ink_backtrace(3);
Review comment:
> the 3rd frame of the call stack
Yeah, the whole "3" thing is awkward, but that's pretty common when looking back on the stack. It seems fragile, but it's also unlikely to change. Meh.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] rob05c commented on a change in pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179#discussion_r804360668
##########
File path: iocore/eventsystem/UnixEvent.cc
##########
@@ -104,3 +106,19 @@ Event::schedule_every(ink_hrtime aperiod, int acallback_event)
ethread->EventQueueExternal.enqueue_local(this);
}
}
+
+#ifdef ENABLE_EVENT_TRACKER
+
+void
+Event::set_location()
+{
+ _location = ink_backtrace(3);
Review comment:
Ah, excellent. I was just curious. It's still very useful in debug mode, but there's so much more we can do if we can just leave it on all the time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #8179: Add a new --enable-event-tracker configure option
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #8179:
URL: https://github.com/apache/trafficserver/pull/8179#issuecomment-891423344
When pigs fly, and https://github.com/apache/trafficserver/pull/7674 gets merged, seems like it might combine nicely with this, to allow use in production:
```
DbgCtl event_problem_xyz_dbg_ctl("event.problem.xyz");
...
if (is_dbg_ctl_enabled(event_problem_xyz_dbg_ctl)) {
e->set_location();
}
e->scheule_imm(cont, EVENT_XYZ, nullptr);
...
DbgCtl event_problem_xyz_abc_dbg_ctl("event.problem.xyz.abc");
...
case EVENT_XYZ:
...
if (bad_thing_abc_happened && is_dbg_cntl_enabled(event_problem_xyz_abc_dbg_ctl)) {
DbgPrint(event_problem_xyz_abc_dbg_ctl, "Oh my ABC happened!!!");
auto loc = e->get_location();
// DbgPrint() calls to output location.
...
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org