You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "moonchen (via GitHub)" <gi...@apache.org> on 2025/11/21 19:30:54 UTC
[PR] Add USDT for cache directory insertion and deletion [trafficserver]
moonchen opened a new pull request, #12688:
URL: https://github.com/apache/trafficserver/pull/12688
Add cache directory insert and delete USDT. These will emit the cache directory stripe, segment, segment offset (implying bucket and depth), disk offset, and approx size.
Pass the subclass StripeSM instead of superclass Stripe into helper functions in order to access stripe->fd.
Add an autest for these.
--
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
Re: [PR] Add USDT for cache directory insertion and deletion [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen commented on PR #12688:
URL: https://github.com/apache/trafficserver/pull/12688#issuecomment-3564422821
[approve ci Rocky]
--
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
Re: [PR] Add USDT for cache directory insertion and deletion [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen commented on code in PR #12688:
URL: https://github.com/apache/trafficserver/pull/12688#discussion_r2561240832
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -724,9 +734,12 @@ Directory::remove(const CacheKey *key, StripeSM *stripe, Dir *del)
return 0;
}
#endif
- if (dir_compare_tag(e, key) && dir_offset(e) == dir_offset(del)) {
+ int64_t offset = dir_offset(e);
+ if (dir_compare_tag(e, key) && offset == dir_offset(del)) {
ts::Metrics::Gauge::decrement(cache_rsb.direntries_used);
ts::Metrics::Gauge::decrement(stripe->cache_vol->vol_rsb.direntries_used);
+ ATS_PROBE7(cache_dir_remove, stripe ? stripe->fd : -1, s, dir_to_offset(e, seg), offset, dir_approx_size(e),
+ key->slice64(0), key->slice64(1));
Review Comment:
Thanks. This was a vestige I forgot to remove.
--
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
Re: [PR] Add USDT for cache directory insertion and deletion [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #12688:
URL: https://github.com/apache/trafficserver/pull/12688#discussion_r2560575283
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -724,9 +734,12 @@ Directory::remove(const CacheKey *key, StripeSM *stripe, Dir *del)
return 0;
}
#endif
- if (dir_compare_tag(e, key) && dir_offset(e) == dir_offset(del)) {
+ int64_t offset = dir_offset(e);
+ if (dir_compare_tag(e, key) && offset == dir_offset(del)) {
ts::Metrics::Gauge::decrement(cache_rsb.direntries_used);
ts::Metrics::Gauge::decrement(stripe->cache_vol->vol_rsb.direntries_used);
+ ATS_PROBE7(cache_dir_remove, stripe ? stripe->fd : -1, s, dir_to_offset(e, seg), offset, dir_approx_size(e),
+ key->slice64(0), key->slice64(1));
Review Comment:
Minor: probably just remove the null check on stripe. You don't do that elsewhere in this patch, and it was just dereferenced on the previous line. So if there's a problem with it being null, it won't be your fault. :)
Maybe add `ink_assert(stripe != nullptr);` to the top of the function stating the invariant that it whould be non-null? Taking a `&` is a possibility too, but then this patch is increasing beyond scope. So the assert should be fine.
--
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
Re: [PR] Add USDT for cache directory insertion and deletion [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen commented on PR #12688:
URL: https://github.com/apache/trafficserver/pull/12688#issuecomment-3564440058
[approve ci rocky]
--
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
Re: [PR] Add USDT for cache directory insertion and deletion [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #12688:
URL: https://github.com/apache/trafficserver/pull/12688#issuecomment-3576247918
[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
Re: [PR] Add USDT for cache directory insertion and deletion [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen commented on code in PR #12688:
URL: https://github.com/apache/trafficserver/pull/12688#discussion_r2561241517
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -724,9 +734,12 @@ Directory::remove(const CacheKey *key, StripeSM *stripe, Dir *del)
return 0;
}
#endif
- if (dir_compare_tag(e, key) && dir_offset(e) == dir_offset(del)) {
+ int64_t offset = dir_offset(e);
+ if (dir_compare_tag(e, key) && offset == dir_offset(del)) {
ts::Metrics::Gauge::decrement(cache_rsb.direntries_used);
ts::Metrics::Gauge::decrement(stripe->cache_vol->vol_rsb.direntries_used);
+ ATS_PROBE7(cache_dir_remove, stripe ? stripe->fd : -1, s, dir_to_offset(e, seg), offset, dir_approx_size(e),
+ key->slice64(0), key->slice64(1));
Review Comment:
Removed with a new commit.
--
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
Re: [PR] Add USDT for cache directory insertion and deletion [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen merged PR #12688:
URL: https://github.com/apache/trafficserver/pull/12688
--
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