You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2020/01/13 22:30:24 UTC

[GitHub] [trafficcontrol] rob05c opened a new pull request #4289: Fix atstccfg caching

rob05c opened a new pull request #4289: Fix atstccfg caching
URL: https://github.com/apache/trafficcontrol/pull/4289
 
 
   Fixes atstccfg caching, which was breaking when the new cache file
   was smaller than the old, because it wasn't truncating the file.
   
   Also changes deliveryservice_server to always fetch all Servers and
   Delivery Services and cache them, because it's faster for an ORT
   run to reuse and cache them all.
   
   Testing a complete `badass` on a largeish CDN, old Perl ORT against Perl TO takes ~4m, atstccfg without fixed caching takes ~5m, with this fix takes ~2.5m. So, this probably isn't a huge deal. But good to fix.
   
   Includes unit tests.
   No docs, no interface change.
   No changelog, no interface change.
   
   - [x] This PR is not related to any other Issue
   
   ## Which Traffic Control components are affected by this PR?
   - Traffic Ops ORT
   
   ## What is the best way to verify this PR?
   
   Run unit tests. Run ORT, verify it's still correct and faster than without this PR.
   
   ## If this is a bug fix, what versions of Traffic Control are affected?
   
   It's in master, but it's not really a "bug" in that it still produces the right output, just not as fast as it could.
   
   ## The following criteria are ALL met by this PR
   
   - [x] This PR includes tests OR I have explained why tests are unnecessary
   - [x] This PR includes documentation OR I have explained why documentation is unnecessary
   - [x] This PR includes an update to CHANGELOG.md OR such an update is not necessary
   - [x] This PR includes any and all required license headers
   - [x] This PR ensures that database migration sequence is correct OR this PR does not include a database migration
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
   
   ## Additional Information

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


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4289: Fix atstccfg caching

Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #4289: Fix atstccfg caching
URL: https://github.com/apache/trafficcontrol/pull/4289#discussion_r367624157
 
 

 ##########
 File path: traffic_ops/ort/atstccfg/toreq/caching.go
 ##########
 @@ -84,7 +84,12 @@ func WriteCacheJSON(tempDir string, cacheFileName string, obj interface{}) {
 	}
 	defer objFile.Close()
 
-	if _, err := objFile.Write(objBts); err != nil {
+	if err := objFile.Truncate(0); err != nil { // must truncate, or Write will leave whatever existed after the beginning, if the file already existed and was longer than our new write.
 
 Review comment:
   Could we just add the `os.O_TRUNC` flag to `os.OpenFile` above? That would be more straightforward than manually truncating the file after opening and writing specifically to offset 0.

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


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rawlinp merged pull request #4289: Fix atstccfg caching

Posted by GitBox <gi...@apache.org>.
rawlinp merged pull request #4289: Fix atstccfg caching
URL: https://github.com/apache/trafficcontrol/pull/4289
 
 
   

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


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rob05c commented on a change in pull request #4289: Fix atstccfg caching

Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4289: Fix atstccfg caching
URL: https://github.com/apache/trafficcontrol/pull/4289#discussion_r370854035
 
 

 ##########
 File path: traffic_ops/ort/atstccfg/toreq/caching.go
 ##########
 @@ -84,7 +84,12 @@ func WriteCacheJSON(tempDir string, cacheFileName string, obj interface{}) {
 	}
 	defer objFile.Close()
 
-	if _, err := objFile.Write(objBts); err != nil {
+	if err := objFile.Truncate(0); err != nil { // must truncate, or Write will leave whatever existed after the beginning, if the file already existed and was longer than our new write.
 
 Review comment:
   Changed.

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


With regards,
Apache Git Services