You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by el...@apache.org on 2018/06/14 17:10:20 UTC
[trafficcontrol] 05/08: Addressing Rawlin's comments: 1. Avoiding
multiple DB querries using Join 2. &log to error logs
This is an automated email from the ASF dual-hosted git repository.
elsloo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
commit cf2b35f2ebb76019b13f377276d42e379e5fc596
Author: Vijayanand Subramanian <vi...@gmail.com>
AuthorDate: Mon May 28 08:22:40 2018 -0400
Addressing Rawlin's comments:
1. Avoiding multiple DB querries using Join
2. &log to error logs
---
lib/go-tc/cachegroupfallback.go | 24 +++++++-------
...sql => 20180528000000_cache_group_fallback.sql} | 4 +--
traffic_ops/app/lib/API/CachegroupFallback.pm | 37 ++++++++--------------
.../traffic_ops_golang/crconfig/edgelocations.go | 28 +++++++---------
4 files changed, 39 insertions(+), 54 deletions(-)
diff --git a/lib/go-tc/cachegroupfallback.go b/lib/go-tc/cachegroupfallback.go
index 7e1f95b..e7ffc18 100644
--- a/lib/go-tc/cachegroupfallback.go
+++ b/lib/go-tc/cachegroupfallback.go
@@ -20,24 +20,24 @@ package tc
* under the License.
*/
-// A List of CACHEGROUPFALLBACKs Response
-// swagger:response CACHEGROUPFALLBACKsResponse
+// A List of cachegroupFallbacks Response
+// swagger:response cachegroupFallbacksResponse
// in: body
-type CACHEGROUPFALLBACKsResponse struct {
+type cachegroupFallbacksResponse struct {
// in: body
- Response []CACHEGROUPFALLBACK `json:"response"`
+ Response []cachegroupFallback `json:"response"`
}
-// A Single CACHEGROUPFALLBACK Response for Update and Create to depict what changed
-// swagger:response CACHEGROUPFALLBACKResponse
+// A Single cachegroupFallback Response for Update and Create to depict what changed
+// swagger:response cachegroupFallbackResponse
// in: body
-type CACHEGROUPFALLBACKResponse struct {
+type cachegroupFallbackResponse struct {
// in: body
- Response CACHEGROUPFALLBACK `json:"response"`
+ Response cachegroupFallback `json:"response"`
}
-// CACHEGROUPFALLBACK ...
-type CACHEGROUPFALLBACK struct {
+// cachegroupFallback ...
+type cachegroupFallback struct {
PrimaryCgId int `json:"primaryId" db:"primary_cg"`
BackupCgId int `json:"backupId" db:"backup_cg"`
@@ -45,8 +45,8 @@ type CACHEGROUPFALLBACK struct {
}
-// CACHEGROUPFALLBACKNullable ...
-type CACHEGROUPFALLBACKNullable struct {
+// cachegroupFallbackNullable ...
+type cachegroupFallbackNullable struct {
PrimaryCgId *int `json:"primaryId" db:"primary_cg"`
BackupCgId *int `json:"backupId" db:"backup_cg"`
diff --git a/traffic_ops/app/db/migrations/20180521000000_cache_group_fallback.sql b/traffic_ops/app/db/migrations/20180528000000_cache_group_fallback.sql
similarity index 93%
rename from traffic_ops/app/db/migrations/20180521000000_cache_group_fallback.sql
rename to traffic_ops/app/db/migrations/20180528000000_cache_group_fallback.sql
index 9238957..dbce434 100644
--- a/traffic_ops/app/db/migrations/20180521000000_cache_group_fallback.sql
+++ b/traffic_ops/app/db/migrations/20180528000000_cache_group_fallback.sql
@@ -19,8 +19,8 @@
-- cachegroup_fallbacks
CREATE TABLE cachegroup_fallbacks (
- primary_cg bigint,
- backup_cg bigint CHECK (primary_cg != backup_cg),
+ primary_cg bigint NOT NULL,
+ backup_cg bigint CHECK (primary_cg != backup_cg) NOT NULL,
set_order bigint NOT NULL,
CONSTRAINT fk_primary_cg FOREIGN KEY (primary_cg) REFERENCES cachegroup(id) ON DELETE CASCADE,
CONSTRAINT fk_backup_cg FOREIGN KEY (backup_cg) REFERENCES cachegroup(id) ON DELETE CASCADE,
diff --git a/traffic_ops/app/lib/API/CachegroupFallback.pm b/traffic_ops/app/lib/API/CachegroupFallback.pm
index 0dfde46..888fd0e 100644
--- a/traffic_ops/app/lib/API/CachegroupFallback.pm
+++ b/traffic_ops/app/lib/API/CachegroupFallback.pm
@@ -52,10 +52,10 @@ sub delete {
&log( $self, "Backup configuration DELETED", "APICHANGE");
return $self->success_message("Backup configuration DELETED");
} else {
- return $self->alert( "Backup configuration DELETED." );
+ return $self->alert( "Backup configuration DELETE Failed!." );
}
} else {
- &log( $self, "No backup Cachegroups found");
+ $self->app->log->error("No backup Cachegroups found");
return $self->not_found();
}
}
@@ -66,21 +66,10 @@ sub show {
my $fallback_id = $self->param("fallbackId");
my $id = $cache_id ? $cache_id : $fallback_id;
- #only integers
- if ( $id !~ /^\d+?$/ ) {
- &log( $self, "No such Cachegroup id $id");
- return $self->success([]);
- }
+ my ( $is_valid, $result ) = $self->is_valid_cachegroup_fallback(undef, $cache_id);
- my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single();
- if ( !defined($cachegroup) ) {
- &log( $self, "No such Cachegroup $id");
- return $self->success([]);
- }
-
- if ( ($cachegroup->type->name ne "EDGE_LOC") ) {
- &log( $self, "cachegroup should be type EDGE_LOC.");
- return $self->success([]);
+ if ( !$is_valid ) {
+ return $self->alert($result);
}
my $rs_backups = undef;
@@ -106,7 +95,7 @@ sub show {
}
return $self->success( $response );
} else {
- &log( $self, "No backup Cachegroups");
+ $self->app->log->error("No backup Cachegroups");
return $self->success([]);
}
}
@@ -138,12 +127,12 @@ sub create {
foreach my $config (@{ $params }) {
my $rs_backup = $self->db->resultset('Cachegroup')->search( { id => $config->{fallbackId} } )->single();
if ( !defined($rs_backup) ) {
- &log( $self, "ERROR Backup config: No such Cache Group $config->{fallbackId}");
+ $self->app->log->error("ERROR Backup config: No such Cache Group $config->{fallbackId}");
next;
}
if ( ($rs_backup->type->name ne "EDGE_LOC") ) {
- &log( $self, "ERROR Backup config: $config->{name} is not EDGE_LOC");
+ $self->app->log->error("ERROR Backup config: $config->{name} is not EDGE_LOC");
next;
}
@@ -160,7 +149,7 @@ sub create {
my $rs_data = $self->db->resultset('CachegroupFallback')->create($values)->insert();
if ( !defined($rs_data)) {
- &log( $self, "Database operation for backup configuration for cache group $cache_id failed.");
+ $self->app->log->error("Database operation for backup configuration for cache group $cache_id failed.");
}
}
@@ -216,12 +205,12 @@ sub update {
foreach my $config (@{ $params }) {
my $rs_backup = $self->db->resultset('Cachegroup')->search( { id => $config->{fallbackId} } )->single();
if ( !defined($rs_backup) ) {
- &log( $self, "ERROR Backup config: No such Cache Group $config->{fallbackId}");
+ $self->app->log->error("ERROR Backup config: No such Cache Group $config->{fallbackId}");
next;
}
if ( ($rs_backup->type->name ne "EDGE_LOC") ) {
- &log( $self, "ERROR Backup config: $config->{name} is not EDGE_LOC");
+ $self->app->log->error("ERROR Backup config: $config->{name} is not EDGE_LOC");
next;
}
@@ -263,12 +252,12 @@ sub is_valid_cachegroup_fallback {
my $cache_id = shift;
if ( $cache_id !~ /^\d+?$/ ) {
- return ( 0, "Invalid cachegroup id" );
+ return ( 0, "Invalid cachegroup id, should be an integer" );
}
my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $cache_id } )->single();
if ( !defined($cachegroup) ) {
- return ( 0, "Invalid cachegroup id, should be an integer" );
+ return ( 0, "Invalid cachegroup id" );
}
if ( ($cachegroup->type->name ne "EDGE_LOC") ) {
diff --git a/traffic_ops/traffic_ops_golang/crconfig/edgelocations.go b/traffic_ops/traffic_ops_golang/crconfig/edgelocations.go
index 1d52356..b981aa5 100644
--- a/traffic_ops/traffic_ops_golang/crconfig/edgelocations.go
+++ b/traffic_ops/traffic_ops_golang/crconfig/edgelocations.go
@@ -32,7 +32,7 @@ func makeLocations(cdn string, db *sql.DB) (map[string]tc.CRConfigLatitudeLongit
// TODO test whether it's faster to do a single query, joining lat/lon into servers
q := `
-select cg.name, t.name as type, cg.latitude, cg.longitude from cachegroup as cg
+select cg.name, cg.id, t.name as type, cg.latitude, cg.longitude from cachegroup as cg
inner join server as s on s.cachegroup = cg.id
inner join type as t on t.id = s.type
inner join status as st ON st.id = s.status
@@ -49,20 +49,20 @@ and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN')
for rows.Next() {
cachegroup := ""
+ primaryCacheID := ""
ttype := ""
latlon := tc.CRConfigLatitudeLongitude{}
- if err := rows.Scan(&cachegroup, &ttype, &latlon.Lat, &latlon.Lon); err != nil {
+ if err := rows.Scan(&cachegroup, &primaryCacheID, &ttype, &latlon.Lat, &latlon.Lon); err != nil {
return nil, nil, errors.New("Error scanning cachegroup: " + err.Error())
}
if ttype == RouterTypeName {
routerLocs[cachegroup] = latlon
} else {
- primaryCacheId := ""
- if err := db.QueryRow(`select id from cachegroup where name = $1`, cachegroup).Scan(&primaryCacheId); err != nil {
- return nil, nil, errors.New("Failed while retrieving from cachegroup: " + err.Error())
- }
-
- dbRows, err := db.Query(`select backup_cg from cachegroup_fallbacks where primary_cg = $1 order by set_order`, primaryCacheId)
+ q := `select cachegroup.name from cachegroup_fallbacks
+join cachegroup on cachegroup_fallbacks.backup_cg = cachegroup.id
+and cachegroup_fallbacks.primary_cg = $1
+`
+ dbRows, err := db.Query(q, primaryCacheID)
if err != nil {
return nil, nil, errors.New("Error retrieving from cachegroup_fallbacks: " + err.Error())
@@ -71,20 +71,16 @@ and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN')
index := 0
for dbRows.Next() {
- backup_id := ""
- backup_name := ""
- if err := dbRows.Scan(&backup_id); err != nil {
+ backupName := ""
+ if err := dbRows.Scan(&backupName); err != nil {
return nil, nil, errors.New("Error while scanning from cachegroup_fallbacks: " + err.Error())
- }
- if err := db.QueryRow(`select name from cachegroup where id = $1`, backup_id).Scan(&backup_name); err != nil {
- return nil, nil, errors.New("Error scanning cachegroup: " + err.Error())
} else {
- latlon.BackupLocations.List = append(latlon.BackupLocations.List, backup_name)
+ latlon.BackupLocations.List = append(latlon.BackupLocations.List, backupName)
index++
}
}
- if err := dbRows.Err(); err != nil {
+ if err := dbRows.Err(); err != nil {
return nil, nil, errors.New("Error iterating cachegroup_fallbacks rows: " + err.Error())
}
edgeLocs[cachegroup] = latlon
--
To stop receiving notification emails like this one, please contact
elsloo@apache.org.