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 2020/04/24 17:46:41 UTC
[GitHub] [trafficserver] ezelkow1 opened a new pull request #6708: Adding logging fields for collapsed forwarding metrics
ezelkow1 opened a new pull request #6708:
URL: https://github.com/apache/trafficserver/pull/6708
This adds 3 new log fields for transaction output (since collapsed forwarding is transaction based)
* crra - The number of read retry attempts for this transaction. If no CF has been done (it has not attempted a write) then this is just the read lock contention. If a write has been attempted then this value gets reset to 0 as it attempts more reads post-write-fail.
* cwra - The number of write retry attempts. This will be either the number based on write lock contention, or it can be the max-write-retries+1. If it is max+1 that means that we have hit the max attempts because the lock was being held by another writer and so this transaction fell back to read retries and attempted to be collapsed
* cccs - Collapsed connection success. This is just 0/1. 0 if a connection either was not attempted to be collapsed, did not need to be collapsed, or it was attempted but ended up going to the origin anyway. If it is 1 then this transaction hit its max write retries but successfully got a cache HIT during its read retries post-write-failure
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] sudheerv commented on a change in pull request #6708: Adding logging fields for collapsed forwarding metrics
Posted by GitBox <gi...@apache.org>.
sudheerv commented on a change in pull request #6708:
URL: https://github.com/apache/trafficserver/pull/6708#discussion_r416037442
##########
File path: proxy/logging/LogAccess.cc
##########
@@ -2631,6 +2631,55 @@ LogAccess::marshal_client_http_transaction_priority_dependence(char *buf)
return INK_MIN_ALIGN;
}
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
+
+int
+LogAccess::marshal_cache_read_retries(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ id = m_http_sm->get_cache_sm().get_open_read_tries();
+ }
+ marshal_int(buf, id);
+ }
+ return INK_MIN_ALIGN;
+}
+
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
+
+int
+LogAccess::marshal_cache_write_retries(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ id = m_http_sm->get_cache_sm().get_open_write_tries();
+ }
+ marshal_int(buf, id);
+ }
+ return INK_MIN_ALIGN;
+}
+
+int
+LogAccess::marshal_cache_collapsed_connection_success(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ SquidLogCode code = m_http_sm->t_state.squid_codes.log_code;
+ if ((m_http_sm->get_cache_sm().get_open_write_tries() == (m_http_sm->t_state.txn_conf->max_cache_open_write_retries + 1)) &&
Review comment:
Seems like it's a bit arguable, but, I agree it's not a big deal as long as it's documented clearly. 👍
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] sudheerv commented on a change in pull request #6708: Adding logging fields for collapsed forwarding metrics
Posted by GitBox <gi...@apache.org>.
sudheerv commented on a change in pull request #6708:
URL: https://github.com/apache/trafficserver/pull/6708#discussion_r414960734
##########
File path: proxy/logging/LogAccess.cc
##########
@@ -2631,6 +2631,55 @@ LogAccess::marshal_client_http_transaction_priority_dependence(char *buf)
return INK_MIN_ALIGN;
}
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
+
+int
+LogAccess::marshal_cache_read_retries(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ id = m_http_sm->get_cache_sm().get_open_read_tries();
+ }
+ marshal_int(buf, id);
+ }
+ return INK_MIN_ALIGN;
+}
+
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
+
+int
+LogAccess::marshal_cache_write_retries(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ id = m_http_sm->get_cache_sm().get_open_write_tries();
+ }
+ marshal_int(buf, id);
+ }
+ return INK_MIN_ALIGN;
+}
+
+int
+LogAccess::marshal_cache_collapsed_connection_success(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ SquidLogCode code = m_http_sm->t_state.squid_codes.log_code;
+ if ((m_http_sm->get_cache_sm().get_open_write_tries() == (m_http_sm->t_state.txn_conf->max_cache_open_write_retries + 1)) &&
Review comment:
Does it makes sense to set this to may be `-1` when CF was never attempted to distinguish from a failed attempt?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #6708: Adding logging fields for collapsed forwarding metrics
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #6708:
URL: https://github.com/apache/trafficserver/pull/6708#issuecomment-623850630
No point in marking it for Project 10.0.x, that's the default (master branch). I'm guessing you want this for 9.0.x, so marking it accordingly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ezelkow1 commented on a change in pull request #6708: Adding logging fields for collapsed forwarding metrics
Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on a change in pull request #6708:
URL: https://github.com/apache/trafficserver/pull/6708#discussion_r414963388
##########
File path: proxy/logging/LogAccess.cc
##########
@@ -2631,6 +2631,55 @@ LogAccess::marshal_client_http_transaction_priority_dependence(char *buf)
return INK_MIN_ALIGN;
}
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
+
+int
+LogAccess::marshal_cache_read_retries(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ id = m_http_sm->get_cache_sm().get_open_read_tries();
+ }
+ marshal_int(buf, id);
+ }
+ return INK_MIN_ALIGN;
+}
+
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
+
+int
+LogAccess::marshal_cache_write_retries(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ id = m_http_sm->get_cache_sm().get_open_write_tries();
+ }
+ marshal_int(buf, id);
+ }
+ return INK_MIN_ALIGN;
+}
+
+int
+LogAccess::marshal_cache_collapsed_connection_success(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ SquidLogCode code = m_http_sm->t_state.squid_codes.log_code;
+ if ((m_http_sm->get_cache_sm().get_open_write_tries() == (m_http_sm->t_state.txn_conf->max_cache_open_write_retries + 1)) &&
Review comment:
👍 yea, gets some more usability out of it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ezelkow1 commented on a change in pull request #6708: Adding logging fields for collapsed forwarding metrics
Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on a change in pull request #6708:
URL: https://github.com/apache/trafficserver/pull/6708#discussion_r415906303
##########
File path: proxy/logging/LogAccess.cc
##########
@@ -2631,6 +2631,55 @@ LogAccess::marshal_client_http_transaction_priority_dependence(char *buf)
return INK_MIN_ALIGN;
}
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
+
+int
+LogAccess::marshal_cache_read_retries(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ id = m_http_sm->get_cache_sm().get_open_read_tries();
+ }
+ marshal_int(buf, id);
+ }
+ return INK_MIN_ALIGN;
+}
+
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
+
+int
+LogAccess::marshal_cache_write_retries(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ id = m_http_sm->get_cache_sm().get_open_write_tries();
+ }
+ marshal_int(buf, id);
+ }
+ return INK_MIN_ALIGN;
+}
+
+int
+LogAccess::marshal_cache_collapsed_connection_success(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ SquidLogCode code = m_http_sm->t_state.squid_codes.log_code;
+ if ((m_http_sm->get_cache_sm().get_open_write_tries() == (m_http_sm->t_state.txn_conf->max_cache_open_write_retries + 1)) &&
Review comment:
I think I may do:
* 0 - not attempted
* -1 - attempted and failed
* 1 - attempted and success
Just like sticking with the negative==failure type returns
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #6708: Adding logging fields for collapsed forwarding metrics
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #6708:
URL: https://github.com/apache/trafficserver/pull/6708#issuecomment-623850707
Cherry-picked to v9.0.x branch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ezelkow1 commented on pull request #6708: Adding logging fields for collapsed forwarding metrics
Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on pull request #6708:
URL: https://github.com/apache/trafficserver/pull/6708#issuecomment-619204376
[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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org