Andy Kurth created VCL-815:
------------------------------
Summary: Problems with update_cluster in OS.pm
Key: VCL-815
URL: https://issues.apache.org/jira/browse/VCL-815
Project: VCL
Issue Type: Bug
Components: vcld (backend)
Reporter: Andy Kurth
Assignee: Andy Kurth
Fix For: 2.4
*Problem 1:*
Due to OS.pm::update_cluster, the processing of the reserved state for each
computer is taking an extremely long time. A fairly large cluster of 80
computers is taking over 20 minutes to complete, not including the time it
spends waiting for the user to click Connect and to actually connect to the
computer. This is longer than it took to actually load the bare-metal
computers via xCAT.
*Problem 2:*
Every reservation that belongs to a cluster request first creates its own
cluster_info file in /tmp. The file that each creates is identical. Every
reservation then proceeds to copy the file it created to every other computer
in the cluster request via SCP. So, for an 80 node cluster, cluster_info files
are needlessly copied back and forth 6,400 times.
*Problem 3:*
Each node does not copy the cluster_info file it created in /tmp to the proper
location on itself. Instead, it sends it to itself via SCP. There's really no
need for this because the file will end up in the proper location when the
other 79 nodes SCP it over.
*Problem 4:*
The subroutine is using the following to determine where to send the file to
via SCP:
{noformat}
my $node_name = $request_data->{reservation}{$resid}{computer}{SHORTNAME};
{noformat}
If you're not familiar with the backend code, the remote computer's short
hostname is being used. For example, if the FQDN is vm9.example.com, the code
is trying to send the file to "vm9". This will only work if "vm9" resolves to
an address the management node can get to. This will not work for distributed
management nodes residing in different locations. Furthermore, with the move
away from relying on /etc/hosts, whenever a hostname is passed to
run_scp_command, the computer's private IP address will be used. Examine the
determine_remote_connection_target for more information. If absolutely
necessary to SCP files back and forth, the public IP address must be used.
*Problem 5:*
The update_cluster subroutine is attempting to open the firewall on each
computer for incoming traffic from the other computers in the cluster. This is
good, but the way it is implemented is very slow an inefficient. It is also
not reliable due to the way the enable_firewall_port subroutines are handling
things. Errors are being generated because the command being generated is too
large:
{noformat}
|10435|2626116|2726142|reserved|Linux.pm:enable_firewall_port|3828| ----
WARNING ----
|10435|2626116|2726142|reserved|Linux.pm:enable_firewall_port|3828| 2015-02-04
16:28:26|10435|2626116|2726142|reserved|Linux.pm:enable_firewall_port|3828|failed
to enable firewall port on blade1f5-5, protocol: tcp, port: any, scope:
<omitted>, exit status: 1, command:
|10435|2626116|2726142|reserved|Linux.pm:enable_firewall_port|3828| iptables -D
INPUT 9 && iptables -D INPUT 8 && iptables -D INPUT 7 && iptables -D INPUT 6 &&
iptables -D INPUT 5 && iptables -D INPUT 4 && iptables -D INPUT 3 && iptables
-D INPUT 2 && iptables -D INPUT 10 && iptables -D INPUT 1 && /sbin/iptables -v
-I INPUT 1 -p tcp -j ACCEPT -s 10.25.0.241 -m state --state
NEW,RELATED,ESTABLISHED &&
...much more omitted...
|10435|2626116|2726142|reserved|Linux.pm:enable_firewall_port|3828| output:
|10435|2626116|2726142|reserved|Linux.pm:enable_firewall_port|3828| iptables:
Index of deletion too big.
|10435|2626116|2726142|reserved|Linux.pm:enable_firewall_port|3828| ( 0)
Linux.pm, enable_firewall_port (line: 3828)
|10435|2626116|2726142|reserved|Linux.pm:enable_firewall_port|3828| (-1) OS.pm,
update_cluster (line: 3909)
|10435|2626116|2726142|reserved|Linux.pm:enable_firewall_port|3828| (-2)
reserved.pm, process (line: 157)
|10435|2626116|2726142|reserved|Linux.pm:enable_firewall_port|3828| (-3) vcld,
make_new_child (line: 587)
|10435|2626116|2726142|reserved|Linux.pm:enable_firewall_port|3828| (-4) vcld,
main (line: 348)
{noformat}
*Problem 6:*
The subroutine is calling $self->data->get_request_data() and then accessing
the hash data directly. Example:
{noformat}
$request_data->{reservation}{$rid}{computer}{IPaddress}
{noformat}
The past few years have been spent trying to get rid of code which directly
accesses the hash constructed by get_request_info. There are functions built
into DataStructure.pm which may be accessed via $self->data to retrieve this
information, including information about other reservations in the cluster.
The get_request_data function should only be used when absolutely necessary --
such as from DataStructure.pm or if the entire hash is needed to create another
module object.
*Problem 7:*
The reserved.pm::process subroutine calculates the exact time it should timeout
a reservation due to no initial connection by taking the time the user clicked
connect and adding the initialconnecttimeout variable value to it. The
update_cluster subroutine gets run in the meantime. As it is currently
written, it can take longer than the entire initialconnecttimeout value. When
control is passed back to reserved.pm::process, the code realizes the initial
connection expire time is in the past so connection checking isn't being done
at all.
*Problem 8:*
The code includes the following:
{noformat}
if ($image_OS_type =~ /linux/i) {
...
}
elsif ($image_OS_type =~ /windows/i) {
...
}
{noformat}
The backend is architected so that these if/else statements should be avoided
whenever possible. Sure, an if/else such as this is quick and easy but the
more elegant way to handle it would be to add a commonly named subroutine to
the Linux and Windows modules that handles the specifics of this statement.
Again, work has been done over the past few years to rework code such as this.
I see no reason to keep adding it back.
Problem 9:
"$image_OS_type" - There are over 9,000 variables defined in the backend code.
Variables intended to act as constants for an entire module are all uppercase.
Nearly all other variables are entirely lowercase, except for a few in some
very old code. I see no reason to mix case.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
