trafficserver-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Igor Galić <i.ga...@brainsware.org>
Subject Re: trafficserver pull request: Magical Mystery Tour through VMap, vaddrs.conf.
Date Thu, 28 Feb 2013 21:54:35 GMT


----- Original Message -----
> GitHub user jkew opened a pull request:
> 
>     https://github.com/apache/trafficserver/pull/6
> 
>     Magical Mystery Tour through VMap, vaddrs.conf.
> 
>     CLEANUP: Virtual IPs were once managed such that within a cluster
>     they
>            would automatically rebalance themselves between nodes by
>            bringing
>            subinterfaces up and down. After ATS was open sourced the
>            original
>            setuid tool, vip_config (or traffic_vip_config) was
>            inadvertently
>            removed; but the code which depended on this tool was not
>            cleaned
>            up.
>     
>            Since modern deployments either do not use this tool at
>            all
>            (because it was broken for a few years) and modern
>            deployments also
>            have some central system for managing cluster state
>            reliably, we
>            do not need VMap to implement some scheme for
>            automatically
>            rebalancing the ips.
>     
>            This CL keeps much of the code for detecting ip address
>            conflicts
>            and for receiving the multicast messages from the cluster;
>            but we
>            remove all instances where we either bring up/down an
>            interface.
>     
>            Deployments should manage this through external state
>            systems.
>     
>            Note: VIPs do not actually bind to the specific addresses
>            in
>            vaddrs; this is just an operations convience to ensure
>            that a
>            cluster has no ip conflicts or unmanaged vips.
>     
>            Note: The *right* thing to do here may be to recall that
>            old tool and let
>            VMap do it's thing.
>     
>            Note: Another *right* thing to do may be to remove VMap
>            entirely, along
>            with the associated cluster messages. At least with this
>            exiting changeset
>            we can detect ip address conflicts.
>     
>     Reviewed-by: TBD
> 
> You can merge this pull request into a Git repository by running:
> 
>     $ git pull https://github.com/jkew/trafficserver
>     remove_vip_rebalance2


This is removing code. A lot of code. Excellent.  I like when we remove
lots of code. Especially code we don't need or use any longer.

+1 -- except for this one 

+  char str_addr[1024];

we should use TS_MAX_SOMETHING_OR_THE_OTHER.

-- i

> 
> Alternatively you can review and apply these changes as the patch at:
> 
>     https://github.com/apache/trafficserver/pull/6.patch
> 
> ----
> commit 3433a4ac96ae95c9a630a33b9bb0a32df5060079
> Author: John Kew <john.kew@socrata.com>
> Date:   2013-02-28T10:57:25Z
> 
>     CLEANUP: Virtual IPs were once managed such that within a cluster
>     they
>            would automatically rebalance themselves between nodes by
>            bringing
>            subinterfaces up and down. After ATS was open source the
>            original
>            setuid tools, vip_config (or traffic_vip_config) was
>            inadvertently
>            removed; but the code which depended on this tool was not
>            cleaned
>            up.
>     
>            Since modern deployments either do not use this tool at
>            all
>            (because it was broken for a few years) and modern
>            deployments also
>            have some central system for managing cluster state
>            reliably, we
>            do not need VMap to implement some scheme for
>            automatically
>            rebalancing the ips.
>     
>            This CL keeps much of the code for detecting ip address
>            conflicts
>            and for receiving the multicast messages from the cluster;
>            but we
>            remove all instances where we either bring up/down an
>            interface.
>     
>            Deployments should manage this through external state
>            systems.
>     
>            Note: VIPs do not actually bind to the specific addresses
>            in
>            vaddrs; this is just an operations convience to ensure
>            that a
>            cluster has no ip conflicts or unmanaged vips.
>     
>     Reviewed-by: TBD
> 
> ----
> 
> 

-- 
Igor Galić

Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 6880 4155 74BD FD7C B515  2EA5 4B1D 9E08 A097 C9AE

Mime
View raw message