Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Address code review comments
  • Loading branch information
voldemort authored and jayjwylie committed Jun 20, 2013
1 parent 407d4e4 commit 9d9b47e
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 11 deletions.
51 changes: 42 additions & 9 deletions rebalance-zone-expansion.sh
@@ -1,4 +1,4 @@
#!/bin/bash
#!/bin/bash -e

#
# Copyright 2013 LinkedIn, Inc
Expand All @@ -15,21 +15,23 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# This script uses getopts which means only a single character switch are allowed.
# This script uses getopts which means only single character switches are allowed.
# Using getopt would allow for multi charcter switch names but would come at a
# cost of being cross compatibility.
# cost of not being cross compatibility.

# This script generates a cluster.xml and a plan for the zone expansion
# Argument = -v vold_home -c current_cluster -s current_stores -i interim_cluster -f final_stores
# -o output dir

# Function to display usage
usage_and_exit() {
echo "ERROR: $1."
cat <<EOF
Usage: $0 options
OPTIONS:
-h Show this message
-v Path to Voldermort
-v Path to Voldemort
-c Current Cluster that desribes the cluster
-s Current Stores that desribes the store
-i Interim Cluster that corresponds to zone expansion.
Expand Down Expand Up @@ -95,15 +97,42 @@ then
exit 1
fi

if [ ! -d $vold_home ]; then
usage_and_exit "Directory '$vold_home' does not exist."
fi

if [ ! -e $current_cluster ]; then
usage_and_exit "File '$current_cluster' does not exist."
fi

if [ ! -e $current_stores ]; then
usage_and_exit "File '$current_stores' does not exist."
fi

if [ ! -e $interim_cluster ]; then
usage_and_exit "File '$interim_cluster' does not exist."
fi

if [ ! -e $final_stores ]; then
usage_and_exit "File '$final_stores' does not exist."
fi


# The final cluster.xml for zone expansion is generated in three steps.
# Step 1 : In step 1 1000 separate ierations of the reparitioner is executed and the one
# Step 1 : In step 1 1000 separate iterations of the repartitioner is executed and the one
# with the minimal utility value is chose for step 2.
# Step 2: In step 2, the cluster.xml from step 1 is fed to the repartitioner along with random swap
# attempts. The repartitioner randomly swaps the partitions and tries to balance the ring.
# attempts. The repartitioner randomly swaps the partitions only in zone 2
# and tries to balance the ring.
# Step 3: Finally, a plan is generated on how to reach from the orignal cluster topology to
# the one that is generated in step 2.
#

step2_zoneid=2
step2_swap_attempts=1000
step2_overall_iterations=5


# Step 1
for i in {1..1000}
do
Expand All @@ -120,15 +149,19 @@ done
#find the run with the best (minimal) utility value
bestUtil=$(grep "Utility" $output_dir/step1/*/final-cluster.xml.analysis | sort -nk 3 | cut -f4 -d / | head -n 1)

if [ ! -e $output_dir/step1/$bestUtil/final-cluster.xml ]; then
usage_and_exit "final cluster.xml from step1 does not exist"
fi

# Step 2
$vold_home/bin/run-class.sh voldemort.tools.RepartitionerCLI \
--current-cluster $output_dir/step1/$bestUtil/final-cluster.xml \
--current-stores $final_stores \
--output-dir $output_dir/step2 \
--enable-random-swaps \
--random-swap-zoneids 2 \
--attempts 5 \
--random-swap-attempts 1000 \
--random-swap-zoneids $step2_zoneid \
--attempts $step2_overall_iterations \
--random-swap-attempts $step2_swap_attempts \

# Step 3
mkdir -p $output_dir/step3/
Expand Down
99 changes: 97 additions & 2 deletions test/unit/voldemort/tools/RepartitionerTest.java
Expand Up @@ -20,8 +20,11 @@
import static org.junit.Assert.assertTrue;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.junit.Test;

Expand Down Expand Up @@ -157,7 +160,7 @@ public void verifyBalanceNodesNotZones(Cluster currentCluster,
false,
0,
0,
null,
null,
false,
null,
0,
Expand Down Expand Up @@ -240,7 +243,7 @@ public void verifyRandomSwapsImproveBalance(Cluster currentCluster,
enableRandomSwaps,
swapAttempts,
swapSuccesses,
null,
Collections.<Integer> emptyList(),
false,
null,
0,
Expand All @@ -256,6 +259,65 @@ public void verifyRandomSwapsImproveBalance(Cluster currentCluster,
assertTrue(repartitionedPb.getUtility() < currentPb.getUtility());
}

/**
* Verify that random swaps within improve balance
*
* @param currentCluster
* @param currentStores
* @param swapZoneIds
*/
public void verifyRandomSwapsWithinZoneOnlyShufflesParitionsInThatZone(Cluster currentCluster,
List<StoreDefinition> currentStores,
List<Integer> swapZoneIds) {

PartitionBalance currentPb = new PartitionBalance(currentCluster, currentStores);
// Disable basic balancing among zones & nodes
boolean disableNodeBalancing = true;
boolean disableZoneBalancing = true;
// Do some random swaps within zone
boolean enableRandomSwaps = true;
int swapAttempts = 100;
int swapSuccesses = 10;
Cluster repartitionedCluster = Repartitioner.repartition(currentCluster,
currentStores,
currentCluster,
currentStores,
null,
1,
disableNodeBalancing,
disableZoneBalancing,
enableRandomSwaps,
swapAttempts,
swapSuccesses,
swapZoneIds,
false,
null,
0,
0,
0,
0);
PartitionBalance repartitionedPb = new PartitionBalance(repartitionedCluster, currentStores);

Set<Integer> allNodeIds = repartitionedCluster.getNodeIds();
Set<Integer> swapNodeIds = null;
for (Integer swapZoneId: swapZoneIds) {
swapNodeIds = repartitionedCluster.getNodeIdsInZone(swapZoneId);
}
// Remove nodes that we don't want to verify the partition count on
allNodeIds.removeAll(swapNodeIds);
for (Integer remainingNodeId : allNodeIds) {
Set<Integer> beforeRepartition = new HashSet<Integer>(currentCluster
.getNodeById(remainingNodeId)
.getPartitionIds());
Set<Integer> afterRepartition = new HashSet<Integer>(currentCluster
.getNodeById(remainingNodeId)
.getPartitionIds());
assertTrue(beforeRepartition.equals(afterRepartition));
assertTrue(repartitionedPb.getUtility() <= currentPb.getUtility());
}

}

/**
* Verify that greedy swaps improve balance.
*
Expand Down Expand Up @@ -388,6 +450,39 @@ public void testShuffle() {
verifyGreedySwapsImproveBalance(currentCluster, storeDefs);
}

@Test
public void testShuffleWithinZone() {
// Two zone cluster
Cluster currentCluster = ClusterTestUtils.getZZCluster();
List<StoreDefinition> storeDefs = ClusterTestUtils.getZZStoreDefsInMemory();
List<Integer> swapZoneIds = new ArrayList<Integer>();
// Only shuffle within zone 1
swapZoneIds.add(1);
verifyRandomSwapsWithinZoneOnlyShufflesParitionsInThatZone(currentCluster, storeDefs,
swapZoneIds);

// Three zone cluster
currentCluster = ClusterTestUtils.getZZZCluster();
storeDefs = ClusterTestUtils.getZZZStoreDefsInMemory();
// Shuffle only within zone 2
swapZoneIds.clear();
swapZoneIds.add(2);
verifyRandomSwapsWithinZoneOnlyShufflesParitionsInThatZone(currentCluster, storeDefs,
swapZoneIds);
// Shuffle only within zone 1, 2
swapZoneIds.clear();
swapZoneIds.add(1);
swapZoneIds.add(2);
verifyRandomSwapsWithinZoneOnlyShufflesParitionsInThatZone(currentCluster, storeDefs,
swapZoneIds);
// Shuffle only within zone 0, 2
swapZoneIds.clear();
swapZoneIds.add(0);
swapZoneIds.add(2);
verifyRandomSwapsWithinZoneOnlyShufflesParitionsInThatZone(currentCluster, storeDefs,
swapZoneIds);
}

@Test
public void testClusterExpansion() {
// Two zone cluster
Expand Down

0 comments on commit 9d9b47e

Please sign in to comment.