[jira] [Commented] (VCL-815) Problems with update_cluster in OS.pm

["ASF subversion and git services (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/VCL-815?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14315193#comment-14315193 ] 

ASF subversion and git services commented on VCL-815:
-----------------------------------------------------

Commit 1658839 from [~arkurth] in branch 'vcl/trunk'
[ https://svn.apache.org/r1658839 ]

VCL-815
Added DataStructure.pm::get_child_reservation_ids. It will be used by OS.pm::update_cluster.

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