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 2023/01/13 21:55:22 UTC

[GitHub] [trafficserver] lzx404243 opened a new pull request, #9306: Updated warning message for adding duplicated remap entries

lzx404243 opened a new pull request, #9306:
URL: https://github.com/apache/trafficserver/pull/9306

   Current logs when the user specifies duplicated remap rules:
   ```
   [Jan 10 23:34:36.437] traffic_server ERROR: Couldn't insert into trie!
   [Jan 10 23:34:36.437] traffic_server WARNING: Could not insert new mapping
   [Jan 10 23:34:36.437] traffic_server ERROR: [ReverseProxy] failed to add remap rule at /tmp/sb/duplicated_remap_entries/ts2/config/remap.config line 2: unable to add mapping rule to lookup table
   [Jan 10 23:34:36.437] traffic_server WARNING: something failed during BuildTable() -- check your remap plugins!
   [Jan 10 23:34:36.437] traffic_server EMERGENCY: remap.config failed to load
   ```
   This PR changes the `Could not insert new mapping` to `Could not insert new mapping: duplicated entry exists.` to make it more explicit why the rule insertion fails.


-- 
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] bneradt commented on a diff in pull request #9306: Updated warning message for adding duplicated remap entries

Posted by GitBox <gi...@apache.org>.
bneradt commented on code in PR #9306:
URL: https://github.com/apache/trafficserver/pull/9306#discussion_r1070130973


##########
proxy/http/remap/UrlRewrite.cc:
##########
@@ -769,7 +769,7 @@ UrlRewrite::TableInsert(std::unique_ptr<URLTable> &h_table, url_mapping *mapping
     h_table->emplace(src_host, ht_contents);
   }
   if (!ht_contents->Insert(mapping)) {
-    Warning("Could not insert new mapping");
+    Warning("Could not insert new mapping: duplicated entry exists.");

Review Comment:
   It's not immediately obvious here that the only reason that `Insert` would return false is that there is a duplicate. Looking at [the code](https://github.com/apache/trafficserver/blob/f1ece8382a54fcbc7e3416e427b26d25b3177986/include/tscore/Trie.h#L169), as I'm sure you did, that seems to be the case. Let's add a comment assuring the reader of the code that this is true:
   
   ```
   // Trie::Insert only fails due to an attempt to add a duplicate entry.
   ```



-- 
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 #9306: Updated warning message for adding duplicated remap entries

Posted by "zwoop (via GitHub)" <gi...@apache.org>.
zwoop commented on PR #9306:
URL: https://github.com/apache/trafficserver/pull/9306#issuecomment-1400714380

   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] maskit merged pull request #9306: Updated warning message for adding duplicated remap entries

Posted by GitBox <gi...@apache.org>.
maskit merged PR #9306:
URL: https://github.com/apache/trafficserver/pull/9306


-- 
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] lzx404243 commented on a diff in pull request #9306: Updated warning message for adding duplicated remap entries

Posted by GitBox <gi...@apache.org>.
lzx404243 commented on code in PR #9306:
URL: https://github.com/apache/trafficserver/pull/9306#discussion_r1070133855


##########
proxy/http/remap/UrlRewrite.cc:
##########
@@ -769,7 +769,7 @@ UrlRewrite::TableInsert(std::unique_ptr<URLTable> &h_table, url_mapping *mapping
     h_table->emplace(src_host, ht_contents);
   }
   if (!ht_contents->Insert(mapping)) {
-    Warning("Could not insert new mapping");
+    Warning("Could not insert new mapping: duplicated entry exists.");

Review Comment:
   Yeah this can be helpful. Added 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.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org