You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ra...@apache.org on 2020/01/30 19:30:54 UTC
[trafficcontrol] branch 4.0.x updated: Fix atstccfg caching (#4289)
(#4358)
This is an automated email from the ASF dual-hosted git repository.
rawlin pushed a commit to branch 4.0.x
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/4.0.x by this push:
new ebdb67f Fix atstccfg caching (#4289) (#4358)
ebdb67f is described below
commit ebdb67fa7c463f61153baa1f2dcbf171554dd055
Author: Rawlin Peters <ra...@apache.org>
AuthorDate: Thu Jan 30 12:30:43 2020 -0700
Fix atstccfg caching (#4289) (#4358)
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.
(cherry picked from commit f3e30a22d5995540eab4c3f89910afcc2ad53a6a)
Co-authored-by: Robert Butts <ro...@users.noreply.github.com>
---
traffic_ops/ort/atstccfg/toreq/caching.go | 3 +-
traffic_ops/ort/atstccfg/toreq/caching_test.go | 68 ++++++++++++++++++++++++++
traffic_ops/ort/atstccfg/toreq/toreq.go | 26 +++++++---
3 files changed, 89 insertions(+), 8 deletions(-)
diff --git a/traffic_ops/ort/atstccfg/toreq/caching.go b/traffic_ops/ort/atstccfg/toreq/caching.go
index 8ae90ea..eec1260 100644
--- a/traffic_ops/ort/atstccfg/toreq/caching.go
+++ b/traffic_ops/ort/atstccfg/toreq/caching.go
@@ -77,7 +77,7 @@ func WriteCacheJSON(tempDir string, cacheFileName string, obj interface{}) {
objPath := filepath.Join(tempDir, cacheFileName)
// Use os.OpenFile, not os.Create, in order to set perms to 0600 - cookies allow login, therefore the file MUST only allow access by the current user, for security reasons
- objFile, err := os.OpenFile(objPath, os.O_RDWR|os.O_CREATE, 0600)
+ objFile, err := os.OpenFile(objPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
if err != nil {
log.Errorln("creating object cache file '" + objPath + "': " + err.Error())
return
@@ -116,6 +116,7 @@ func GetJSONObjFromFile(tempDir string, cacheFileName string, cacheFileMaxAge ti
}
if err := json.Unmarshal(bts, obj); err != nil {
+ log.Errorln("GetJSONObjFromFile loaded '" + cacheFileName + "' but failed to parse JSON: " + err.Error())
return errors.New("unmarshalling object from file '" + objPath + "':" + err.Error())
}
diff --git a/traffic_ops/ort/atstccfg/toreq/caching_test.go b/traffic_ops/ort/atstccfg/toreq/caching_test.go
new file mode 100644
index 0000000..160254f
--- /dev/null
+++ b/traffic_ops/ort/atstccfg/toreq/caching_test.go
@@ -0,0 +1,68 @@
+package toreq
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+ "os"
+ "path/filepath"
+ "testing"
+ "time"
+)
+
+func TestWriteCacheJSON(t *testing.T) {
+ type Obj struct {
+ S string `json:"s"`
+ }
+
+ fileName := "TestWriteCacheJSON.json"
+ tmpDir := os.TempDir()
+ filePath := filepath.Join(tmpDir, fileName)
+ defer os.Remove(filePath)
+
+ {
+ largeObj := Obj{
+ S: `
+ Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
+
+ Curabitur pretium tincidunt lacus. Nulla gravida orci a odio. Nullam varius, turpis et commodo pharetra, est eros bibendum elit, nec luctus magna felis sollicitudin mauris. Integer in mauris eu nibh euismod gravida. Duis ac tellus et risus vulputate vehicula. Donec lobortis risus a elit. Etiam tempor. Ut ullamcorper, ligula eu tempor congue, eros est euismod turpis, id tincidunt sapien risus a quam. Maecenas fermentum consequat mi. Donec fermentum. Pellentesque malesuada nulla a mi. [...]
+`,
+ }
+
+ WriteCacheJSON(tmpDir, fileName, largeObj)
+ loadedLargeObj := Obj{}
+ if err := GetJSONObjFromFile(tmpDir, fileName, time.Hour, &loadedLargeObj); err != nil {
+ t.Fatalf("GetJSONObjFromFile large error expected nil, actual: " + err.Error())
+ } else if largeObj.S != loadedLargeObj.S {
+ t.Fatalf("GetJSONObjFromFile expected %+v actual %+v\n", largeObj.S, loadedLargeObj.S)
+ }
+ }
+
+ {
+ // Write a smaller object to the same file, to make sure it properly truncates, and doesn't leave old text lying around (resulting in malformed json)
+ smallObj := Obj{S: `foo`}
+ WriteCacheJSON(tmpDir, fileName, smallObj)
+ loadedSmallObj := Obj{}
+ if err := GetJSONObjFromFile(tmpDir, fileName, time.Hour, &loadedSmallObj); err != nil {
+ t.Fatalf("GetJSONObjFromFile small error expected nil, actual: " + err.Error())
+ } else if smallObj.S != loadedSmallObj.S {
+ t.Fatalf("GetJSONObjFromFile expected %+v actual %+v\n", smallObj.S, loadedSmallObj.S)
+ }
+ }
+}
diff --git a/traffic_ops/ort/atstccfg/toreq/toreq.go b/traffic_ops/ort/atstccfg/toreq/toreq.go
index a1a90a6..c0fec11 100644
--- a/traffic_ops/ort/atstccfg/toreq/toreq.go
+++ b/traffic_ops/ort/atstccfg/toreq/toreq.go
@@ -212,21 +212,33 @@ func GetCacheGroups(cfg config.TCCfg) ([]tc.CacheGroupNullable, error) {
return cacheGroups, nil
}
+// DeliveryServiceServersAlwaysGetAll indicates whether to always get all delivery service servers from Traffic Ops, and cache all in a file (but still return to the caller only the objects they requested).
+// This exists and is currently true, because with an ORT run, it's typically more efficient to get them all in a single request, and re-use that cache; than for every config file to get and cache its own unique set.
+// If your use case is more efficient to only get the needed objects, for example if you're frequently requesting one file, set this false to get and cache the specific needed delivery services and servers.
+const DeliveryServiceServersAlwaysGetAll = true
+
func GetDeliveryServiceServers(cfg config.TCCfg, dsIDs []int, serverIDs []int) ([]tc.DeliveryServiceServer, error) {
- sortIDsInHash := true
+ const sortIDsInHash = true
+
serverIDsStr := ""
- if len(serverIDs) > 0 {
- serverIDsStr = base64.RawURLEncoding.EncodeToString((util.HashInts(serverIDs, sortIDsInHash)))
- }
dsIDsStr := ""
- if len(dsIDs) > 0 {
- dsIDsStr = base64.RawURLEncoding.EncodeToString((util.HashInts(dsIDs, sortIDsInHash)))
+ dsIDsToFetch := ([]int)(nil)
+ sIDsToFetch := ([]int)(nil)
+ if !DeliveryServiceServersAlwaysGetAll {
+ if len(dsIDs) > 0 {
+ dsIDsStr = base64.RawURLEncoding.EncodeToString((util.HashInts(dsIDs, sortIDsInHash)))
+ }
+ if len(serverIDs) > 0 {
+ serverIDsStr = base64.RawURLEncoding.EncodeToString((util.HashInts(serverIDs, sortIDsInHash)))
+ }
+ dsIDsToFetch = dsIDs
+ sIDsToFetch = serverIDs
}
dsServers := []tc.DeliveryServiceServer{}
err := GetCachedJSON(cfg, "deliveryservice_servers_s"+serverIDsStr+"_d_"+dsIDsStr+".json", &dsServers, func(obj interface{}) error {
const noLimit = 999999 // TODO add "no limit" param to DSS endpoint
- toDSS, reqInf, err := (*cfg.TOClient).GetDeliveryServiceServersWithLimits(noLimit, dsIDs, serverIDs)
+ toDSS, reqInf, err := (*cfg.TOClient).GetDeliveryServiceServersWithLimits(noLimit, dsIDsToFetch, sIDsToFetch)
if err != nil {
return errors.New("getting delivery service servers from Traffic Ops '" + MaybeIPStr(reqInf) + "': " + err.Error())
}