[GitHub] [trafficcontrol] traeak commented on a change in pull request #4493: Change ORT/atstccfg to generate all files at once, massive performance improvement

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

traeak commented on a change in pull request #4493: Change ORT/atstccfg to generate all files at once, massive performance improvement
URL: https://github.com/apache/trafficcontrol/pull/4493#discussion_r393697493
 
 

 ##########
 File path: traffic_ops/ort/traffic_ops_ort.pl
 ##########
 @@ -1833,159 +1558,103 @@ sub check_log_level {
 	}
 }
 
-sub set_domainname {
-	my $hostname;
-	if ($RELEASE eq "EL7") {
-		$hostname = `cat /etc/hostname`;
-		chomp($hostname);
-	} else {
-		$hostname = `cat /etc/sysconfig/network | grep HOSTNAME`;
-		chomp($hostname);
-		$hostname =~ s/HOSTNAME\=//g;
-	}
-	chomp($hostname);
-	$hostname =~ s/HOSTNAME\=//g;
-	my $domainname;
-	( my @parts ) = split( /\./, $hostname );
-	for ( my $i = 1; $i < scalar(@parts); $i++ ) {
-		$domainname .= $parts[$i] . ".";
-	}
-	$domainname =~ s/\.$//g;
-	return $domainname;
-}
-
+# get_cfg_file_list gets the config files from atstccfg via Traffic Ops.
+# See parse_multipart_config_files for the return type.
 sub get_cfg_file_list {
 	my $host_name = shift;
-	my $tm_host   = shift;
 	my $script_mode = shift;
-	my $cfg_files;
-	my $profile_name;
-	my $cdn_name;
-	my $uri = "/api/2.0/servers/$host_name/configfiles/ats"; #This doesn't actually exist in 2.0, but atstccfg doesn't care
 
-	my $result = &lwp_get($uri);
+	my $atstccfg_reval_arg = '';
+	if ( $script_mode == $REVALIDATE ) {
+		$atstccfg_reval_arg = '--revalidate-only';
+	}
 
-	if ($result eq '404') {
-		$api_in_use = 0;
-		( $log_level >> $ERROR ) && printf("ERROR Traffic Ops version does not support config files API. Please upgrade to Traffic Ops 2.2.\n");
+	my $result = `$atstccfg_cmd --traffic-ops-user='$TO_USER' --traffic-ops-password='$TO_PASS' --traffic-ops-url='$TO_URL' --cache-host-name='$host_name' $atstccfg_reval_arg --log-location-error=stderr --log-location-warning=stderr --log-location-info=null 2>$atstccfg_log_path`;
+	my $atstccfg_exit_code = $?;
+	if ($atstccfg_exit_code != 0) {
+		( $log_level >> $ERROR ) && printf("ERROR getting config files from atstccfg via Traffic Ops. See $atstccfg_log_path for details\n");
 		exit 1;
 	}
 
-	my $ort_ref = decode_json($result);
+	return &parse_multipart_config_files($result);
+}
 
-	if ($api_in_use == 1) {
-		$to_rev_proxy_url = $ort_ref->{'info'}->{'toRevProxyUrl'};
-		if ( $to_rev_proxy_url && $rev_proxy_disable == 0 ) {
-			$to_rev_proxy_url =~ s/\/*$//g;
-			# Note: If traffic_ops_url is changing, would be suggested to get a new cookie.
-			#       Secrets might not be the same on all Traffic Ops instance.
-			$traffic_ops_host = $to_rev_proxy_url;
-			$rev_proxy_in_use = 1;
-			( $log_level >> $INFO ) && printf("INFO Found Traffic Ops Reverse Proxy URL from Traffic Ops: $to_rev_proxy_url\n");
-		} else {
-			if ( $rev_proxy_disable == 1 ) {
-				( $log_level >> $INFO ) && printf("INFO Reverse proxy disabled - connecting directly to traffic ops for all files.\n");
-			}
-			$traffic_ops_host = $to_url;
-		}
-		$profile_name = $ort_ref->{'info'}->{'profileName'};
-		( $log_level >> $INFO ) && printf("INFO Found profile from Traffic Ops: $profile_name\n");
-		$cdn_name = $ort_ref->{'info'}->{'cdnName'};
-		( $log_level >> $INFO ) && printf("INFO Found CDN_name from Traffic Ops: $cdn_name\n");
-		$server_tcp_port = $ort_ref->{'info'}->{'serverTcpPort'};
-		( $log_level >> $INFO ) && printf("INFO Found cache server tcp port from Traffic Ops: $server_tcp_port\n");
-		$server_ipv4 = $ort_ref->{'info'}->{'serverIpv4'};
-		( $log_level >> $INFO ) && printf("INFO Found cache server ipv4 from Traffic Ops: $server_ipv4\n");
+# parse_multipart_config_files parses the multipart/mixed message returned by atstccfg, and returns a map of file names to content.
+# Returns a hash of the file name to a sub-hash. Each sub-hash contains the values 'headers' and 'body'.
+#  The 'headers' is a hash of lowercased header names to header values.
+#  The 'body' is the string body
+sub parse_multipart_config_files {
+	my $multipart_txt = shift;
+
+	# Note this doesn't enforce RFC compliance for the boundary. We assume any char is valid, where in reality only certain characters are allowed in the unquoted value. See RFC2616s3.6
+	my $boundary = '';
+	if ($multipart_txt =~ m/boundary="/) {
+		($boundary) = $multipart_txt =~ m/boundary="((?:[^"\\]|\\.)*)"/g; # this regex gets the quoted boundary="foo" value
+	} else {
+		($boundary) = $multipart_txt =~ m/boundary=([^ \r\n\t]+)/g; # this regex gets the unquoted boundary=foo value
 	}
-	else {
-		$profile_name = $ort_ref->{'profile'}->{'name'};
-		( $log_level >> $INFO ) && printf("INFO Found profile from Traffic Ops: $profile_name\n");
-		$cdn_name = $ort_ref->{'other'}->{'CDN_name'};
-		( $log_level >> $INFO ) && printf("INFO Found CDN_name from Traffic Ops: $cdn_name\n");
+
+	if ( length($boundary) == 0 ) {
+		( $log_level >> $FATAL ) && print "FATAL Error getting package list from Traffic Ops! Could not find boundary for multipart message\n";
+		exit 1;
 	}
-	if ( $script_mode == $REVALIDATE ) {
-		foreach my $cfg_file ( @{$ort_ref->{'configFiles'}} ) {
-			if ( $cfg_file->{'fnameOnDisk'} eq "regex_revalidate.config" ) {
-				my $fname_on_disk = &get_filename_on_disk( $cfg_file->{'fnameOnDisk'} );
-				( $log_level >> $INFO )
-					&& printf( "INFO Found config file (on disk: %-41s): %-41s with location: %-50s\n", $fname_on_disk, $cfg_file->{'fnameOnDisk'}, $cfg_file->{'location'} );
-				$cfg_files->{$fname_on_disk}->{'location'} = $cfg_file->{'location'};
-				if ($api_in_use == 1) {
-					$cfg_files->{$fname_on_disk}->{'apiUri'} = $cfg_file->{'apiUri'};
-				}
-				$cfg_files->{$fname_on_disk}->{'fname-in-TO'} = $cfg_file->{'fnameOnDisk'};
-			}
+
+	my @files = split("--" . $boundary . "\r\n", $multipart_txt);
+
+	my %all_files;
+	for my $i (1 .. $#files) { # start at 1, because 0 is the MIME-Version and Content-Type: multipart/mixed
+		my $file = $files[$i];
+		my @headers_body = split("\r\n\r\n", $file);
+
+		if ( @headers_body < 2 ) {
+			print "FATAL Error getting package list from Traffic Ops! malformed headers on file $i\n";
+			exit 1;
 		}
-	}
-	else {
-		if ( $reval_in_use == 1 ) {
-			( $log_level >> $WARN ) && printf("WARN Instant Invalidate is enabled.  Skipping regex_revalidate.config.\n");
-			if ( $api_in_use == 1 ) {
-				my @new = grep { $_->{'fnameOnDisk'} ne 'regex_revalidate.config' } @{$ort_ref->{'configFiles'}};
-				$ort_ref->{'configFiles'} = \@new;
-			}
-			else {
-				delete $ort_ref->{'config_files'}->{'regex_revalidate.config'};
-			}
-		}
-		if ( $api_in_use == 1 ) {
-			foreach my $cfg_file (@{$ort_ref->{'configFiles'}} ) {
-				my $fname_on_disk = &get_filename_on_disk( $cfg_file->{'fnameOnDisk'} );
-				( $log_level >> $INFO )
-					&& printf( "INFO Found config file (on disk: %-41s): %-41s with location: %-50s\n", $fname_on_disk, $cfg_file->{'fnameOnDisk'}, $cfg_file->{'location'} );
-				$cfg_files->{$fname_on_disk}->{'location'} = $cfg_file->{'location'};
-				if ( defined($cfg_file->{'apiUri'} ) ) {
-					$cfg_files->{$fname_on_disk}->{'apiUri'} = $cfg_file->{'apiUri'};
-					( $log_level >> $DEBUG ) && print "DEBUG apiUri found: $cfg_files->{$fname_on_disk}->{'apiUri'}.\n";
-				}
-				elsif ( defined($cfg_file->{'url'} ) ) {
-					$cfg_files->{$fname_on_disk}->{'url'} = $cfg_file->{'url'};
-					( $log_level >> $DEBUG ) && print "DEBUG URL found: $cfg_files->{$fname_on_disk}->{'url'}.\n";
-				}
-				$cfg_files->{$fname_on_disk}->{'fname-in-TO'} = $cfg_file->{'fnameOnDisk'};
 
-			}
+		my @headers_arr = split("\r\n", $headers_body[0]);
+		my %headers;
+		for my $i (0 .. $#headers_arr) {
+				my $header = $headers_arr[$i];
+				my @header_name_val = split(": ", $header, 2);
+				my $header_name = lc($header_name_val[0]);
+				my $header_val = $header_name_val[1];
+				$headers{$header_name} = $header_val;
 		}
-		else {
-			foreach my $cfg_file ( keys %{ $ort_ref->{'config_files'} } ) {
-				my $fname_on_disk = &get_filename_on_disk( $cfg_file );
-				( $log_level >> $INFO )
-					&& printf( "INFO Found config file (on disk: %-41s): %-41s with location: %-50s\n", $fname_on_disk, $cfg_file, $ort_ref->{'config_files'}->{$cfg_file}->{'location'} );
-				$cfg_files->{$fname_on_disk}->{'location'} = $ort_ref->{'config_files'}->{$cfg_file}->{'location'};
-				if ($api_in_use == 1) {
-					$cfg_files->{$fname_on_disk}->{'apiUri'} = $cfg_file->{'apiUri'};
-				}
-				$cfg_files->{$fname_on_disk}->{'fname-in-TO'} = $cfg_file;
-			}
+
+		my $file_name = basename($headers{'path'});
+		if ( length($file_name) == 0 ) {
+			print "FATAL Error getting package list from Traffic Ops! Headers on file $i missing Path!\n";
+			exit 1;
 		}
+
+		my %file_obj;
+		$file_obj{'headers'} = \%headers;
+		$file_obj{'body'} = $headers_body[1];
 
 Review comment:
   		$file_obj{'body'} = substr($headers_body[1], 0, -2); // remove trailiing \r\n
   

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