[GitHub] [trafficserver] SolidWallOfCode commented on a change in pull request #7782: Short circuit remap reload when a valid remap file is not specified

[GitBox <gi...@apache.org>]

SolidWallOfCode commented on a change in pull request #7782:
URL: https://github.com/apache/trafficserver/pull/7782#discussion_r626125336



##########
File path: proxy/ReverseProxy.cc
##########
@@ -67,7 +67,8 @@ init_reverse_proxy()
 
   Note("%s loading ...", ts::filename::REMAP);
   if (!rewrite_table->load()) {
-    Fatal("%s failed to load", ts::filename::REMAP);
+    Warning("%s failed to load", ts::filename::REMAP);
+    return 0;

Review comment:
       I must disagree with @randall - this return will prevent unrelated configuration update handlers from being installed. See lines 74-77. The GC protection on `rewrite_table` is also lost - that might cause a dangling pointer. You might do better to add an `else`.
   ```
   if (rewrite_table.load()) {
     Note("loaded");
   } else {
     Warning("failed");
   }
   ```
   Note there should be nothing to prevent adding a valid "remap.config" file after process start. It's just another variant of reloading.
   




-- 
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