Hoststatus remove stats creation. #8689
Conversation
50bf469 to
a2d99b8
Compare
|
[approve ci rocky] |
|
#8694 should fix the autest. Once it's merged into 10-Dev you can rebase. |
a2d99b8 to
72a1503
Compare
|
[approve ci autest] |
5e441ef to
7e27dad
Compare
80eac88 to
0aa2f12
Compare
46bdebd to
5602294
Compare
|
[approve ci fedora] |
627dd3d to
aea5c1d
Compare
- Updates HostStatus to eliminate stats creation. - Updates traffic_ctl JSON-RPC to query HostStatus directly. - adds in a new persistance mechanism for HosStatus.
195567b to
2816009
Compare
|
[approve ci fedora] |
|
@jrushford this looks good, just have a question. |
@brbzull0 I didn't have any plans to use the JSON-RPC node to inject them but, that could be useful. In this PR, the host_records.yaml is loaded at startup by trafficserver. However, I'll take a look and make sure that the names are the same, someone might have a use case for the injection. |
|
|
||
| std::string | ||
| get_method() const | ||
| { |
There was a problem hiding this comment.
@brbzull0 it's not clear why this even needs to be a virtual function:
Should derived classes just set the "method" member string?
There was a problem hiding this comment.
it's not clear why this even needs to be a virtual function
Because of this
Should derived classes just set the "method" member string?
yes they can, in which case JSONRPCRequest::get_method will return it.
I wasn't very concern about the virtual thing in here as this is used by traffic_ctl and traffic_top.
src/traffic_server/HostStatus.cc
Outdated
| } | ||
| } | ||
| ts::Rv<YAML::Node> server_get_status(std::string_view const &id, YAML::Node const ¶ms); | ||
| ts::Rv<YAML::Node> server_set_status(std::string_view const &id, YAML::Node const ¶ms); |
There was a problem hiding this comment.
Why reference to string_view ?
| ts::Rv<YAML::Node> server_get_status(std::string_view const &id, YAML::Node const ¶ms); | ||
| ts::Rv<YAML::Node> server_set_status(std::string_view const &id, YAML::Node const ¶ms); | ||
| ts::Rv<YAML::Node> server_get_status(std::string_view const id, YAML::Node const ¶ms); | ||
| ts::Rv<YAML::Node> server_set_status(std::string_view const id, YAML::Node const ¶ms); |
There was a problem hiding this comment.
It's redundant to use 'string_view const' instead of just plain 'string_view' but harmless.
There was a problem hiding this comment.
It's redundant to use 'string_view const' instead of just plain 'string_view' but harmless.
Totally, but the reason is this:
It is expected to be like this.
--
The reason is const& is becase I wanted to make it clear that this should not mutate despite the fact that you cannot change it if it was only a std::string_view. I always thought that this is ambiguous and if someone try to change it well, good luck. Now kind of I am inclined to remove the const &
|
Looks good to me, let's see what Damian says. |
|
[approve ci fedora] |
1 similar comment
|
[approve ci fedora] |
|
[approve ci] |
|
Looks good to me. |
HostStatus::createHostStat(), that was removed with PR apache#8689.
Modifications to the traffic_ctl host status command and HostStatus
Host status will no-longer create persistent stats so now when a host is marked up or down using traffic_ctl, the entire set of HostStatus records will be dumped from memory to var/trafficserver/host_records.yaml so that when ATS restarts, the records are re-loaded. stats_over_http will no-longer show "proxy.process.host_status" stats and are no-longer seen with traffic_ctl metric match.
traffic_ctl host status without any other arguments will list all host status records replacing the metric match usage to list all host records.